diff --git a/crates/vtop-node/src/lease_agent.rs b/crates/vtop-node/src/lease_agent.rs index 2e72173..45ae5a1 100644 --- a/crates/vtop-node/src/lease_agent.rs +++ b/crates/vtop-node/src/lease_agent.rs @@ -286,6 +286,13 @@ struct Promoter { /// the boundary it is about to publish. node_uuid: Uuid, range_uuid: Uuid, + /// Set when a refusal was an ELIGIBILITY verdict — the quorum answered + /// and this node's log is the problem — rather than a transient quorum + /// miss. The agent turns it into a campaign hold-off: a refused + /// candidate that keeps winning the acquisition race starves the very + /// replica its own refusal named, because a suspended non-leader + /// receives no replication with which to become eligible. + stand_aside: bool, } impl Promoter { @@ -361,6 +368,33 @@ impl Promoter { // Also retryable in principle: probes are a snapshot, // and a stale follower answer can transiently place // the boundary above this node's own disk. + self.stand_aside = true; + self.suspended(fencing_epoch); + return false; + } + crate::promotion::Promotion::CandidateBehindVoters { + candidate_offset, + votes, + required, + most_complete, + } => { + tracing::warn!( + range = %self.range_uuid, + fencing_epoch, + candidate_offset, + votes, + required, + most_complete_node = %most_complete.0, + most_complete_offset = most_complete.1, + "refusing promotion: fewer than a majority of the fenced \ + replicas are at or below this node's offset (Raft §5.4.1); \ + letting the lease lapse so the more complete replica can \ + win the range" + ); + // Retryable for the same reason as LeaderBehind: the + // right fix is a different candidate, and suspending + // leaves the epoch grantable to it. + self.stand_aside = true; self.suspended(fencing_epoch); return false; } @@ -386,6 +420,12 @@ impl Promoter { self.publisher.suspend(fencing_epoch); self.verified_epoch = None; } + + /// Whether the last refusal was an eligibility verdict; reading clears + /// it, because one verdict funds one hold-off. + fn take_stand_aside(&mut self) -> bool { + std::mem::take(&mut self.stand_aside) + } } /// How the agent paces itself. @@ -545,6 +585,17 @@ pub struct LeaseAgent { range_uuid: Uuid, promoter: Promoter, state: LeaseState, + /// Rounds of the run loop during which this node will NOT campaign for + /// the lease, set when a promotion was refused on eligibility grounds + /// (LeaderBehind or the §5.4.1 vote check). A refused candidate that + /// keeps winning the acquisition race starves the replica its own + /// refusal named — it holds the lease it cannot serve, lets it lapse, + /// and wins again — while receiving no replication with which to become + /// eligible. Standing aside for a bounded window gives the eligible + /// replica uncontested acquisitions; BOUNDED, not until-another-holder, + /// because if the eligible replica is down someone must keep probing, + /// and the refusal repeating is the honest unavailability signal. + campaign_hold_off_rounds: u32, /// Local upper bound on how long the current hold may be trusted without /// hearing from metadata, in the same wall-clock the envelope carries. /// @@ -591,9 +642,11 @@ impl LeaseAgent { verified_epoch: None, node_uuid, range_uuid, + stand_aside: false, }, state: LeaseState::NotHeld, held_until_ms: None, + campaign_hold_off_rounds: 0, }) } @@ -787,6 +840,18 @@ impl LeaseAgent { if let LeaseState::Held { fencing_epoch } = self.state { self.publish_lost(fencing_epoch); } + // Standing aside after an eligibility refusal: campaigning + // now would only take the lease away from the replica the + // refusal named, hold it unserved, and lapse it again. + if self.campaign_hold_off_rounds > 0 { + self.campaign_hold_off_rounds -= 1; + tracing::debug!( + range = %self.range_uuid, + rounds_remaining = self.campaign_hold_off_rounds, + "standing aside from the lease race after an eligibility refusal" + ); + return Ok(self.config.poll_interval); + } match self.acquire(expected_range_generation).await? { Some(fencing_epoch) => { let granted_until = Some( @@ -824,6 +889,14 @@ impl LeaseAgent { // monotonic and idempotent, so repeating it every poll is // free. self.promoter.lost(fencing_epoch); + // A rival holding the range is the stand-aside's purpose + // ACHIEVED: the replica this node's refusal made way for (or + // any other eligible one) has the lease. Clearing the + // hold-off here means a much later failure of that holder is + // answered by an immediate campaign, not by serving out the + // residue of a wait that already did its job (review round + // four). + self.campaign_hold_off_rounds = 0; Ok(self.config.poll_interval) } LeaseDecision::RangeMissing => { @@ -881,6 +954,13 @@ impl LeaseAgent { async fn publish_held(&mut self, fencing_epoch: u64, held_until_ms: Option) -> bool { if !self.promoter.ensure(fencing_epoch).await { + if self.promoter.take_stand_aside() { + // Two lease lifetimes of poll rounds: enough for the replica + // the refusal named to see the lapse and win at least one + // uncontested acquisition, however the two agents' polls + // interleave. + self.campaign_hold_off_rounds = self.stand_aside_rounds(); + } self.state = LeaseState::NotHeld; self.held_until_ms = None; return false; @@ -890,6 +970,14 @@ impl LeaseAgent { true } + /// How many poll rounds an eligibility refusal sits out: two lease + /// lifetimes, expressed in this agent's own polling cadence. + fn stand_aside_rounds(&self) -> u32 { + let lease_ms = self.config.lease_duration.as_millis().max(1); + let poll_ms = self.config.poll_interval.as_millis().max(1); + (lease_ms.saturating_mul(2).div_ceil(poll_ms)).min(u128::from(u32::MAX)) as u32 + } + fn publish_lost(&mut self, fencing_epoch: u64) { tracing::warn!( range = %self.range_uuid, @@ -1153,6 +1241,7 @@ mod tests { publisher, probe, verified_epoch: None, + stand_aside: false, // Matches `at(1, ..)`: the tests' candidate is node 1. node_uuid: Uuid::from_u128(1), range_uuid: Uuid::from_u128(21), @@ -1255,6 +1344,41 @@ mod tests { ); } + /// An eligibility refusal — the quorum answered and this node's log is + /// the problem — requests a stand-aside, so the agent stops winning + /// acquisition races away from the replica the refusal named. A quorum + /// MISS does not: standing aside there would delay recovery for a + /// verdict nobody reached. + #[tokio::test] + async fn an_eligibility_refusal_requests_a_stand_aside_and_a_quorum_miss_does_not() { + let recorder = Arc::new(Recorder::default()); + // Node 1 answers 100 and holds the floor, but node 2 is ahead at + // 101: one vote of a required two — the §5.4.1 refusal. + let mut behind = promoter( + Arc::clone(&recorder) as Arc, + Some(fixed(vec![at(1, Some(100)), at(2, Some(101))], 3)), + ); + assert!(!behind.ensure(7).await); + assert!( + behind.take_stand_aside(), + "an eligibility verdict must request a stand-aside, or the refused candidate keeps winning the race away from the replica it named" + ); + assert!( + !behind.take_stand_aside(), + "one verdict funds one hold-off; reading clears it" + ); + + let mut miss = promoter( + Arc::clone(&recorder) as Arc, + Some(fixed(vec![at(1, Some(10)), at(2, None), at(3, None)], 3)), + ); + assert!(!miss.ensure(8).await); + assert!( + !miss.take_stand_aside(), + "a quorum miss is not an eligibility verdict; standing aside would delay recovery for nothing" + ); + } + /// The recovery half of the transient-refusal story, end to end against a /// real fencing view: quorum miss, then quorum back, and the broker must /// actually serve again at the SAME epoch. diff --git a/crates/vtop-node/src/promotion.rs b/crates/vtop-node/src/promotion.rs index 977b564..d8ca04d 100644 --- a/crates/vtop-node/src/promotion.rs +++ b/crates/vtop-node/src/promotion.rs @@ -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. One further gate applies: +//! 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. +//! 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, }, + /// 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 + // 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 + .values() + .filter(|offset| **offset <= candidate_offset) + .count(); + 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);