Skip to content

Commit 5899db7

Browse files
committed
fix(cluster): seed Calvin vote-completion count on every replica
Previously expected_participants was only seeded on the sequencer leader when a transaction was assigned, so a replica that later becomes leader via failover had no way to detect vote completeness for epochs applied before the failover. Seed the count deterministically from the replicated EpochBatch apply arm on all replicas, taking the max against any leader-side seed so ordering between the two paths is safe regardless of which runs first.
1 parent 48d30d0 commit 5899db7

2 files changed

Lines changed: 106 additions & 8 deletions

File tree

nodedb-cluster/src/calvin/completion.rs

Lines changed: 96 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,25 @@ impl CalvinCompletionRegistry {
294294
}
295295
}
296296

297+
/// Seed the expected participant count for `txn` deterministically from the
298+
/// replicated `SequencerEntry::EpochBatch` — this runs on every replica (not
299+
/// just the epoch's originating leader), so vote-completeness becomes
300+
/// detectable even on a replica that later becomes leader via failover and
301+
/// never observed the original `note_assigned` seeding.
302+
///
303+
/// Idempotent: takes the max with any existing seed, so ordering against
304+
/// `note_assigned` / `register_completion` (which also seed this field) is
305+
/// safe regardless of which one runs first. `expected == 0` is a harmless
306+
/// no-op (max with the existing value).
307+
pub fn seed_expected(&self, txn: TxnId, expected: usize) {
308+
let mut inner = self.inner.lock().unwrap_or_else(|p| p.into_inner());
309+
let entry = inner
310+
.completions
311+
.entry(txn)
312+
.or_insert_with(|| PendingCompletion::new(0));
313+
entry.expected_participants = entry.expected_participants.max(expected);
314+
}
315+
297316
/// Record one participant vshard's durable commit vote for `txn`, tallied
298317
/// from a replicated `SequencerEntry::Vote`.
299318
///
@@ -311,13 +330,16 @@ impl CalvinCompletionRegistry {
311330
entry.votes.insert(vshard, commit);
312331

313332
// On the transition to a complete tally, emit the aggregated verdict
314-
// exactly once. `expected_participants` is seeded on the sequencer
315-
// leader (the participant count arrives with the assignment); a replica
316-
// whose registry never received that seed keeps `expected == 0` and
317-
// never emits here — correct, because only the leader proposes the
318-
// verdict. Deterministic all-replica seeding for failover is a
319-
// follow-up. The `verdict_proposed` guard dedups a re-tally caused by a
320-
// re-proposed vote on retry.
333+
// exactly once. `expected_participants` is seeded deterministically on
334+
// every replica from the `EpochBatch` apply arm (see `seed_expected`),
335+
// as well as opportunistically by `note_assigned` / `register_completion`
336+
// on the leader/coordinator — so completeness is detectable here on any
337+
// replica, including one that becomes leader via failover after the
338+
// epoch was originally applied elsewhere. Only the leader's
339+
// `SequencerService` actually proposes the verdict from this signal
340+
// (leader-gated at the propose site); a follower computing and sending
341+
// the signal here is harmless. The `verdict_proposed` guard dedups a
342+
// re-tally caused by a re-proposed vote on retry.
321343
if entry.expected_participants > 0
322344
&& entry.votes.len() == entry.expected_participants
323345
&& !entry.verdict_proposed
@@ -646,6 +668,73 @@ mod tests {
646668
assert!(rx.try_recv().is_err());
647669
}
648670

671+
#[tokio::test]
672+
async fn seed_expected_is_idempotent_max_and_enables_verdict_without_note_assigned() {
673+
// Mirrors complete_all_true_tally_emits_commit_verdict_once, but seeds
674+
// expected_participants via seed_expected (the EpochBatch apply-arm path
675+
// that runs on every replica) instead of the leader-only note_assigned.
676+
let (tx, mut rx) = mpsc::channel(8);
677+
let reg = CalvinCompletionRegistry::new(tx);
678+
let txn = TxnId::new(40, 0);
679+
680+
// Seeding a smaller value after a larger one must not shrink the count.
681+
reg.seed_expected(txn, 2);
682+
reg.seed_expected(txn, 1);
683+
684+
reg.note_vote(txn, 1, true);
685+
assert!(
686+
rx.try_recv().is_err(),
687+
"only 1 of 2 expected votes in; must not emit yet"
688+
);
689+
690+
reg.note_vote(txn, 2, true);
691+
assert_eq!(
692+
rx.try_recv().expect("verdict emitted"),
693+
(txn, true),
694+
"seed_expected alone (no note_assigned) must make completeness detectable"
695+
);
696+
}
697+
698+
#[tokio::test]
699+
async fn seed_expected_and_note_assigned_take_the_max_regardless_of_order() {
700+
let (tx, mut rx) = mpsc::channel(8);
701+
let reg = CalvinCompletionRegistry::new(tx);
702+
703+
// seed_expected first (larger), then note_assigned with a smaller count:
704+
// the max (3) must win, so 2 votes must NOT be enough to emit a verdict.
705+
let txn_a = TxnId::new(41, 0);
706+
reg.seed_expected(txn_a, 3);
707+
reg.note_assigned(1, txn_a, 1);
708+
reg.note_vote(txn_a, 1, true);
709+
reg.note_vote(txn_a, 2, true);
710+
assert!(
711+
rx.try_recv().is_err(),
712+
"expected_participants must be max(3, 1) = 3; 2 votes is not complete"
713+
);
714+
reg.note_vote(txn_a, 3, true);
715+
assert_eq!(
716+
rx.try_recv().expect("verdict emitted at the 3rd vote"),
717+
(txn_a, true)
718+
);
719+
720+
// note_assigned first (smaller), then seed_expected with a larger count:
721+
// same invariant, opposite call order.
722+
let txn_b = TxnId::new(41, 1);
723+
reg.note_assigned(2, txn_b, 1);
724+
reg.seed_expected(txn_b, 3);
725+
reg.note_vote(txn_b, 1, true);
726+
reg.note_vote(txn_b, 2, true);
727+
assert!(
728+
rx.try_recv().is_err(),
729+
"expected_participants must be max(1, 3) = 3, not the smaller seed"
730+
);
731+
reg.note_vote(txn_b, 3, true);
732+
assert_eq!(
733+
rx.try_recv().expect("verdict emitted at the 3rd vote"),
734+
(txn_b, true)
735+
);
736+
}
737+
649738
#[tokio::test]
650739
async fn mismatch_takes_precedence_over_pending_acks() {
651740
let reg = CalvinCompletionRegistry::new_detached();

nodedb-cluster/src/calvin/sequencer/state_machine.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,18 @@ impl SequencerStateMachine {
262262
// epoch targeting a given vShard is stamped with the same count.
263263
let mut vshard_txn_counts: HashMap<u32, u32> = HashMap::new();
264264
for txn in &batch.txns {
265-
for vshard_id in txn.tx_class.participating_vshards() {
265+
let participating = txn.tx_class.participating_vshards();
266+
for vshard_id in participating {
266267
*vshard_txn_counts.entry(vshard_id.as_u32()).or_insert(0) += 1;
267268
}
269+
// Seed the expected vote-participant count deterministically on
270+
// EVERY replica (not just the epoch's originating leader), so a
271+
// post-failover sequencer leader can still detect vote
272+
// completeness and aggregate the verdict.
273+
self.completion_registry.seed_expected(
274+
crate::calvin::TxnId::new(batch.epoch, txn.position),
275+
participating.len(),
276+
);
268277
}
269278

270279
for txn in &batch.txns {

0 commit comments

Comments
 (0)