[GitHub] zookeeper pull request #453: ZOOKEEPER-2845: Send a SNAP if transactions can...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r167838309
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/LearnerHandlerTest.java ---
@@ -462,6 +469,8 @@ public void testNewEpochZxid() throws Exception {
// Peer has zxid of epoch 1
peerZxid = getZxid(1, 0);
+//We are on a different epoch so we don't know 1, 0 is in our log
or not.
+// So we need to do a full SNAP
--- End diff --
I think this comment has been added by mistake. You added (1,0) to the log
above, hence syncFollower() returns false which means we don't need to do full
SNAP.
---
[GitHub] zookeeper pull request #453: ZOOKEEPER-2845: Send a SNAP if transactions can...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r167838605
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/LearnerHandlerTest.java ---
@@ -498,31 +507,20 @@ public void testNewEpochZxidWithTxnlogOnly() throws
Exception {
// Peer has zxid of epoch 3
peerZxid = getZxid(3, 0);
-assertFalse(learnerHandler.syncFollower(peerZxid, db, leader));
-// We send DIFF to (6,0) and forward any packet starting at (4,1)
-assertOpType(Leader.DIFF, getZxid(6, 0), getZxid(4, 1));
-// DIFF + 1 proposals + 1 commit
-assertEquals(3, learnerHandler.getQueuedPackets().size());
-queuedPacketMatches(new long[] { getZxid(4, 1)});
+//There is no 3, 0 proposal in the committed log so sync
+assertTrue(learnerHandler.syncFollower(peerZxid, db, leader));
--- End diff --
It seems to me that this test checking the same thing 3 times in a row.
Do you think it's necessary to do so?
---
[GitHub] zookeeper pull request #453: ZOOKEEPER-2845: Send a SNAP if transactions can...
Github user mfenes commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r167835407
--- Diff:
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java ---
@@ -758,6 +760,11 @@ public boolean syncFollower(long peerLastZxid,
ZKDatabase db, Leader leader) {
currentZxid = maxCommittedLog;
needOpPacket = false;
needSnap = false;
+} else if (peerLastEpoch != lastProcessedEpoch &&
!db.isInCommittedLog(peerLastZxid)) {
--- End diff --
Could you please add a description to the comments above (to "Here are the
cases that we want to handle") what this else if case is doing?
---
[GitHub] zookeeper pull request #453: ZOOKEEPER-2845: Send a SNAP if transactions can...
Github user mfenes commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r167513290
--- Diff:
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java ---
@@ -758,6 +760,11 @@ public boolean syncFollower(long peerLastZxid,
ZKDatabase db, Leader leader) {
currentZxid = maxCommittedLog;
needOpPacket = false;
needSnap = false;
+} else if (peerLastEpoch != lastProcessedEpoch &&
!db.isInCommittedLog(peerLastZxid)) {
+//Be sure we do a snap, because if the epochs are not the
same we don't know what
+// could have happened in between and it may take a TRUNC
+ UPDATES to get them in SYNC
+LOG.debug("Will send SNAP to peer sid: {} epochs are too
our of sync local 0x{} remote 0x{}",
--- End diff --
I think there is a typo here: "our of sync"
---
[GitHub] zookeeper pull request #453: ZOOKEEPER-2845: Send a SNAP if transactions can...
GitHub user revans2 opened a pull request: https://github.com/apache/zookeeper/pull/453 ZOOKEEPER-2845: Send a SNAP if transactions cannot be verified. I will be creating a patch/pull request for 3.4 and 3.5 too, but I wanted to get a pull request up for others to look at ASAP. I have a version of this based off of #310 at https://github.com/revans2/zookeeper/tree/ZOOKEEPER-2845-orig-test-patch but the test itself is flaky. Frequently leader election does not go as planned on the test and it ends up failing but not because it ended up in an inconsistent state. I am happy to answer any questions anyone has about the patch. You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/zookeeper ZOOKEEPER-2845-master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/453.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 #453 commit 0219b2c9e44527067cd5fed4b642729171721886 Author: Robert Evans Date: 2018-01-29T20:27:10Z ZOOKEEPER-2845: Send a SNAP if transactions cannot be verified. ---
