node: promotion enforces Raft's election restriction — a majority must vouch for the candidate (#240) #342
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,11 +65,17 @@ | |
| //! | ||
| //! This is the same arithmetic the replication path already uses to advance the | ||
| //! watermark during steady-state produce; promotion applies it once, from a | ||
| //! standing start, to state written by someone else. Two further gates apply: | ||
| //! the candidate must itself hold the boundary the quorum proved | ||
| //! ([`Promotion::LeaderBehind`]) — a leader behind the boundary would publish | ||
| //! a high-water mark covering offsets its own log does not contain, and the | ||
| //! produce fast path would then acknowledge fresh writes into them — and a | ||
| //! majority of the fenced replicas must be at or below the candidate's own | ||
| //! offset ([`Promotion::CandidateBehindVoters`]), which is Raft's election | ||
| //! restriction (§5.4.1): the floor alone can sit below a record acknowledged | ||
| //! on a quorum whose survivors straddle the candidate, and a candidate that | ||
| //! majority would refuse the vote used to promote anyway and let | ||
| //! reconciliation truncate the acknowledged record away (#240). | ||
| //! | ||
| //! # Why an inherited watermark is never lowered | ||
| //! | ||
|
|
@@ -129,6 +135,33 @@ pub enum Promotion { | |
| /// which is refused for the same reason. | ||
| leader_committed_offset: Option<u64>, | ||
| }, | ||
| /// The candidate holds the proven floor, but fewer than a majority of the | ||
| /// fenced replicas are at or below its own offset — Raft's election | ||
| /// restriction (§5.4.1), in its per-voter form. | ||
| /// | ||
| /// The floor alone is not enough: the k-th largest can sit BELOW a record | ||
| /// that was acknowledged on a quorum whose survivors now straddle the | ||
| /// candidate — a candidate at 100 counting a fenced replica at 101 passes | ||
| /// the floor check (the floor computes to 100) and would then publish a | ||
| /// boundary under which reconciliation truncates the acknowledged record | ||
| /// away. In Raft terms, a replica whose log is ahead of the candidate | ||
| /// would refuse it the vote; counting it toward the quorum anyway is how | ||
| /// promotion used to conclude an entry was uncommitted merely because | ||
| /// the candidate had not seen it. Deliberately per-voter rather than | ||
| /// candidate-must-hold-the-maximum: a majority at or below the candidate | ||
| /// is exactly §5.4.1's guarantee (any acknowledged record's quorum | ||
| /// intersects every vote quorum), and the stricter form would refuse a | ||
| /// legitimate leader over a record that was never acknowledged. | ||
| CandidateBehindVoters { | ||
| /// The candidate's own offset. | ||
| candidate_offset: u64, | ||
| /// How many fenced replicas are at or below the candidate. | ||
| votes: usize, | ||
| required: usize, | ||
| /// The most complete replica observed — the one an operator or the | ||
| /// lease agent should let win the range instead. | ||
| most_complete: (Uuid, u64), | ||
| }, | ||
| } | ||
|
|
||
| /// Majority of a replica set, including the leader itself. | ||
|
|
@@ -199,6 +232,60 @@ pub fn establish(probes: &[ReplicaProbe], replication_factor: usize, leader_id: | |
| leader_committed_offset, | ||
| }; | ||
| } | ||
| let candidate_offset = | ||
| leader_committed_offset.expect("a candidate below the floor returned above"); | ||
| // Raft's election restriction (§5.4.1), per voter: only replicas at or | ||
| // below the candidate's own offset would have granted it the vote, and a | ||
| // majority of grants is what makes the promotion safe — any record | ||
| // acknowledged on a quorum lives on at least one member of every | ||
| // majority, so a candidate a majority can vouch for holds every | ||
| // acknowledged record. The floor check above cannot substitute: the | ||
| // k-th largest can sit below an acknowledged record whose surviving | ||
| // holders straddle the candidate. | ||
| // | ||
| // These are POST-RECONCILIATION offsets — the fence reconciles before it | ||
| // answers (#263) — and that is correct, not a leak in the restriction. | ||
| // A same-epoch prefix relationship is not divergence (`compare_lineage` | ||
| // answers Agreed and reconciliation touches nothing), so the replica | ||
| // ahead of the candidate still answers with its full offset and refuses | ||
| // the candidate here. Truncation before answering happens only on | ||
|
Comment on lines
+246
to
+251
There was a problem hiding this comment.
When the surviving voter and candidate have genuinely divergent epoch histories, this post-reconciliation vote can still delete an acknowledged record: Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. You're right that the DivergesAt path truncates before the vote is read, and that its guard only knows a possibly-stale HWM — verified. What makes it safe is not the guard but reachability, and 3fc7d07 commits the argument into the module doc: a record acknowledged on a quorum intersects every later epoch's fence-majority, so with this gate in force at EVERY promotion, the intersecting voter refuses any candidate whose log lacks the record. By induction, every granted epoch's leader holds every previously acknowledged record, every subsequent leadership line contains them, and any suffix a voter loses to DivergesAt was written under a superseded line and never acknowledged. Divergence below an acknowledged record therefore requires a promotion that already violated the restriction — the pre-#263 bugs, or a history predating this gate — which is exactly what the below-HWM guard remains the loud last resort for. Your proposed reordering (a fence that votes before it reconciles) would make the property LOCAL rather than inductive, and it is the right shape for that — but it is a replica-plane protocol change, and it belongs to #240's remaining design conversation rather than riding on this bounded slice; the doc says so explicitly, and I'll record it on the issue so it isn't lost. |
||
| // GENUINE lineage divergence, and there a pre-truncation offset would be | ||
| // the wrong vote input: offsets are only comparable within an agreed | ||
| // lineage, and counting bytes from a contradicted leadership line as | ||
| // log-completeness is the exact mistake epoch qualification (#258) | ||
| // exists to prevent. | ||
| // | ||
| // Can a divergent reconciliation delete an ACKNOWLEDGED record before | ||
| // the vote is read? Only if some past promotion already violated this | ||
| // restriction. The argument is inductive: a record acknowledged on a | ||
| // quorum intersects every later epoch's fence-majority, so with this | ||
| // gate in force at every promotion, the intersecting voter refuses any | ||
| // candidate whose log lacks the record — meaning every granted epoch's | ||
| // leader holds every previously acknowledged record, every subsequent | ||
| // leadership line contains them, and any suffix a voter loses to | ||
| // `DivergesAt` was written under a superseded line and never | ||
| // acknowledged. The truncation-below-HWM guard stays as the last | ||
| // resort for histories that predate the restriction; making the | ||
| // property local rather than inductive — a fence that votes before it | ||
| // reconciles — is a protocol reordering that belongs to #240's | ||
| // remaining design conversation, not to this gate. | ||
| let votes = answered | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| .values() | ||
| .filter(|offset| **offset <= candidate_offset) | ||
| .count(); | ||
|
Comment on lines
+272
to
+275
There was a problem hiding this comment.
In the real Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Verified against the code, and the premise does not hold for the motivating scenario. The fence does reconcile before answering (#263, confirmed at replication/mod.rs — adopt, reconcile, then read offsets) — but reconciliation cannot truncate B in the A-acks-on-{A,B}-and-dies case, because B and C hold IDENTICAL epoch vectors there: |
||
| if votes < required { | ||
| let most_complete = answered | ||
| .iter() | ||
| .max_by_key(|(_, offset)| **offset) | ||
| .map(|(node, offset)| (*node, *offset)) | ||
| .expect("a quorum answered"); | ||
| return Promotion::CandidateBehindVoters { | ||
| candidate_offset, | ||
| votes, | ||
| required, | ||
| most_complete, | ||
| }; | ||
| } | ||
| Promotion::Established { | ||
| committed_offset, | ||
| answered, | ||
|
|
@@ -216,6 +303,76 @@ mod tests { | |
| } | ||
| } | ||
|
|
||
| /// REGRESSION shape, from #240's #265 postmortem: A acknowledges a | ||
| /// record on {A, B} and dies; B answers 101, candidate C answers 100. | ||
| /// The floor computes to 100 and C holds it, so every pre-§5.4.1 check | ||
| /// passed — and B's acknowledged record was then reconciled away under | ||
| /// C's boundary. The election restriction refuses C: only one fenced | ||
| /// replica is at or below C's offset, and one is not a majority. | ||
| #[test] | ||
| fn a_candidate_a_fenced_replica_would_refuse_the_vote_is_not_promoted() { | ||
| let outcome = establish( | ||
| &[probe(2, Some(101)), probe(3, Some(100))], | ||
| 3, | ||
| Uuid::from_u128(3), | ||
| ); | ||
| assert_eq!( | ||
| outcome, | ||
| Promotion::CandidateBehindVoters { | ||
| candidate_offset: 100, | ||
| votes: 1, | ||
| required: 2, | ||
| most_complete: (Uuid::from_u128(2), 101), | ||
| }, | ||
| "a candidate counting a fenced replica ahead of its own log used to promote at a floor below an acknowledged record; the replica ahead is the one that must win" | ||
| ); | ||
| } | ||
|
|
||
| /// The remedy the refusal names: the more complete replica promotes over | ||
| /// the identical probe set. | ||
| #[test] | ||
| fn the_replica_the_refusal_names_promotes_over_the_same_probes() { | ||
| let outcome = establish( | ||
| &[probe(2, Some(101)), probe(3, Some(100))], | ||
| 3, | ||
| Uuid::from_u128(2), | ||
| ); | ||
| match outcome { | ||
| Promotion::Established { | ||
| committed_offset, .. | ||
| } => assert_eq!( | ||
| committed_offset, 100, | ||
| "the floor is still what the quorum can vouch for; the record above it is protected by the candidate holding it, not by the floor" | ||
| ), | ||
| other => panic!("the most complete replica must promote: {other:?}"), | ||
| } | ||
| } | ||
|
|
||
| /// Deliberately Raft's PER-VOTER form, not candidate-holds-the-maximum: | ||
| /// at RF 5 a candidate with a majority at or below it may lead even | ||
| /// though one fenced replica is ahead — the record making that replica | ||
| /// ahead was never acknowledged (its quorum would have needed three), | ||
| /// and refusing here would trade availability for nothing. | ||
| #[test] | ||
| fn a_majority_at_or_below_the_candidate_promotes_despite_a_more_complete_minority() { | ||
| let outcome = establish( | ||
| &[ | ||
| probe(2, Some(101)), | ||
| probe(3, Some(100)), | ||
| probe(4, Some(100)), | ||
| probe(5, Some(100)), | ||
| ], | ||
| 5, | ||
| Uuid::from_u128(3), | ||
| ); | ||
| match outcome { | ||
| Promotion::Established { | ||
| committed_offset, .. | ||
| } => assert_eq!(committed_offset, 100), | ||
| other => panic!("a candidate with majority votes must promote: {other:?}"), | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn a_majority_needs_more_than_half_even_at_even_sizes() { | ||
| assert_eq!(majority(1), 1); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When this stable refusal occurs because a majority is genuinely ahead, suspending only stops renewal; after the lease expires, the same node immediately returns to the ordinary acquisition race and can repeatedly reacquire before the
most_completereplica. Since a suspended candidate receives no replication with which to become eligible, consistent poll timing or lower metadata latency can leave the range unavailable indefinitely despite a valid candidate being online. Suppress this node's reacquisition until another holder/epoch is observed, or otherwise give the identified eligible replica an opportunity to acquire.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Real, and fixed in cbd1c6c — with the note that the starvation was pre-existing for LeaderBehind and this variant merely made it visible, so the fix covers both. An eligibility refusal (either behind-variant, NOT a quorum miss — that is a verdict nobody reached) now requests a stand-aside: the agent skips the campaign for two lease lifetimes of poll rounds, giving the replica the refusal named uncontested acquisitions however the two agents' polls interleave. Deliberately BOUNDED rather than suppressed-until-another-holder: if the eligible replica is down, someone must keep probing, and the refusal repeating on a duty cycle is the honest unavailability signal — a suppression waiting for a holder that never comes would wedge the range on a second failure. Pinned by an_eligibility_refusal_requests_a_stand_aside_and_a_quorum_miss_does_not.