zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From eolive...@apache.org
Subject [zookeeper] branch branch-3.5 updated: ZOOKEEPER-3056: Fails to load database with missing snapshot file but with valid transaction log file.
Date Tue, 03 Sep 2019 06:51:17 GMT
This is an automated email from the ASF dual-hosted git repository.

eolivelli pushed a commit to branch branch-3.5
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/branch-3.5 by this push:
     new 77496b9  ZOOKEEPER-3056: Fails to load database with missing snapshot file but with
valid transaction log file.
77496b9 is described below

commit 77496b948f1eec3f8fa243bb8003cc97839982e4
Author: Michael Han <lhan@twitter.com>
AuthorDate: Tue Sep 3 08:51:10 2019 +0200

    ZOOKEEPER-3056: Fails to load database with missing snapshot file but with valid transaction
log file.
    
    This is a port of patch https://github.com/apache/zookeeper/pull/1069
    
    Author: Michael Han <lhan@twitter.com>
    
    Reviewers: Enrico Olivelli<eolivelli@apache.org>, Brian Nixon <nixon@fb.com>
    
    Closes #1072 from hanm/ZOOKEEPER-3056-3.5
---
 .../src/main/resources/markdown/zookeeperAdmin.md  | 14 +++++++++
 .../server/persistence/FileTxnSnapLog.java         | 18 ++++++++++--
 .../test/EmptiedSnapshotRecoveryTest.java          | 33 ++++++++++++++++------
 3 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index fd0f8ee..73a6f9d 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -702,6 +702,20 @@ property, when available, is noted below.
     This should be set to `NettyServerCnxnFactory` in order to use TLS based server communication.
     Default is `NIOServerCnxnFactory`.
 
+* *snapshot.trust.empty* :
+    (Java system property only: **zookeeper.snapshot.trust.empty**)
+    **New in 3.5.6:**
+    This property controls whether or not ZooKeeper should treat missing
+    snapshot files as a fatal state that can't be recovered from.
+    Set to true to allow ZooKeeper servers recover without snapshot
+    files. This should only be set during upgrading from old versions of
+    ZooKeeper (3.4.x, pre 3.5.3) where ZooKeeper might only have transaction
+    log files but without presence of snapshot files. If the value is set
+    during upgrade, we recommend to set the value back to false after upgrading
+    and restart ZooKeeper process so ZooKeeper can continue normal data
+    consistency check during recovery process.
+    Default value is false.
+
 <a name="sc_clusterOptions"></a>
 
 #### Cluster Options
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
index dbd5279..47d8799 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
@@ -55,6 +55,7 @@ public class FileTxnSnapLog {
     private final File snapDir;
     private TxnLog txnLog;
     private SnapShot snapLog;
+    private final boolean trustEmptySnapshot;
     public final static int VERSION = 2;
     public final static String version = "version-";
 
@@ -65,6 +66,10 @@ public class FileTxnSnapLog {
 
     public static final String ZOOKEEPER_DATADIR_AUTOCREATE_DEFAULT = "true";
 
+    public static final String ZOOKEEPER_SNAPSHOT_TRUST_EMPTY = "zookeeper.snapshot.trust.empty";
+
+    private static final String EMPTY_SNAPSHOT_WARNING = "No snapshot found, but there are
log entries. ";
+
     /**
      * This listener helps
      * the external apis calling
@@ -94,6 +99,9 @@ public class FileTxnSnapLog {
                 System.getProperty(ZOOKEEPER_DATADIR_AUTOCREATE,
                         ZOOKEEPER_DATADIR_AUTOCREATE_DEFAULT));
 
+        trustEmptySnapshot = Boolean.getBoolean(ZOOKEEPER_SNAPSHOT_TRUST_EMPTY);
+        LOG.info(ZOOKEEPER_SNAPSHOT_TRUST_EMPTY + " : " + trustEmptySnapshot);
+
         if (!this.dataDir.exists()) {
             if (!enableAutocreate) {
                 throw new DatadirException("Missing data directory "
@@ -208,9 +216,13 @@ public class FileTxnSnapLog {
             /* this means that we couldn't find any snapshot, so we need to
              * initialize an empty database (reported in ZOOKEEPER-2325) */
             if (txnLog.getLastLoggedZxid() != -1) {
-                throw new IOException(
-                        "No snapshot found, but there are log entries. " +
-                        "Something is broken!");
+                // ZOOKEEPER-3056: provides an escape hatch for users upgrading
+                // from old versions of zookeeper (3.4.x, pre 3.5.3).
+                if (!trustEmptySnapshot) {
+                    throw new IOException(EMPTY_SNAPSHOT_WARNING + "Something is broken!");
+                } else {
+                    LOG.warn(EMPTY_SNAPSHOT_WARNING + "This should only be allowed during
upgrading.");
+                }
             }
             /* TODO: (br33d) we should either put a ConcurrentHashMap on restore()
              *       or use Map on save() */
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java
index 5b2f8a4..9cb91bb 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java
@@ -22,7 +22,6 @@ import java.io.IOException;
 import java.io.File;
 import java.io.PrintWriter;
 import java.util.List;
-import java.util.LinkedList;
 
 import org.apache.log4j.Logger;
 import org.apache.zookeeper.CreateMode;
@@ -32,7 +31,6 @@ import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.ZooDefs.Ids;
-import org.apache.zookeeper.server.quorum.Leader.Proposal;
 import org.apache.zookeeper.server.ServerCnxnFactory;
 import org.apache.zookeeper.server.SyncRequestProcessor;
 import org.apache.zookeeper.server.ZooKeeperServer;
@@ -41,7 +39,7 @@ import org.junit.Assert;
 import org.junit.Test;
 
 /** If snapshots are corrupted to the empty file or deleted, Zookeeper should 
- *  not proceed to read its transactiong log files
+ *  not proceed to read its transaction log files
  *  Test that zxid == -1 in the presence of emptied/deleted snapshots
  */
 public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements  Watcher {
@@ -51,7 +49,7 @@ public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements 
Watcher
     private static final int N_TRANSACTIONS = 150;
     private static final int SNAP_COUNT = 100;
 
-    public void runTest(boolean leaveEmptyFile) throws Exception {
+    public void runTest(boolean leaveEmptyFile, boolean trustEmptySnap) throws Exception
{
         File tmpSnapDir = ClientBase.createTmpDir();
         File tmpLogDir  = ClientBase.createTmpDir();
         ClientBase.setupTestEnv();
@@ -96,15 +94,29 @@ public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements
 Watcher
             }
         }
 
+        if (trustEmptySnap) {
+          System.setProperty(FileTxnSnapLog.ZOOKEEPER_SNAPSHOT_TRUST_EMPTY, "true");
+        }
+
         // start server again with corrupted database
         zks = new ZooKeeperServer(tmpSnapDir, tmpLogDir, 3000);
         try {
             zks.startdata();
             zxid = zks.getZKDatabase().loadDataBase();
-            Assert.fail("Should have gotten exception for corrupted database");
+            if (!trustEmptySnap) {
+                Assert.fail("Should have gotten exception for corrupted database");
+            }
         } catch (IOException e) {
             // expected behavior
-        } 
+            if (trustEmptySnap) {
+                Assert.fail("Should not get exception for empty database");
+            }
+        } finally {
+            if (trustEmptySnap) {
+              System.clearProperty(FileTxnSnapLog.ZOOKEEPER_SNAPSHOT_TRUST_EMPTY);
+            }
+        }
+
         zks.shutdown();
     }
 
@@ -114,7 +126,7 @@ public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements
 Watcher
      */
     @Test
     public void testRestoreWithEmptySnapFiles() throws Exception {
-        runTest(true);
+        runTest(true, false);
     }
 
     /**
@@ -123,7 +135,12 @@ public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements
 Watcher
      */
     @Test
     public void testRestoreWithNoSnapFiles() throws Exception {
-        runTest(false);
+        runTest(false, false);
+    }
+
+    @Test
+    public void testRestoreWithTrustedEmptySnapFiles() throws Exception {
+        runTest(false, true);
     }
 
     public void process(WatchedEvent event) {


Mime
View raw message