node: promotion enforces Raft's election restriction — a majority must vouch for the candidate (#240) - #342
Conversation
…t vouch for the candidate (#240) The #265 postmortem left one safety question open, stated rather than closed: whether the fence plus the acknowledged-records bound fully substitute for Raft §5.4.1, given that the lease is granted with no log-completeness condition and LeaderBehind only refuses a candidate behind the proven floor — which can itself sit below an acknowledged record. They do not substitute, and the postmortem's own replay shows it: A acknowledges on {A,B} and dies; B answers 101, candidate C answers 100; the k-th largest floor computes to 100, C holds it, every existing gate passes, and B's acknowledged record is reconciled away under C's boundary. establish now applies the restriction in Raft's PER-VOTER form: only replicas at or below the candidate's own offset would have granted it the vote, and a majority of grants is what makes promotion safe — any acknowledged record's quorum intersects every vote quorum, so a candidate a majority can vouch for holds every acknowledged record. Deliberately 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, because the record making it ahead was never acknowledged, and the stricter form would trade availability for nothing. The refusal names the most complete replica — the one the lease should go to — and the lease agent suspends rather than demotes, exactly as LeaderBehind does, so the epoch stays grantable to that replica. The hazard replay is pinned as a test, alongside its remedy (the named replica promotes over the identical probes) and the availability guarantee. Scenario 09 passes live — its promotion of the most caught-up follower is precisely the candidate the new gate blesses. Item 5 of #240 — the signed transition record — remains open by design: it is evidence, not safety, and the ROADMAP frames it as its own design conversation.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc75b206db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let votes = answered | ||
| .values() | ||
| .filter(|offset| **offset <= candidate_offset) | ||
| .count(); |
There was a problem hiding this comment.
In the real ReplicaPlaneProbe::probe path, these offsets are not the voters' pre-election log positions: InProcessFollower::fence calls reconcile_with before reading and returning local_committed_offset. In the motivating case where A acknowledged offset 100 on {A, B} but died before propagating the high-water mark to B, candidate C's fence can therefore truncate B from 101 to 100 because B does not know that record was acknowledged; this filter then counts both B and C as votes and returns Established after the acknowledged record has already been deleted. The election restriction must evaluate the follower's position before candidate-driven reconciliation can alter it, or the follower must explicitly refuse/truncate only after voting.
Useful? React with 👍 / 👎.
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: compare_lineage explicitly treats a same-epoch prefix relationship as NOT divergence ('the shorter replica has simply recorded less, and nothing here proves anything about the records beyond its last entry'), answers Agreed, and reconcile_with returns without touching the log. B answers 101 untruncated, and the new gate refuses C — the pinned test models exactly the post-fence numbers that path produces. Where truncation-before-answering DOES happen — genuine lineage divergence — 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; Raft's own (term,index) comparison encodes the same judgment. The residual interplay — an acknowledged record above a stale HWM at a genuinely diverged position — is the #264/#266 HWM-propagation arc, guarded by TruncationBelowAcknowledged failing the fence loudly, and orthogonal to the election restriction. The rationale is now in the restriction's comment (12a7a4a) so the next reader doesn't have to re-derive it.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…e point Review round one (Codex + cubic, same P1): the concern was that the fence reconciles before it answers, so the replica ahead of the candidate could be truncated down before its vote is counted — passing the restriction in the exact scenario it exists to refuse. Verified against the code, the premise does not hold for that scenario: a same-epoch prefix relationship is not divergence — compare_lineage answers Agreed and reconciliation touches nothing — so the replica ahead answers with its full offset and the candidate is refused. And where truncation before answering DOES happen, genuine lineage divergence, 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 mistake epoch qualification exists to prevent. The rationale now lives in the restriction's comment. The warn line's embedded indentation is fixed alongside (cubic P3).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12a7a4a346
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // 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 |
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: InProcessFollower::fence maps Lineage::DivergesAt to truncate_to before reading the returned offset, and the truncation guard only knows the follower's possibly stale high-water mark. Thus, if the old leader acknowledged offset 100 on itself and this voter but died before propagating the HWM, a divergent candidate can truncate the voter to 100, after which both replicas satisfy this predicate and establish a quorum despite the acknowledged record being gone. The newly added rationale only establishes that a prefix relationship is not truncated; the explicit DivergesAt path is fresh evidence that the destructive ordering remains. Divergent replicas must evaluate log freshness before candidate-driven reconciliation, or any replica truncated during probing must not grant the candidate a vote.
Useful? React with 👍 / 👎.
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.
…cord — by induction Review round two (Codex): the DivergesAt path does truncate before the vote is read, and its guard only knows a possibly-stale high-water mark. What makes that safe is not the guard — it is that the hazard state is unreachable through restricted promotions. A record acknowledged on a quorum intersects every later epoch's fence-majority, so with this gate at every promotion the intersecting voter refuses any candidate whose log lacks the record; every granted epoch's leader therefore 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 requires a promotion that already violated the restriction — the pre-#263 bugs, or a history predating this gate — and the below-HWM guard remains the last resort for exactly those. The local (non-inductive) form — a fence that votes before it reconciles — is a protocol reordering for #240's remaining design conversation, stated in the doc rather than smuggled into this slice.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fc7d07efb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
| // Retryable for the same reason as LeaderBehind: the | ||
| // right fix is a different candidate, and suspending | ||
| // leaves the epoch grantable to it. |
There was a problem hiding this comment.
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_complete replica. 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.
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.
…nnot serve Review round three (Codex P2), verified real — and pre-existing for LeaderBehind, which the new variant merely made visible: suspend only stops renewal, so a refused candidate re-enters the acquisition race, can keep beating the replica its own refusal named to the CAS, holds a lease it cannot serve, lapses it, and wins again — while receiving no replication with which to become eligible. Consistent poll timing can starve an eligible, online replica indefinitely. An eligibility refusal — LeaderBehind or the §5.4.1 vote check, NOT a quorum miss, which is a verdict nobody reached — now requests a stand-aside: the agent skips the campaign for two lease lifetimes of poll rounds, giving the named replica uncontested acquisitions however the two agents' polls interleave. BOUNDED, deliberately, not until-another-holder-is-observed: 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 that waits for a holder that never comes would wedge the range on a second failure. Pinned: the verdict requests the stand-aside and a quorum miss does not; one verdict funds one hold-off.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbd1c6c500
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self.campaign_hold_off_rounds > 0 { | ||
| self.campaign_hold_off_rounds -= 1; |
There was a problem hiding this comment.
When the eligible replica successfully acquires during this hold-off, subsequent Wait rounds leave the counter untouched because it is decremented only in Acquire. If that holder later releases or fails—even long afterward—this node still skips the next two lease lifetimes of acquisition rounds (30 seconds with the defaults), unnecessarily delaying an otherwise immediate failover. Clear the hold-off once a different holder/epoch is observed, since the stand-aside has then achieved its purpose.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid — fixed in the pushed commit: the Wait arm (exactly where this node observes a rival holding the range) now clears the hold-off, since a rival's acquisition is the stand-aside's purpose achieved. A later failure of that holder is answered by an immediate campaign; if this node is still behind at that point, the refusal and a fresh hold-off will say so.
…chieved Review round four (Codex P2): the hold-off decremented only in Acquire rounds, so a node that stood aside successfully — the eligible replica took the lease — kept the residual counter through every Wait round, and a failure of that holder even long afterward was answered by serving out up to two lease lifetimes of leftover wait before campaigning. The Wait arm, which is exactly where this node observes a rival holding the range, now clears the hold-off: the stand-aside exists to let an eligible replica win, and once one has, an immediate campaign on the next vacancy is the correct posture — if this node is still behind, the refusal and a fresh hold-off will say so.
1 participant
Part (a) of #240's remainder — the §5.4.1 election restriction. Does NOT close the issue: item 5 (the signed transition record) remains open by design — it is evidence, not safety, and the ROADMAP frames it as its own design conversation (the #265 lesson: design first).
The open question, answered in the negative
The #265 postmortem left one safety question stated rather than closed: "Whether the fence plus the acknowledged-records bound fully substitute for the election restriction is the open question." They do not, and the postmortem's own replay shows it: A acknowledges a record on {A, B} and dies; B answers 101, candidate C answers 100. The k-th-largest floor computes to 100, C holds it, every existing gate passes — and B's acknowledged record is reconciled away under C's published boundary. (#266 narrowed this by making HWM updates non-droppable, but a leader can die before propagation; the follower-side bound cannot be made reliable against death.)
The restriction, in Raft's per-voter form
establishnow requires a majority of the fenced, answering replicas to be at or below the candidate's own offset — each such replica is one that would have granted the vote. Why this and not candidate-holds-the-maximum:votes: 1, required: 2), naming B as the replica that must win.The new
Promotion::CandidateBehindVotersrefusal carries the vote arithmetic and the most complete replica — the remedy pointer. The lease agent handles it exactly asLeaderBehind: suspend, not demote, so the epoch stays grantable to the replica the refusal names.Tests
a_candidate_a_fenced_replica_would_refuse_the_vote_is_not_promoted— the node,broker: prove a boundary under the new epoch before publishing it (#240) #265 hazard replay, refused (REGRESSION-shape doc).the_replica_the_refusal_names_promotes_over_the_same_probes— the remedy works on identical probes.a_majority_at_or_below_the_candidate_promotes_despite_a_more_complete_minority— the per-voter form's availability guarantee at RF 5.63 vtop-node tests, clippy
-D warnings, fmt — all green.Summary by cubic
Enforces Raft §5.4.1 during promotion and adds a stand-aside policy after eligibility refusals. This prevents truncating acknowledged records and lets the most complete replica win the lease (part of #240); the hold-off now clears as soon as another replica holds the range.
establish: require a majority of fenced replicas at or below the candidate’s offset.Promotion::CandidateBehindVoterswithvotes,required, andmost_complete; votes use post-reconciliation offsets; docs explain why that’s correct and whyDivergesAtcannot delete acknowledged records (by induction). The below-HWM guard remains for pre-restriction histories. The lease agent stands aside for two lease lifetimes on this andLeaderBehind; quorum misses do not trigger the hold-off; one verdict funds one hold-off; the hold-off clears when another replica is observed holding the lease.Written for commit a29ec1c. Summary will update on new commits.