Skip to content

ZOOKEEPER-5094: Discard incomplete state after interrupted TRUNC synchronization - #2459

Open
JHSUYU wants to merge 2 commits into
apache:masterfrom
JHSUYU:fix-trunc-sync-shutdown-state
Open

JHSUYU wants to merge 2 commits into
apache:masterfrom
JHSUYU:fix-trunc-sync-shutdown-state

Conversation

@JHSUYU

@JHSUYU JHSUYU commented Sep 17, 2026

Copy link
Copy Markdown

JIRA: ZOOKEEPER-5094

Description

A follower interrupted during TRUNC synchronization can retain an in-memory database that is ahead of its transaction log.

For TRUNC synchronization, syncWithLeader() sets snapshotNeeded to true. Committed transactions received before NEWLEADER are therefore applied directly to memory:

if (!writeToTxnLog) {
    zk.processTxn(pif.toRequest());
}

The normal path persists this state by taking a snapshot when NEWLEADER arrives:

if (snapshotNeeded) {
    zk.takeSnapshot(syncSnapshot);
}

If the quorum connection fails before NEWLEADER, the follower can be left with:

in-memory lastProcessedZxid = Z2
on-disk transaction log     = Z1

Learner.shutdown() previously requested a full database shutdown only for an incomplete SNAP synchronization:

zk.shutdown(
    self.getSyncMode().equals(QuorumPeer.SyncMode.SNAP)
);

An incomplete TRUNC database was therefore retained.

On the next connection, getLastLoggedZxid() reads Z2 from the initialized in-memory database. The leader assumes Z2 is already durable and can continue with Z3. Once Z3 is persisted, the follower's transaction log contains a gap:

memory: Z1 -> Z2 -> Z3
disk:   Z1 ------> Z3

After a process restart, transaction-log replay detects the gap in ZKDatabase.addCommittedProposal() and throws:

IllegalStateException: Committed proposal cached out of order:
Z3 is not the next proposal of Z1

The follower then fails during database loading and cannot rejoin the quorum.

Solution

Treat an incomplete TRUNC synchronization like an incomplete SNAP synchronization during learner shutdown:

QuorumPeer.SyncMode syncMode = self.getSyncMode();
zk.shutdown(
    syncMode == QuorumPeer.SyncMode.SNAP
        || syncMode == QuorumPeer.SyncMode.TRUNC
);

This clears the incomplete in-memory database. On the next synchronization attempt, the follower reloads its durable zxid from disk and requests all missing transactions from the leader.

Completed synchronization is unaffected because syncWithLeader() changes the mode to NONE after the required snapshot and transaction persistence have completed.

Tests

Added LearnerTest.incompleteTruncSyncClearsInMemoryDatabase.

The test:

  1. Persists Z1 and a divergent Z2 in the follower's transaction log.
  2. Sends TRUNC(Z1).
  3. Sends a replacement PROPOSAL(Z2) and COMMIT(Z2).
  4. Ends the input stream before NEWLEADER, simulating a quorum connection failure.
  5. Verifies that replacement Z2 exists in memory while the disk still ends at Z1.
  6. Shuts down the learner and verifies that the incomplete database is cleared.
  7. Reloads the database and verifies that it correctly returns to the durable Z1 state.

Without the fix, the database remains initialized at Z2 and the test fails. With the fix, the database is reloaded from Z1 and can be synchronized correctly.

@JHSUYU

JHSUYU commented Sep 17, 2026

Copy link
Copy Markdown
Author

Hi @kezhuw, thanks for reviewing my previous PR #2454. Could you take a look at this one when you have time? It's a similar sync-phase issue: an interrupted TRUNC sync keeps non-durable state in memory, which later causes a txn log gap and startup failure. Thanks!

@kezhuw kezhuw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the next connection, getLastLoggedZxid() reads Z2 from the initialized in-memory database. The leader assumes Z2 is already durable and can continue with Z3. Once Z3 is persisted, the follower's transaction log contains a gap:

After a process restart, transaction-log replay detects the gap in ZKDatabase.addCommittedProposal() and throws:

It would be nice for tests to assert above.

Comment on lines +406 to +407
assertFalse(zkDb.isInitialized());
assertEquals(1, zkDb.loadDataBase());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertFalse(zkDb.isInitialized());
assertEquals(1, zkDb.loadDataBase());
if (!zkDb.isInitialized()) {
zkDb.loadDataBase();
}

I think we can treat zkDb.isInitialized() as implementation details, so we can assert what important.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in a follow-up commit.

@JHSUYU
JHSUYU requested a review from kezhuw September 18, 2026 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants