[GitHub] [zookeeper] eolivelli commented on a change in pull request #1445: ZOOKEEPER-3911: Data inconsistency caused by DIFF sync uncommitted log

[GitBox <gi...@apache.org>]

eolivelli commented on a change in pull request #1445:
URL: https://github.com/apache/zookeeper/pull/1445#discussion_r512734631



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
##########
@@ -741,18 +741,30 @@ protected void syncWithLeader(long newLeaderZxid) throws Exception {
                     }
 
                     self.setCurrentEpoch(newEpoch);
-                    writeToTxnLog = true; //Anything after this needs to go to the transaction log, not applied directly in memory
+                    writeToTxnLog = true;
+                    //Anything after this needs to go to the transaction log, not applied directly in memory
                     isPreZAB1_0 = false;
+
+                    // ZOOKEEPER-3911: make sure sync the uncommitted logs before commit them (ACK NEWLEADER).
+                    sock.setSoTimeout(self.tickTime * self.syncLimit);
+                    self.setSyncMode(QuorumPeer.SyncMode.NONE);
+                    zk.startupWithoutServing();
+                    if (zk instanceof FollowerZooKeeperServer) {
+                        FollowerZooKeeperServer fzk = (FollowerZooKeeperServer) zk;
+                        for (PacketInFlight p : packetsNotCommitted) {
+                            fzk.logRequest(p.hdr, p.rec, p.digest);
+                        }
+                        packetsNotCommitted.clear();
+                    }
+
                     writePacket(new QuorumPacket(Leader.ACK, newLeaderZxid, null, null), true);
                     break;
                 }
             }
         }
         ack.setZxid(ZxidUtils.makeZxid(newEpoch, 0));
         writePacket(ack, true);
-        sock.setSoTimeout(self.tickTime * self.syncLimit);
-        self.setSyncMode(QuorumPeer.SyncMode.NONE);
-        zk.startup();
+        zk.startServing();

Review comment:
       One question:
   
   this "startServing" works if you executed 'zk.startupWithoutServing();' in your new code.
   but in other code paths we are not calling "startupWithoutServing"
   so when we come to this point zk.startServing() will only change the state to RUNNING, and we had not called 'startupWithServerState'
   
   can you please explain ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org