jonmv commented on code in PR #1925:
URL: https://github.com/apache/zookeeper/pull/1925#discussion_r1095682994
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java:
##########
@@ -543,35 +543,150 @@ protected long registerWithLeader(int pktType) throws
IOException {
* @throws InterruptedException
*/
protected void syncWithLeader(long newLeaderZxid) throws Exception {
- QuorumPacket ack = new QuorumPacket(Leader.ACK, 0, null, null);
- QuorumPacket qp = new QuorumPacket();
long newEpoch = ZxidUtils.getEpochFromZxid(newLeaderZxid);
-
QuorumVerifier newLeaderQV = null;
- // In the DIFF case we don't need to do a snapshot because the
transactions will sync on top of any existing snapshot
- // For SNAP and TRUNC the snapshot is needed to save that history
- boolean snapshotNeeded = true;
- boolean syncSnapshot = false;
+ class SyncHelper {
+
+ // In the DIFF case we don't need to do a snapshot because the
transactions will sync on top of any existing snapshot.
+ // For SNAP and TRUNC the snapshot is needed to save that history.
+ boolean willSnapshot = true;
+ boolean syncSnapshot = false;
+
+ // PROPOSALs received during sync, for matching up with COMMITs.
+ Deque<PacketInFlight> proposals = new ArrayDeque<>();
+
+ // PROPOSALs we delay forwarding to the ZK server until sync is
done.
+ Deque<PacketInFlight> delayedProposals = new ArrayDeque<>();
+
+ // COMMITs we delay forwarding to the ZK server until sync is done.
+ Deque<Long> delayedCommits = new ArrayDeque<>();
+
+ public void syncSnapshot() {
+ syncSnapshot = true;
+ }
+
+ public void noSnapshot() {
+ willSnapshot = false;
+ }
+
+ public void propose(PacketInFlight pif) {
+ proposals.add(pif);
+ delayedProposals.add(pif);
+ }
+
+ public PacketInFlight nextProposal() {
+ return proposals.peekFirst();
+ }
+
+ public void commit() {
+ PacketInFlight packet = proposals.remove();
+ if (willSnapshot) {
+ zk.processTxn(packet.hdr, packet.rec);
+ delayedProposals.remove();
+ } else {
+ delayedCommits.add(packet.hdr.getZxid());
+ }
+ }
+
+ public void proposeAndCommit(PacketInFlight packet) {
+ // Should be able to do propose(packet); commit(); because
INFORM with non-empty proposals would be an error ...
Review Comment:
Yes ... I think this was a "maybe-do" while I was working on this. AFAICT
both `proposals` should remain empty while we're receiving `INFORM`s, and
`delayedProposals` should remain empty as long as `willSnapshot` remains
`true`, and that can only change to `false`—not back again.
If there are (erroneously) `INFORM`s after `PROPOSAL`s, this breaks down. I
wasn't confident enough to make the change at the time, but I see all tests
pass with this change, so I guess we should do it.
--
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.
To unsubscribe, e-mail: notifications-unsubscr...@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
