Skip to content

Commit fe0e16c

Browse files
committed
fix(cluster): hold down sequencer-log compaction below armed catch-up
A Calvin scheduler that misses a fan-out (channel full/closed, or it subscribed after the sequencer already committed epochs for its vShard) recovers by replaying the sequencer log from an armed catch-up index. Compaction could previously advance past that index, permanently losing the only durable copy of a cross-shard txn on that replica. Switch catch-up tracking from take-then-clear to peek/confirm (peek_catch_up_from / clear_catch_up_up_to) so a replay that cannot complete in one drain tick stays armed instead of being silently dropped, add min_catch_up_from() to floor the sequencer-group compactor below the lowest outstanding miss, and arm a spawn-time catch-up from the earliest retained index when a scheduler first subscribes to a vShard.
1 parent a724d31 commit fe0e16c

8 files changed

Lines changed: 247 additions & 27 deletions

File tree

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,62 @@ impl SequencerStateMachine {
188188
map.remove(&vshard)
189189
}
190190

191+
/// Arm a catch-up for `vshard` from `index` (min-collapse), so the
192+
/// scheduler-side drain replays committed sequencer entries from there.
193+
///
194+
/// Called when a scheduler subscribes for a vShard: the sequencer may have
195+
/// already committed (and fanned out to a then-absent sender — silently
196+
/// skipped) epochs for this vShard before the scheduler existed. A fresh
197+
/// node has nothing durably applied to rebuild from, so it would otherwise
198+
/// consider itself caught up and never replay those txns. Arming from the
199+
/// first available committed index makes the drain replay every committed
200+
/// entry for this vShard applied before subscription (idempotent: the
201+
/// scheduler's in-flight guard and Reserve/Release no-ops absorb re-apply).
202+
pub fn arm_catch_up_from(&self, vshard: u32, index: u64) {
203+
self.record_catch_up(vshard, index);
204+
}
205+
206+
/// Read (WITHOUT removing) the catch-up-from Raft index for `vshard`.
207+
///
208+
/// The scheduler drain peeks rather than takes so a replay that cannot
209+
/// complete this tick (committed index not yet known, transient log-read
210+
/// fault) leaves the entry armed for the next tick instead of silently
211+
/// dropping it — the loss the old take-then-early-return had.
212+
pub fn peek_catch_up_from(&self, vshard: u32) -> Option<u64> {
213+
let map = self.catch_up_from.lock().unwrap_or_else(|p| p.into_inner());
214+
map.get(&vshard).copied()
215+
}
216+
217+
/// Clear `vshard`'s catch-up entry only if its recorded index is `<= up_to`.
218+
///
219+
/// Called after a successful replay of `lo ..= up_to`: the recorded miss is
220+
/// now covered, so clear it — unless a concurrent drop has already lowered
221+
/// the entry to an index the just-finished replay did not cover (only
222+
/// possible for an index `<= up_to` given min-collapse, hence the guard is a
223+
/// belt-and-braces no-op in that case). A newer drop recorded at an index
224+
/// `> up_to` is preserved for the next drain.
225+
pub fn clear_catch_up_up_to(&self, vshard: u32, up_to: u64) {
226+
let mut map = self.catch_up_from.lock().unwrap_or_else(|p| p.into_inner());
227+
if let Some(&idx) = map.get(&vshard)
228+
&& idx <= up_to
229+
{
230+
map.remove(&vshard);
231+
}
232+
}
233+
234+
/// The smallest armed catch-up index across ALL vShards, or `None` when no
235+
/// catch-up is pending.
236+
///
237+
/// The sequencer-group log compactor floors its compaction index at this
238+
/// value so a dropped/undelivered fan-out is always replayable from the
239+
/// retained log — the hold-down the scheduler-side drain's `LogCompacted`
240+
/// arm depends on. Only hosted vShards ever arm a catch-up, so this never
241+
/// pins compaction on a vShard this node does not serve.
242+
pub fn min_catch_up_from(&self) -> Option<u64> {
243+
let map = self.catch_up_from.lock().unwrap_or_else(|p| p.into_inner());
244+
map.values().copied().min()
245+
}
246+
191247
/// Apply a committed Raft log entry.
192248
///
193249
/// Decodes the `SequencerEntry`, checks epoch monotonicity, fans out to
@@ -721,6 +777,66 @@ mod tests {
721777
assert_eq!(sm.take_catch_up_from(va), None);
722778
}
723779

780+
/// PEEK must not consume: the scheduler drain reads the armed index, and
781+
/// only clears it after a confirmed replay. A take-then-early-return (the
782+
/// old shape) silently lost the miss when the replay could not complete.
783+
#[test]
784+
fn peek_catch_up_from_does_not_consume() {
785+
let (batch, va, _vb) = make_batch_with_two_vshards();
786+
let (tx_a, _rx_a) = mpsc::channel(1);
787+
let _ = tx_a.try_send(SchedulerInput::Txn(batch.txns[0].clone()));
788+
let mut senders = HashMap::new();
789+
senders.insert(va, tx_a);
790+
let mut sm = SequencerStateMachine::new(senders, CalvinCompletionRegistry::new_detached());
791+
792+
sm.apply(9, &encode_entry(&SequencerEntry::EpochBatch { batch }));
793+
794+
// Repeated peeks keep returning the same armed index.
795+
assert_eq!(sm.peek_catch_up_from(va), Some(9));
796+
assert_eq!(sm.peek_catch_up_from(va), Some(9));
797+
}
798+
799+
/// Clearing is bounded by the replayed upper bound: a miss covered by the
800+
/// replay is cleared, one recorded ABOVE it survives for the next drain.
801+
#[test]
802+
fn clear_catch_up_up_to_respects_replayed_upper_bound() {
803+
let senders = HashMap::new();
804+
let sm = SequencerStateMachine::new(senders, CalvinCompletionRegistry::new_detached());
805+
let v = 42u32;
806+
807+
// Armed at 5, replay covered through 10 → cleared.
808+
sm.arm_catch_up_from(v, 5);
809+
sm.clear_catch_up_up_to(v, 10);
810+
assert_eq!(sm.peek_catch_up_from(v), None);
811+
812+
// Armed at 20, replay only covered through 10 → still armed.
813+
sm.arm_catch_up_from(v, 20);
814+
sm.clear_catch_up_up_to(v, 10);
815+
assert_eq!(sm.peek_catch_up_from(v), Some(20));
816+
}
817+
818+
/// The sequencer-log compaction hold-down floors on the LOWEST armed index
819+
/// across all vShards, so no replica's replay range is compacted away.
820+
#[test]
821+
fn min_catch_up_from_is_lowest_armed_index_across_vshards() {
822+
let senders = HashMap::new();
823+
let sm = SequencerStateMachine::new(senders, CalvinCompletionRegistry::new_detached());
824+
assert_eq!(sm.min_catch_up_from(), None);
825+
826+
sm.arm_catch_up_from(1, 30);
827+
sm.arm_catch_up_from(2, 12);
828+
sm.arm_catch_up_from(3, 25);
829+
assert_eq!(sm.min_catch_up_from(), Some(12));
830+
831+
// Draining the lowest lifts the floor to the next outstanding miss.
832+
sm.clear_catch_up_up_to(2, 12);
833+
assert_eq!(sm.min_catch_up_from(), Some(25));
834+
835+
sm.clear_catch_up_up_to(1, 30);
836+
sm.clear_catch_up_up_to(3, 25);
837+
assert_eq!(sm.min_catch_up_from(), None);
838+
}
839+
724840
#[test]
725841
fn catch_up_from_records_dropped_index_on_closed_channel() {
726842
let (batch, va, vb) = make_batch_with_two_vshards();

nodedb-cluster/src/multi_raft/core.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,18 @@ impl MultiRaft {
464464
Ok(entries.to_vec())
465465
}
466466

467+
/// The lowest committed index still available in `group_id`'s retained log
468+
/// (`snapshot_index + 1`), or `None` when the group is absent on this node.
469+
///
470+
/// Used to arm a Calvin scheduler catch-up from the earliest replayable
471+
/// sequencer index so its drain reads exactly the retained log and never
472+
/// faults on a compacted range.
473+
pub fn first_available_index(&self, group_id: u64) -> Option<u64> {
474+
self.groups
475+
.get(&group_id)
476+
.map(|n| n.first_available_index())
477+
}
478+
467479
/// Auto-compact a group's log if its configured threshold has been
468480
/// reached, given the DATA-PLANE applied watermark `applied_index`.
469481
///

nodedb-raft/src/node/core.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,15 @@ impl<S: LogStorage> RaftNode<S> {
215215
self.durable_applied
216216
}
217217

218+
/// The lowest log index still available in the retained (post-compaction)
219+
/// log — `snapshot_index + 1`. A committed-entry read below this yields
220+
/// [`RaftError::LogCompacted`]. Used to arm a Calvin scheduler catch-up from
221+
/// the earliest replayable index so the drain never faults on a compacted
222+
/// range.
223+
pub fn first_available_index(&self) -> u64 {
224+
self.log.snapshot_index() + 1
225+
}
226+
218227
/// Persist `index` as the durable applied floor.
219228
///
220229
/// The caller MUST only pass an index whose state-machine effects are

nodedb/src/control/cluster/calvin/scheduler/driver/core/catch_up.rs

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -35,24 +35,33 @@ impl Scheduler {
3535
/// it; this drain takes them strictly one-at-a-time (SM → release → MultiRaft
3636
/// → release → SM → release), so the two paths can never form a lock cycle.
3737
pub(in crate::control::cluster::calvin::scheduler::driver::core) fn drain_catch_up(&mut self) {
38-
// 1. SM-lock scope: TAKE the earliest dropped index for this vShard.
39-
// `None` (the common case) means no drop is pending — return O(1).
38+
// 1. SM-lock scope: PEEK the earliest armed index for this vShard.
39+
// `None` (the common case) means no catch-up is pending — return O(1).
4040
// Otherwise pair it with the committed-index watermark as the replay
41-
// upper bound. Release the SM lock before the MultiRaft read.
41+
// upper bound. We PEEK rather than TAKE: the entry is cleared only
42+
// after a confirmed replay (step 4), so a tick that cannot complete
43+
// the replay (committed index not yet known, transient log-read
44+
// fault) leaves the catch-up armed for the next tick instead of
45+
// silently dropping it. Release the SM lock before the MultiRaft read.
4246
let (lo, hi) = {
4347
let sm = self
4448
.sequencer_state_machine
4549
.lock()
4650
.unwrap_or_else(|p| p.into_inner());
47-
let Some(lo) = sm.take_catch_up_from(self.vshard_id) else {
51+
let Some(lo) = sm.peek_catch_up_from(self.vshard_id) else {
4852
return;
4953
};
5054
let Some(hi) = sm.current_committed_index() else {
51-
// A drop was recorded but nothing is applied yet — nothing to
52-
// replay. The take already cleared the entry; the next drop
53-
// re-records a fresh index.
55+
// Armed but nothing applied yet — leave it armed and retry once
56+
// an entry is applied and `hi` is known.
5457
return;
5558
};
59+
if lo > hi {
60+
// Armed ahead of the committed watermark (e.g. spawn-armed from
61+
// the first available index before any entry applied on this
62+
// replica). Nothing to replay yet; stay armed.
63+
return;
64+
}
5665
(lo, hi)
5766
};
5867

@@ -65,29 +74,31 @@ impl Scheduler {
6574
Err(nodedb_cluster::error::ClusterError::Raft(
6675
nodedb_raft::RaftError::LogCompacted { .. },
6776
)) => {
68-
// The dropped index has been compacted into a snapshot.
69-
// Reachable ONLY via a sequencer-group snapshot-install
70-
// resync, where the installed snapshot already subsumes the
71-
// dropped index — so this replica's state is already correct
72-
// and no replay is owed. Escalate non-silently (error log +
73-
// metric) and RETURN WITHOUT re-arming: the take already
74-
// cleared the entry, and re-inserting it would infinite-loop
75-
// since the log stays compacted. The provably-unreachable fix
76-
// (a sequencer-compaction hold-down keyed on
77-
// `min(catch_up_from)`) is a separately-scoped follow-up.
77+
// The armed index has been compacted below the retained log.
78+
// The sequencer-group compaction hold-down (floored at
79+
// `min_catch_up_from`) is meant to make this unreachable for
80+
// an armed catch-up; if it is nonetheless hit (e.g. a
81+
// snapshot-install resync that already subsumes this index),
82+
// no replay is owed. Escalate non-silently, CLEAR the entry
83+
// to avoid an infinite retry against a permanently-compacted
84+
// index, and return.
7885
self.metrics.record_catch_up_log_compacted();
7986
tracing::error!(
8087
vshard = self.vshard_id,
8188
lo,
82-
"calvin catch-up: sequencer log compacted below dropped index; \
89+
"calvin catch-up: sequencer log compacted below armed index; \
8390
state is snapshot-covered"
8491
);
92+
self.sequencer_state_machine
93+
.lock()
94+
.unwrap_or_else(|p| p.into_inner())
95+
.clear_catch_up_up_to(self.vshard_id, hi);
8596
return;
8697
}
8798
Err(e) => {
88-
// Transient infra fault (e.g. group transiently absent). The
89-
// take cleared the entry; a subsequent drop re-records and a
90-
// later drain retries. Surface it rather than swallow.
99+
// Transient infra fault (e.g. group transiently absent).
100+
// Leave the catch-up ARMED (we peeked, did not take) so a
101+
// later drain retries it. Surface it rather than swallow.
91102
tracing::warn!(
92103
vshard = self.vshard_id,
93104
lo,
@@ -119,6 +130,16 @@ impl Scheduler {
119130
self.process_scheduler_input(input);
120131
}
121132

133+
// Replay of `lo ..= hi` is complete: clear the armed catch-up, but only
134+
// up to `hi` — a concurrent drop recorded at an index `> hi` while this
135+
// replay ran is preserved for the next drain. This is the CONFIRM step
136+
// the peek-not-take at the top defers to; a transient failure above
137+
// returned early and left the entry armed.
138+
self.sequencer_state_machine
139+
.lock()
140+
.unwrap_or_else(|p| p.into_inner())
141+
.clear_catch_up_up_to(self.vshard_id, hi);
142+
122143
if replayed > 0 {
123144
self.metrics.record_catch_up_replayed(replayed);
124145
tracing::info!(

nodedb/src/control/cluster/start_raft/core.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ pub fn start_raft(
4343
loop_build.tracker,
4444
loop_build.apply_rx,
4545
loop_build.calvin_read_result_senders,
46+
loop_build.sequencer_state_machine,
4647
);
4748

4849
let ready_rx = finish_observability(

nodedb/src/control/cluster/start_raft/loop_build.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ use std::sync::{Arc, Mutex};
1111
use tokio::sync::mpsc::{self, Sender};
1212

1313
use nodedb_cluster::calvin::{
14-
CalvinCompletionRegistry, SequencerConfig, SequencerReceivers, SequencerService, new_inbox,
15-
new_reservation_inbox,
14+
CalvinCompletionRegistry, SequencerConfig, SequencerReceivers, SequencerService,
15+
SequencerStateMachine, new_inbox, new_reservation_inbox,
1616
};
1717

1818
use crate::control::cluster::calvin::executor::ollp::OllpConfig;
@@ -49,6 +49,9 @@ pub(super) struct LoopBuild {
4949
pub(super) apply_rx: mpsc::Receiver<ApplyBatch>,
5050
pub(super) calvin_read_result_senders: Arc<Mutex<BTreeMap<u32, Sender<ReadResultEvent>>>>,
5151
pub(super) calvin_completion_registry: Arc<CalvinCompletionRegistry>,
52+
/// Shared with the compactor wiring so sequencer-group log compaction can
53+
/// be held down below any armed scheduler catch-up index.
54+
pub(super) sequencer_state_machine: Arc<Mutex<SequencerStateMachine>>,
5255
}
5356

5457
/// Build the `RaftLoop`, consume `pending_subsystems`, build the sequencer
@@ -184,5 +187,6 @@ pub(super) fn build_raft_loop(
184187
apply_rx,
185188
calvin_read_result_senders,
186189
calvin_completion_registry,
190+
sequencer_state_machine,
187191
})
188192
}

nodedb/src/control/cluster/start_raft/proposer_wiring.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub(super) fn wire_proposers(
2525
tracker: Arc<ProposeTracker>,
2626
apply_rx: mpsc::Receiver<ApplyBatch>,
2727
calvin_read_result_senders: Arc<Mutex<BTreeMap<u32, Sender<ReadResultEvent>>>>,
28+
sequencer_state_machine: Arc<Mutex<nodedb_cluster::calvin::SequencerStateMachine>>,
2829
) {
2930
// Wire the Raft proposer into SharedState so CP dispatch paths
3031
// (pgwire, HTTP, array inbound) can route writes through Raft.
@@ -58,14 +59,45 @@ pub(super) fn wire_proposers(
5859
// `log_compaction_threshold` is `None`.
5960
// Weak for the same cycle-breaking reason as `raft_proposer` above.
6061
let raft_loop_for_compact = Arc::downgrade(raft_loop);
62+
let sm_for_compact = Arc::clone(&sequencer_state_machine);
6163
let compactor: Arc<crate::control::wal_replication::RaftCompactor> =
6264
Arc::new(move |group_id, applied_index| {
6365
let rl = raft_loop_for_compact
6466
.upgrade()
6567
.ok_or_else(|| crate::Error::Internal {
6668
detail: "raft log compaction: cluster not running".into(),
6769
})?;
68-
rl.maybe_compact_group(group_id, applied_index)
70+
71+
// Sequencer-group hold-down. Unlike a data group — whose entries are
72+
// replayable from each replica's own durable state — a cross-shard
73+
// Calvin txn is re-derived on every replica ONLY from the sequencer
74+
// log. A scheduler that missed a fan-out (channel full/closed, or it
75+
// had not subscribed yet) recovers by replaying that log from its
76+
// armed catch-up index. Compacting past an armed index destroys the
77+
// only copy, permanently losing the txn on that replica — which for a
78+
// cross-shard graph edge means the edge silently vanishes from that
79+
// node's index. Floor the compaction boundary strictly below the
80+
// lowest armed catch-up so the replay range always survives.
81+
let effective_index = if group_id == nodedb_cluster::calvin::SEQUENCER_GROUP_ID {
82+
match sm_for_compact
83+
.lock()
84+
.unwrap_or_else(|p| p.into_inner())
85+
.min_catch_up_from()
86+
{
87+
// Keep index `m` itself: compaction discards entries at and
88+
// below its boundary, and the replay range starts AT `m`.
89+
Some(m) => applied_index.min(m.saturating_sub(1)),
90+
None => applied_index,
91+
}
92+
} else {
93+
applied_index
94+
};
95+
if effective_index == 0 {
96+
// Nothing compactable once held down.
97+
return Ok(false);
98+
}
99+
100+
rl.maybe_compact_group(group_id, effective_index)
69101
.map_err(|e| crate::Error::Internal {
70102
detail: format!("raft log compaction: {e}"),
71103
})

0 commit comments

Comments
 (0)