diff --git a/tidal-server/src/cluster/election_driver.rs b/tidal-server/src/cluster/election_driver.rs index 42d37f4..44e9961 100644 --- a/tidal-server/src/cluster/election_driver.rs +++ b/tidal-server/src/cluster/election_driver.rs @@ -777,7 +777,31 @@ fn decide_join( return JoinDecision::Quarantine; } match own.cmp(&prev_log) { - std::cmp::Ordering::Less => JoinDecision::ReseedRequired, + std::cmp::Ordering::Less => { + // `own < prev_log` lexically on (tail_term, frontier). That is + // genuinely BEHIND only when the FRONTIER is behind. The other way to + // land here is a caught-up node whose WAL-tail term is merely stale: a + // checkpoint-based reseed restores an EMPTY WAL, so `tail_term` reads 0 + // (m12 reseed-loop-fix) while the replication frontier already covers + // the leader's election baseline. Such a node holds every committed + // entry up to the baseline — it is NOT behind, it only lacks the + // term-marker RECORD. Reseeding it is futile: the leader answers + // `needed=false` (its WAL covers the frontier), so it never installs, + // never repairs the marker, and self-restart-loops forever (observed + // live on tidaldb-2's CAUGHT-UP shards, which an install-only term- + // marker synthesis cannot reach). Classify it Clean; the clean-join + // path (`note_term_joined`) durably repairs the term marker. Divergence + // is already excluded above — a future-term marker (`tail_term > term`) + // and an un-replicated leader-acked suffix (`leader_acked > + // prev_log.frontier`) both quarantine first — so a frontier at/above + // the baseline with nothing leader-acked beyond it is caught up, not + // divergent. + if own.frontier >= prev_log.frontier { + JoinDecision::Clean + } else { + JoinDecision::ReseedRequired + } + } // Greater (a benign uncommitted/replicated tail) or Equal → clean. _ => JoinDecision::Clean, } @@ -1019,45 +1043,63 @@ mod tests { ); } - /// m12 reseed-loop-fix: the durable term marker is what stops the - /// false-reseed loop a checkpoint-restored (empty-WAL) node hits. - /// - /// `election_log_position` builds `own` from `wal_term_mark()`. Before the - /// fix the restored WAL is empty so `tail_term = 0`, and because - /// `decide_join` compares `(tail_term, frontier)` with `tail_term` FIRST, - /// `0 < leader_term` classifies the node `ReseedRequired` on EVERY boot — the - /// correctly-seeded frontier is never even consulted. The seed now durably - /// synthesizes the artifact's term marker, so `own.tail_term` is truthful and - /// the within-term clause classifies the reseeded shard `Clean`. + /// m12 reseed-loop-fix: a checkpoint-restored (empty-WAL) node reads + /// `tail_term = 0` from `wal_term_mark()`. `decide_join` must classify it on + /// whether its FRONTIER is caught up — NOT loop it through a futile reseed + /// just because the lexicographic `(tail_term, frontier)` compare is dominated + /// by `tail_term`. Two complementary mechanisms keep it out of the loop: the + /// frontier-covers-baseline arm here (for a CAUGHT-UP shard the leader answers + /// needed=false, so it never installs), and the durable term marker (for a + /// shard that DID install) reaching the `tail_term == term` fast path. #[test] - fn reseed_term_marker_breaks_the_false_reseed_loop() { + fn reseed_loop_fix_classifies_caught_up_behind_and_divergent() { let leader_term = 7; - let snapshot_seq = 1_040_000; // the seeded frontier (Fix 1) let baseline = 1_040_000; // the leader's election-time frontier - // THE BUG: a checkpoint-restored node reports tail_term 0. Even with the - // frontier correctly seeded to `snapshot_seq`, the term-0 tail loses the - // lexicographic compare → ReseedRequired → re-latch → loop. (A follower - // never ack=leader-writes, so leader_acked = 0.) + // CAUGHT UP after a checkpoint-restore reseed: tail_term reads 0 (empty + // WAL) but the frontier already COVERS the leader's baseline. It holds + // every committed entry up to the baseline — it only lacks the term-marker + // RECORD. Clean (the clean-join path durably repairs the marker). Before + // the fix this looped ReseedRequired forever — the leader answers + // needed=false, so the reseed never installs and never breaks the loop. + assert_eq!( + decide_join(leader_term, pos(0, baseline), pos(leader_term, baseline), 0), + JoinDecision::Clean, + "caught-up-but-stale-tail-term (frontier covers the baseline) is Clean, not a loop" + ); + // A frontier ABOVE the baseline (a benign replicated/uncommitted tail, + // nothing leader-acked) is likewise caught up → Clean. assert_eq!( decide_join( leader_term, - pos(0, snapshot_seq), + pos(0, baseline + 5_000), + pos(leader_term, baseline), + 0 + ), + JoinDecision::Clean, + "a frontier above the baseline with nothing leader-acked is caught up → Clean" + ); + + // GENUINELY BEHIND: tail_term 0 AND the frontier SHORT of the baseline → + // the node is missing committed entries → ReseedRequired. A real reseed + // installs and Fix 2's post-open seed synthesizes the term marker. + assert_eq!( + decide_join( + leader_term, + pos(0, baseline - 1), pos(leader_term, baseline), 0 ), JoinDecision::ReseedRequired, - "tail_term 0 after a checkpoint restore loses the (term, frontier) compare → \ - the false reseed loop" + "a frontier short of the baseline is genuinely behind → reseed" ); - // THE FIX: the durable term marker makes `own.tail_term == leader_term`, - // so the within-term clause returns Clean regardless of the frontier — the - // loop is broken. The catch-up pull streams any residual tail. + // A truthful term marker (Fix 2, after an install boot) is the within-term + // rejoin: tail_term == term → Clean regardless of frontier. assert_eq!( decide_join( leader_term, - pos(leader_term, snapshot_seq), + pos(leader_term, baseline), pos(leader_term, baseline), 0 ), @@ -1065,20 +1107,19 @@ mod tests { "a truthful artifact term marker classifies the reseeded shard Clean" ); - // Even if the leadership advanced a term between fetch and the join check, - // the node reseeds ONCE MORE against the newer term (own.tail_term < - // newer_term, nothing leader-acked) — bounded, self-healing, NOT a loop: - // the next install stamps the newer term and converges. - let newer_term = leader_term + 1; + // SAFETY preserved: an un-replicated ack=leader suffix above the baseline + // STILL quarantines, even with the frontier covering the baseline — the + // frontier-covers-baseline Clean arm never reaches a divergent node (the + // `leader_acked > prev_log.frontier` check quarantines it first). assert_eq!( decide_join( - newer_term, - pos(leader_term, snapshot_seq), - pos(newer_term, baseline), - 0 + leader_term, + pos(0, baseline + 10), + pos(leader_term, baseline), + baseline + 10 ), - JoinDecision::ReseedRequired, - "a leadership that advanced a term reseeds once more, then converges — not a loop" + JoinDecision::Quarantine, + "an un-replicated ack=leader suffix above the baseline still quarantines" ); } } diff --git a/tidal-server/src/cluster/node.rs b/tidal-server/src/cluster/node.rs index 8f79042..0db2216 100644 --- a/tidal-server/src/cluster/node.rs +++ b/tidal-server/src/cluster/node.rs @@ -2901,7 +2901,33 @@ impl ShardReplica { // `election_log_position` in the joined leader's numbering. db.replication_state() .advance(shard_of_region(leader), baseline + 1); - db.fold_term_marker(term, baseline + 1, leader.0); + // m12 reseed-loop-fix: durably repair a STALE WAL-tail term on a clean + // join — the checkpoint-restore empty-WAL anomaly where `wal_term_mark()` + // reads a term BELOW `term` while the replication frontier already covers + // the baseline. A CAUGHT-UP shard that never installs a snapshot (the + // leader answers needed=false) is not reached by the install-boot term- + // marker synthesis, so without this it would re-read tail_term 0 on every + // reboot and self-restart-loop through a futile reseed (observed live on + // tidaldb-2). Writing a real kind-3 record makes tail_term truthful across + // reboots, reaching decide_join's `tail_term == term` clean-join fast path. + // Guarded so it fires once per stale term — a steady clean re-join just + // folds in memory (cheap), never a WAL append per heartbeat. On append + // failure, fall back to the in-memory fold: decide_join's frontier-covers- + // baseline arm still classifies the node Clean, so the loop stays broken. + if db.wal_term_mark().0 < term { + if let Err(e) = db.append_term_marker(term, leader.0) { + tracing::warn!( + term, + leader = leader.0, + error = %e, + "reseed-loop-fix: durable term-marker repair failed on clean join; \ + folded in memory (tail-term is still truthful this boot)" + ); + db.fold_term_marker(term, baseline + 1, leader.0); + } + } else { + db.fold_term_marker(term, baseline + 1, leader.0); + } } // m12 election-divergence-fix: a CLEAN join proves this node is caught up // to (subsumed by) the new leadership, so it holds nothing un-replicated