Repository navigation
Conversation
Root cause: RaftNode::become_leader (xerj-cluster/src/raft.rs) only reset
next_index/match_index and appended nothing. The commit rule in
handle_append_entries_response correctly counts replicas only for entries
of the current term (Raft §5.4.2), so a tail left by the previous leader,
even when it sat on a majority, stayed uncommitted until a client proposed
something. Metadata writes interrupted by an election hung at commit=0 and
timed out. The rc.101 CHANGELOG "Fixed" line claimed this fix; no such code
existed.
Fix:
- ClusterCommand::Noop, added as the last variant. Entries are serde_json
(externally tagged), so older variants encode exactly as before; Noop
encodes as "Noop". An older build cannot decode it (upgrade note added).
- become_leader appends Noop at current_term through append_own, the same
persist path propose uses (append + fsync + log end; single-node commits
at once), and returns false after stepping down to follower if that
write fails. Both callers send AppendEntries immediately on success.
- Apply sites ignore it: ClusterMetadata::apply has an empty arm (the
applied index still advances); ClusterNode::apply_entry returns before
the engine applier and the waiter lookup, so no client waiter resolves
on a Noop index (waiters key on ClientMetadata request ids anyway).
cluster_apply's catch-all already returns Applied; snapshot/compaction
and take_installed_entries carry it like any entry.
Tests:
- tests/prev_term_commit.rs (from the audit repro): entry on a majority
is committed by the new leader. main: FAILED (leader commit=0);
this branch: ok, 20/20 runs.
- unit: a new leader's log ends with a Noop of its term; a leader that
cannot persist its no-op steps down and sends nothing.
- persistent_raft_test: Noop round-trips through the file log.
- log_catchup deflake: proposals go through propose_until, which
re-proposes only when the write is provably dead (a member committed a
different entry at its index); a timer-based retry committed CreateIndex
twice in one 60 ms run (6 create events vs 3). Members step in parallel,
as separate machines do, instead of summing each member's fsync waits.
No timeout changed (pump 10 s, propose 20 s).
Numbers (log_catchup binary under strace fsync/fdatasync delay injection,
--test-threads=2, 10 runs per delay):
before (main): 20ms 3/10 fail, 40ms 7/10, 60ms 10/10;
leader commit=0 timeouts 0/4/7 = 11 in 30 runs
after (no-op + retry): 20ms 0/10, 40ms 0/10, 60ms 10/10; commit=0 0
after (+parallel step): 20ms 0/10, 40ms 0/10, 60ms 7/10; commit=0 0
The remaining 60 ms failures are all "no leader": every member a
candidate or leaderless at the 20 s deadline. Each vote or term change
costs two syncs (hard.meta write + dir sync) and each append three, which
at 60 ms per sync exceeds the 150-300 ms election timeout. That is an
election-timeout vs disk-latency problem, not this bug; left for a
follow-up.
cargo test --release -p xerj-cluster: 127 passed, 0 failed, 5/5 runs.
xerj-api cluster_metadata_replicates 18/18, cluster_metadata_survives_restart
13/13, cluster_settings_merge 5/5; xerj-engine cluster_state_compatibility
7/7, lib cluster* 13/13.
The Unreleased note that corrects the premature rc.101 claim now names the PR that carries the fix.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A newly elected leader never committed entries left by the previous term.
RaftNode::become_leader(engine/crates/xerj-cluster/src/raft.rs) only resetnext_index/match_index. The commit rule inhandle_append_entries_responsecorrectly counts replicas only for entries of the current term (Raft §5.4.2), so a tail from term T-1 stayed atcommit=0until some client proposed something. Metadata writes interrupted by an election hung and timed out. The rc.101 CHANGELOG "Fixed" line said this was fixed, but no such code existed.Fix
ClusterCommand::Noop, added as the last variant. Log entries, snapshots and wire messages areserde_json(externally tagged), so every existing variant encodes exactly as before.Noopencodes as"Noop".become_leaderappends aNoopatcurrent_termthroughappend_own, the persist pathproposenow shares (append, fsync, log end; a single-node cluster commits it at once). Both callers send AppendEntries right after. If the persist fails, the node steps down to follower and sends nothing, the same waystart_electionhandles a failed vote write. The leader counts itself implicitly in the commit rule (1 + match_index ≥ n), so it has no ownmatch_indexentry to update.ClusterMetadata::applyhas an empty arm (the applied index still advances).ClusterNode::apply_entryreturns before the engine applier and before the waiter lookup, so no client waiter can resolve on a Noop index (waiters are keyed onClientMetadatarequest ids anyway). The catch-all incluster_applyalready returnsApplied. Snapshots, compaction andtake_installed_entriescarry it like any other entry.An older build cannot decode
Noop: a frame failsdeserialize RaftMessage, and an on-disk entry failsdecode raft entry. A rolling upgrade has to upgrade every member before a new-build node can lead, and there is no in-place downgrade after a new build has written a no-op. This is in the CHANGELOG upgrade notes.Regression test:
tests/prev_term_commit.rsAdapted from the audit repro. n1 leads term 1 and appends entry X. Only n2 stores X, and n2's ack is lost. n1 goes away and n2 wins term 2 with n3's vote. Assertion: entry on a majority is committed by the new leader.
entry 1 on a majority is committed by the new leader (leader commit=0)New unit tests: a freshly elected leader's log ends with a
Noopat its own term, and a leader that cannot persist its no-op (blockedhard.meta.tmp) steps down and sends nothing.persistent_raft_testchecks thatNoopround-trips through the file log.log_catchup deflake (D2)
propose_until, which finds the leader and re-proposes only once the write is provably dead, meaning some member has committed a different entry at its index. The first version re-proposed whenever the new leader lacked the entry. Under 60 ms fsync it committedCreateIndextwice in one run (6 create events instead of 3), because a later leader that still held the entry committed it as well. The no-op is what makes a dead write show up promptly. The task text suggested going throughClusterHandle. These tests driveClusterNode::stepfrom the test task itself, so awaiting a handle proposal there would block the loop that has to commit it.pump_untilis still 10 s, andpropose_untiluses the 20 s deadline from the brief.Numbers (all from runs on this host)
log_catchuptest binary underTMPDIR=<ext4 dir> strace -f -qq -o /dev/null -e trace=fsync,fdatasync -e inject=fsync:delay_exit=D -e inject=fdatasync:delay_exit=D $B --test-threads=2, 10 runs per delay. "commit=0" counts timeouts where the leader was stuck at commit=0.The 60 ms target (≤1/10) is not met. All of the remaining 60 ms failures are "no leader": every member is a candidate or has no leader when the 20 s deadline hits. None is a commit stall. Each term or vote change costs two syncs (the
hard.metawrite and a directory sync), and each append costs three. At 60 ms per sync that is longer than the 150–300 ms election timeout, so elections livelock. That is a mismatch between the election timeout and disk latency (pre-vote, or a timeout scaled to measured fsync latency). It is a separate follow-up, and I did not loosen timeouts here.Other suites:
cargo test --release -p xerj-cluster: 127 passed, 0 failed, 5/5 runs. fmt and clippy-D warningsare clean.cluster_metadata_replicates18/18,cluster_metadata_survives_restart13/13,cluster_settings_merge5/5.cluster_state_compatibility7/7, libcluster*13/13.Reference-coding
No raft or consensus corpus exists in
~/.xerj-code/corpora(no etcd, raft-rs, tikv or openraft). The reference server on :9200 reports 0 of 21 corpora loaded, and those indexes are more than 30 days old. Nothing relevant was retrieved. The design follows the Raft paper (§5.4.2, §8: "each leader commit a blank no-op entry into the log at the start of its term").🤖 Generated with Claude Code