[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/zookeeper/pull/117


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[breed]

Github user breed commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/117#discussion_r90791441
  
--- Diff: 
src/java/test/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java ---
@@ -0,0 +1,134 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+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;
+import org.apache.zookeeper.PortAssignment;
+import org.apache.zookeeper.WatchedEvent;
+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;
+import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
+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
+ *  Test that zxid == -1 in the presence of emptied/deleted snapshots
+ */
+public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements  
Watcher {
+private static final Logger LOG = 
Logger.getLogger(RestoreCommittedLogTest.class);
+private static String HOSTPORT = "127.0.0.1:" + 
PortAssignment.unique();
+private static final int CONNECTION_TIMEOUT = 3000;
+private static final int N_TRANSACTIONS = 150;
+private static final int SNAP_COUNT = 100;
+
+public void runTest(boolean leaveEmptyFile) throws Exception {
--- End diff --

i think we should skip that test. we have a fix for ZOOKEEPER-261 that 
fixes that specific scenario.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[rgs1]

Github user rgs1 commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/117#discussion_r90761121
  
--- Diff: 
src/java/test/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java ---
@@ -0,0 +1,134 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+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;
+import org.apache.zookeeper.PortAssignment;
+import org.apache.zookeeper.WatchedEvent;
+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;
+import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
+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
+ *  Test that zxid == -1 in the presence of emptied/deleted snapshots
+ */
+public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements  
Watcher {
+private static final Logger LOG = 
Logger.getLogger(RestoreCommittedLogTest.class);
+private static String HOSTPORT = "127.0.0.1:" + 
PortAssignment.unique();
+private static final int CONNECTION_TIMEOUT = 3000;
+private static final int N_TRANSACTIONS = 150;
+private static final int SNAP_COUNT = 100;
+
+public void runTest(boolean leaveEmptyFile) throws Exception {
--- End diff --

@breed do you want to take @hanm's suggestion or should I merge this and we 
get that in another pass?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[hanm]

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/117#discussion_r90516233
  
--- Diff: 
src/java/test/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java ---
@@ -0,0 +1,134 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+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;
+import org.apache.zookeeper.PortAssignment;
+import org.apache.zookeeper.WatchedEvent;
+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;
+import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
+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
+ *  Test that zxid == -1 in the presence of emptied/deleted snapshots
+ */
+public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements  
Watcher {
+private static final Logger LOG = 
Logger.getLogger(RestoreCommittedLogTest.class);
+private static String HOSTPORT = "127.0.0.1:" + 
PortAssignment.unique();
+private static final int CONNECTION_TIMEOUT = 3000;
+private static final int N_TRANSACTIONS = 150;
+private static final int SNAP_COUNT = 100;
+
+public void runTest(boolean leaveEmptyFile) throws Exception {
--- End diff --

Test coverage improvement suggestion: here we don't cover the case where 
both transaction log files and snap shot files are missing (either deleted, or 
empty) - in which case the ZK server should happily recover w/o problem. 
Something like this should work: `runTest(boolean leaveEmptySnapshotFile, 
boolean leaveEmptyTxnLogFile)`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[hanm]

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/117#discussion_r90516950
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -165,8 +165,22 @@ public File getSnapDir() {
  */
 public long restore(DataTree dt, Map sessions,
 PlayBackListener listener) throws IOException {
-snapLog.deserialize(dt, sessions);
+long deserializeResult = snapLog.deserialize(dt, sessions);
 FileTxnLog txnLog = new FileTxnLog(dataDir);
+if (-1L == deserializeResult) {
+/* this means that we couldn't find any snapshot, so we need to
+ * initialize an empty database */
+if (txnLog.getLastLoggedZxid() != -1) {
+throw new IOException(
+"No snapshot found, but there are log entries. " +
+"Something is broken!");
+}
+/* TODO: (br33d) we should either put a ConcurrentHashMap on 
restore()
+ *   or use Map on save() */
+save(dt, (ConcurrentHashMap)sessions);
--- End diff --

I think we need it here because if we are getting here then the zxid of 
this server must be -1, so it would not win leader election if at least one 
other server is sane (with valid snapshot/txn log to recover.), so this server 
will become a follow and sync the (none empty) snapshot from the leader. If all 
servers have empty snapshots then this save is also required to bootstrap the 
recover process.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[breed]

Github user breed commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/117#discussion_r90523812
  
--- Diff: 
src/java/test/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java ---
@@ -0,0 +1,134 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+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;
+import org.apache.zookeeper.PortAssignment;
+import org.apache.zookeeper.WatchedEvent;
+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;
+import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
+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
+ *  Test that zxid == -1 in the presence of emptied/deleted snapshots
+ */
+public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements  
Watcher {
+private static final Logger LOG = 
Logger.getLogger(RestoreCommittedLogTest.class);
+private static String HOSTPORT = "127.0.0.1:" + 
PortAssignment.unique();
+private static final int CONNECTION_TIMEOUT = 3000;
+private static final int N_TRANSACTIONS = 150;
+private static final int SNAP_COUNT = 100;
+
+public void runTest(boolean leaveEmptyFile) throws Exception {
+File tmpSnapDir = ClientBase.createTmpDir();
+File tmpLogDir  = ClientBase.createTmpDir();
+ClientBase.setupTestEnv();
+ZooKeeperServer zks = new ZooKeeperServer(tmpSnapDir, tmpLogDir, 
3000);
+SyncRequestProcessor.setSnapCount(SNAP_COUNT);
+final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
+ServerCnxnFactory f = ServerCnxnFactory.createFactory(PORT, -1);
+f.startup(zks);
+Assert.assertTrue("waiting for server being up ",
+ClientBase.waitForServerUp(HOSTPORT,CONNECTION_TIMEOUT));
+ZooKeeper zk = new ZooKeeper(HOSTPORT, CONNECTION_TIMEOUT, this);
+try {
+for (int i = 0; i< N_TRANSACTIONS; i++) {
+zk.create("/node-" + i, new byte[0], Ids.OPEN_ACL_UNSAFE,
+CreateMode.PERSISTENT);
+}
+} finally {
+zk.close();
+}
+f.shutdown();
+zks.shutdown();
+Assert.assertTrue("waiting for server to shutdown",
+ClientBase.waitForServerDown(HOSTPORT, 
CONNECTION_TIMEOUT));
+
+// start server again with intact database
+zks = new ZooKeeperServer(tmpSnapDir, tmpLogDir, 3000);
+zks.startdata();
+long zxid = zks.getZKDatabase().getDataTreeLastProcessedZxid();
+LOG.info("After clean restart, zxid = " + zxid);
+Assert.assertTrue("zxid > 0", zxid > 0);
+zks.shutdown();
+
+// Make all snapshots empty
+FileTxnSnapLog txnLogFactory = zks.getTxnLogFactory();
+List snapshots = txnLogFactory.findNRecentSnapshots(10);
+Assert.assertTrue("We have a snapshot to corrupt", 
snapshots.size() > 0);
+for (File file: snapshots) {
+if (leaveEmptyFile) {
+new PrintWriter(file).close ();
+}
+else {
--- End diff --

yes that is ugly!


---
If your project is set up for it, you can reply to this email and have your
reply appear on 

[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[breed]

Github user breed commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/117#discussion_r90523518
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -165,8 +165,22 @@ public File getSnapDir() {
  */
 public long restore(DataTree dt, Map sessions,
 PlayBackListener listener) throws IOException {
-snapLog.deserialize(dt, sessions);
+long deserializeResult = snapLog.deserialize(dt, sessions);
 FileTxnLog txnLog = new FileTxnLog(dataDir);
+if (-1L == deserializeResult) {
+/* this means that we couldn't find any snapshot, so we need to
+ * initialize an empty database */
+if (txnLog.getLastLoggedZxid() != -1) {
+throw new IOException(
+"No snapshot found, but there are log entries. " +
+"Something is broken!");
+}
+/* TODO: (br33d) we should either put a ConcurrentHashMap on 
restore()
+ *   or use Map on save() */
+save(dt, (ConcurrentHashMap)sessions);
--- End diff --

yes, ZOOKEEPER-261 is still a problem. is that what you are referring to? 
brian has a patch coming that builds on this one to fix that problem.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[rgs1]

Github user rgs1 commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/117#discussion_r90491384
  
--- Diff: 
src/java/test/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java ---
@@ -0,0 +1,134 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+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;
+import org.apache.zookeeper.PortAssignment;
+import org.apache.zookeeper.WatchedEvent;
+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;
+import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
+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
+ *  Test that zxid == -1 in the presence of emptied/deleted snapshots
+ */
+public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements  
Watcher {
+private static final Logger LOG = 
Logger.getLogger(RestoreCommittedLogTest.class);
+private static String HOSTPORT = "127.0.0.1:" + 
PortAssignment.unique();
+private static final int CONNECTION_TIMEOUT = 3000;
+private static final int N_TRANSACTIONS = 150;
+private static final int SNAP_COUNT = 100;
+
+public void runTest(boolean leaveEmptyFile) throws Exception {
+File tmpSnapDir = ClientBase.createTmpDir();
+File tmpLogDir  = ClientBase.createTmpDir();
+ClientBase.setupTestEnv();
+ZooKeeperServer zks = new ZooKeeperServer(tmpSnapDir, tmpLogDir, 
3000);
+SyncRequestProcessor.setSnapCount(SNAP_COUNT);
+final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
+ServerCnxnFactory f = ServerCnxnFactory.createFactory(PORT, -1);
+f.startup(zks);
+Assert.assertTrue("waiting for server being up ",
+ClientBase.waitForServerUp(HOSTPORT,CONNECTION_TIMEOUT));
+ZooKeeper zk = new ZooKeeper(HOSTPORT, CONNECTION_TIMEOUT, this);
+try {
+for (int i = 0; i< N_TRANSACTIONS; i++) {
+zk.create("/node-" + i, new byte[0], Ids.OPEN_ACL_UNSAFE,
+CreateMode.PERSISTENT);
+}
+} finally {
+zk.close();
+}
+f.shutdown();
+zks.shutdown();
+Assert.assertTrue("waiting for server to shutdown",
+ClientBase.waitForServerDown(HOSTPORT, 
CONNECTION_TIMEOUT));
+
+// start server again with intact database
+zks = new ZooKeeperServer(tmpSnapDir, tmpLogDir, 3000);
+zks.startdata();
+long zxid = zks.getZKDatabase().getDataTreeLastProcessedZxid();
+LOG.info("After clean restart, zxid = " + zxid);
+Assert.assertTrue("zxid > 0", zxid > 0);
+zks.shutdown();
+
+// Make all snapshots empty
+FileTxnSnapLog txnLogFactory = zks.getTxnLogFactory();
+List snapshots = txnLogFactory.findNRecentSnapshots(10);
+Assert.assertTrue("We have a snapshot to corrupt", 
snapshots.size() > 0);
+for (File file: snapshots) {
+if (leaveEmptyFile) {
+new PrintWriter(file).close ();
+}
+else {
--- End diff --

nit: coding style `} else {`


---
If your project is set up for it, you can reply to this email and have your
reply 

[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[rgs1]

Github user rgs1 commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/117#discussion_r90490925
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -165,8 +165,22 @@ public File getSnapDir() {
  */
 public long restore(DataTree dt, Map sessions,
 PlayBackListener listener) throws IOException {
-snapLog.deserialize(dt, sessions);
+long deserializeResult = snapLog.deserialize(dt, sessions);
 FileTxnLog txnLog = new FileTxnLog(dataDir);
+if (-1L == deserializeResult) {
+/* this means that we couldn't find any snapshot, so we need to
+ * initialize an empty database */
--- End diff --

nit: can you add a reference to ZOOKEEPER-2325 here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[lvfangmin]

Github user lvfangmin commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/117#discussion_r90368114
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -165,8 +165,22 @@ public File getSnapDir() {
  */
 public long restore(DataTree dt, Map sessions,
 PlayBackListener listener) throws IOException {
-snapLog.deserialize(dt, sessions);
+long deserializeResult = snapLog.deserialize(dt, sessions);
 FileTxnLog txnLog = new FileTxnLog(dataDir);
+if (-1L == deserializeResult) {
+/* this means that we couldn't find any snapshot, so we need to
+ * initialize an empty database */
+if (txnLog.getLastLoggedZxid() != -1) {
+throw new IOException(
+"No snapshot found, but there are log entries. " +
+"Something is broken!");
+}
+/* TODO: (br33d) we should either put a ConcurrentHashMap on 
restore()
+ *   or use Map on save() */
+save(dt, (ConcurrentHashMap)sessions);
--- End diff --

n00b question, why we need to save here? I saw there is potential that the 
follower will sync with with leader just to send over the empty snapshot.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #117: ZOOKEEPER-2325: Data inconsistency if all snaps...

[breed]

GitHub user breed opened a pull request:

https://github.com/apache/zookeeper/pull/117

ZOOKEEPER-2325: Data inconsistency if all snapshots empty or missing



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/breed/zookeeper ZOOKEEPER-2325

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/117.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #117


commit 02bf3d57786d51da205e78a070a45703da21f916
Author: Benjamin Reed 
Date:   2016-11-29T22:08:22Z

ZOOKEEPER-2325: Data inconsistency if all snapshots empty or missing




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---