Skip to content

Commit 4e2d520

Browse files
committed
fix(cluster): tighten leader liveness, lease gating, and dial handling
Raft: a successful AppendEntries response now reports the last entry the follower shares with the leader and holds durably, and the leader uses it as the follower's match index. Quorum contact tracking and the leader lease are reworked around this, and the contested-election marker is removed. Transport: concurrent sends to one peer share a single dial, and a connection is evicted only when it failed. Control plane: the auth lease, leased reads, the calvin scheduler, gateway routing, and graph dispatch read leaders through a new live-leaders snapshot that takes the Raft status before the routing guard. The lock order between MultiRaft and the routing table is documented to prevent a nested-read deadlock.
1 parent a51e6ed commit 4e2d520

62 files changed

Lines changed: 2164 additions & 690 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎nodedb-cluster-tests/tests/common_suite/cases/sql_schedule_lease_gating.rs‎

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
//! - While one node holds the lease, only that node records runs.
1111
//! - Once that node is cut off from both peers, its lease lapses and it
1212
//! records no further run. A peer takes the lease and records runs.
13+
//! - The leader balancer can move the lease between the live peers, so
14+
//! either peer can fire. No two nodes ever fire the same minute.
1315
//!
1416
//! The test waits on minute boundaries, so it runs for a few minutes.
1517
@@ -26,21 +28,28 @@ const CONVERGE: Duration = Duration::from_secs(30);
2628
const FIRE_WAIT: Duration = Duration::from_secs(150);
2729
const STEP: Duration = Duration::from_millis(250);
2830

29-
/// Runs `node` recorded for the schedule.
30-
fn runs(node: &TestClusterNode) -> usize {
31+
/// The minute of each run `node` recorded for the schedule.
32+
fn run_minutes(node: &TestClusterNode) -> Vec<u64> {
3133
let Some(def) = node
3234
.shared
3335
.schedule_registry
3436
.list_all()
3537
.into_iter()
3638
.find(|def| def.name == SCHEDULE)
3739
else {
38-
return 0;
40+
return Vec::new();
3941
};
4042
node.shared
4143
.job_history
4244
.last_runs(def.database_id, def.tenant_id, SCHEDULE, 100)
43-
.len()
45+
.iter()
46+
.map(|run| run.started_at / 60_000)
47+
.collect()
48+
}
49+
50+
/// Runs `node` recorded for the schedule.
51+
fn runs(node: &TestClusterNode) -> usize {
52+
run_minutes(node).len()
4453
}
4554

4655
/// Cut `node_id` off from every other node, both ways.
@@ -144,38 +153,25 @@ async fn a_cross_collection_schedule_fires_only_on_the_lease_holder() {
144153
== 1
145154
})
146155
.await;
147-
let second = peers
148-
.iter()
149-
.find(|node| holds_vshard0_lease(node))
150-
.map(|node| node.node_id)
151-
.expect("the new holder");
152156

153-
// The new holder fires. The cut-off node, through the same minute
154-
// boundaries, fires nothing more.
155-
wait_for(
156-
"the new lease holder fires the schedule",
157-
FIRE_WAIT,
158-
STEP,
159-
|| {
160-
peers
161-
.iter()
162-
.any(|node| node.node_id == second && runs(node) > 0)
163-
},
164-
)
157+
// A peer fires. The cut-off node, through the same minute boundaries,
158+
// fires nothing more.
159+
wait_for("a peer fires the schedule", FIRE_WAIT, STEP, || {
160+
peers.iter().any(|node| runs(node) > 0)
161+
})
165162
.await;
166163
assert_eq!(
167164
runs(cut_off),
168165
lapsed_runs,
169166
"a node without its lease fires nothing"
170167
);
171-
for node in peers.iter().filter(|node| node.node_id != second) {
172-
assert_eq!(
173-
runs(node),
174-
0,
175-
"node {} holds no lease and fires nothing",
176-
node.node_id
177-
);
178-
}
168+
169+
// One coordinator fires each minute, whichever peer holds the lease.
170+
let mut minutes: Vec<u64> = cluster.nodes.iter().flat_map(run_minutes).collect();
171+
let fired = minutes.len();
172+
minutes.sort_unstable();
173+
minutes.dedup();
174+
assert_eq!(minutes.len(), fired, "two nodes fired the same minute");
179175

180176
cluster.shutdown().await;
181177
}

‎nodedb-cluster/src/group_disk/staged.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,18 @@ impl StagedLogStorage {
6262
pub fn ticket(&self) -> Option<DurabilityTicket> {
6363
DurabilityTicket::for_staged(&self.disk)
6464
}
65+
66+
/// The sequence number of the last staged write. A caller reads it
67+
/// before a Raft call and passes it to [`Self::reply_ticket`].
68+
pub fn staged_through(&self) -> u64 {
69+
self.disk.staged_through()
70+
}
71+
72+
/// A ticket for the writes a reply built after `mark` depends on, or
73+
/// `None` when they are durable. See [`DurabilityTicket::for_reply`].
74+
pub fn reply_ticket(&self, mark: u64) -> Option<DurabilityTicket> {
75+
DurabilityTicket::for_reply(&self.disk, mark)
76+
}
6577
}
6678

6779
impl Drop for StagedLogStorage {

‎nodedb-cluster/src/group_disk/ticket.rs‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,25 @@ impl DurabilityTicket {
3939
})
4040
}
4141

42+
/// A ticket for a reply staged after `mark`, or `None` when the writes it
43+
/// depends on are durable. `mark` is [`GroupDisk::staged_through`] read
44+
/// before the Raft call that built the reply.
45+
///
46+
/// A reply depends on the latest hard state, whoever staged it: its term
47+
/// and vote must survive a restart. When the call staged writes, the reply
48+
/// depends on them too. Writes are durable in staging order, so the
49+
/// ticket then covers every write staged so far. An `AppliedIndex` write
50+
/// staged by the apply loop is no dependency of a reply that staged
51+
/// nothing.
52+
pub(super) fn for_reply(disk: &Arc<GroupDisk>, mark: u64) -> Option<Self> {
53+
let (staged, hard_state) = disk.staged_marks();
54+
let seq = if staged > mark { staged } else { hard_state };
55+
(disk.progress().durable_seq < seq).then(|| Self {
56+
disk: Arc::clone(disk),
57+
seq,
58+
})
59+
}
60+
4261
/// The group the ticket belongs to.
4362
pub fn group_id(&self) -> u64 {
4463
self.disk.group_id()
@@ -71,3 +90,63 @@ impl DurabilityTicket {
7190
self.disk.wait_blocking(self.seq)
7291
}
7392
}
93+
94+
#[cfg(test)]
95+
mod tests {
96+
use nodedb_raft::message::LogEntry;
97+
use nodedb_raft::state::HardState;
98+
99+
use super::super::writer::StorageOp;
100+
use super::*;
101+
102+
/// A disk with no writer thread: nothing staged becomes durable.
103+
fn idle_disk() -> (tempfile::TempDir, Arc<GroupDisk>) {
104+
let dir = tempfile::tempdir().expect("tempdir");
105+
let path = dir.path().join("group-3.redb");
106+
let disk = Arc::new(GroupDisk::open(3, &path).expect("open"));
107+
(dir, disk)
108+
}
109+
110+
#[test]
111+
fn a_reply_that_staged_nothing_waits_on_no_applied_index() {
112+
let (_dir, disk) = idle_disk();
113+
disk.stage(StorageOp::AppliedIndex(4));
114+
let mark = disk.staged_through();
115+
116+
assert!(DurabilityTicket::for_reply(&disk, mark).is_none());
117+
assert!(
118+
DurabilityTicket::for_staged(&disk).is_some(),
119+
"the applied index is not durable"
120+
);
121+
}
122+
123+
#[test]
124+
fn a_reply_waits_on_the_latest_hard_state_whoever_staged_it() {
125+
let (_dir, disk) = idle_disk();
126+
disk.stage(StorageOp::HardState(HardState {
127+
current_term: 2,
128+
voted_for: 5,
129+
}));
130+
disk.stage(StorageOp::AppliedIndex(4));
131+
let mark = disk.staged_through();
132+
133+
let ticket =
134+
DurabilityTicket::for_reply(&disk, mark).expect("the hard state is not durable");
135+
assert_eq!(ticket.seq, 1, "the later applied index is no dependency");
136+
}
137+
138+
#[test]
139+
fn a_reply_that_staged_writes_waits_on_every_staged_write() {
140+
let (_dir, disk) = idle_disk();
141+
disk.stage(StorageOp::AppliedIndex(4));
142+
let mark = disk.staged_through();
143+
disk.stage(StorageOp::Append(vec![LogEntry {
144+
term: 1,
145+
index: 5,
146+
data: Vec::new(),
147+
}]));
148+
149+
let ticket = DurabilityTicket::for_reply(&disk, mark).expect("the append is not durable");
150+
assert_eq!(ticket.seq, 2);
151+
}
152+
}

‎nodedb-cluster/src/group_disk/writer.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ struct Queue {
6060
ops: VecDeque<(u64, StorageOp)>,
6161
/// The sequence number of the last staged write.
6262
staged_through: u64,
63+
/// The sequence number of the last staged hard state, or 0.
64+
hard_state_seq: u64,
6365
closed: bool,
6466
}
6567

@@ -113,6 +115,9 @@ impl GroupDisk {
113115
let mut queue = self.queue.lock().unwrap_or_else(|p| p.into_inner());
114116
queue.staged_through += 1;
115117
let seq = queue.staged_through;
118+
if matches!(op, StorageOp::HardState(_)) {
119+
queue.hard_state_seq = seq;
120+
}
116121
queue.ops.push_back((seq, op));
117122
self.work.notify_one();
118123
}
@@ -125,6 +130,13 @@ impl GroupDisk {
125130
.staged_through
126131
}
127132

133+
/// The sequence numbers of the last staged write and of the last staged
134+
/// hard state, read together.
135+
pub(super) fn staged_marks(&self) -> (u64, u64) {
136+
let queue = self.queue.lock().unwrap_or_else(|p| p.into_inner());
137+
(queue.staged_through, queue.hard_state_seq)
138+
}
139+
128140
/// How far the disk has come.
129141
pub fn progress(&self) -> DiskProgress {
130142
*self.progress.lock().unwrap_or_else(|p| p.into_inner())

‎nodedb-cluster/src/multi_raft/core.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ pub struct MultiRaft {
3636
/// conf-changes applied here (via `apply_conf_change`) write THROUGH to
3737
/// the one table the query/data plane reads. Raft is the convergence
3838
/// mechanism on every applying node (leader and follower).
39+
///
40+
/// Lock order: this struct's mutex first, then this routing guard.
41+
/// See the lock-order section on [`RoutingTable`].
3942
pub(super) routing: Arc<RwLock<RoutingTable>>,
4043
/// Default election timeout range.
4144
pub(super) election_timeout_min: Duration,

‎nodedb-cluster/src/multi_raft/membership.rs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,6 @@ impl MultiRaft {
8888
self.groups.get(&group_id)?.peer_ack_count(peer)
8989
}
9090

91-
/// Whether this node leads `group_id` by an election against other
92-
/// voters that no leadership transfer started.
93-
pub fn leads_by_contested_election(&self, group_id: u64) -> bool {
94-
self.groups
95-
.get(&group_id)
96-
.is_some_and(|node| node.leads_by_contested_election())
97-
}
98-
9991
/// Whether `peer` holds every entry this leader has committed in
10092
/// `group_id`, and did not ask for a snapshot last. `false` when this
10193
/// node does not lead the group.

‎nodedb-cluster/src/multi_raft/rpc_dispatch.rs‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ impl MultiRaft {
123123
/// changed since the last persist. Runs under the `MultiRaft` lock and
124124
/// never waits on disk. Before an RPC reply that granted a vote or bumped
125125
/// the term leaves this node, the caller awaits the group's
126-
/// [`MultiRaft::durability_ticket`], so a restart cannot forget the vote
127-
/// and let two leaders form.
126+
/// [`MultiRaft::reply_ticket`], so a restart cannot forget the vote and
127+
/// let two leaders form.
128128
///
129129
/// No-op when the group is not mounted on this node.
130130
pub fn persist_group_hard_state(&mut self, group_id: u64) -> Result<()> {
@@ -134,6 +134,27 @@ impl MultiRaft {
134134
Ok(())
135135
}
136136

137+
/// The sequence number of the last write `group_id` staged, or 0 when the
138+
/// group is not mounted. A reply handler reads it before its Raft call
139+
/// and passes it to [`Self::reply_ticket`].
140+
pub fn staged_through(&self, group_id: u64) -> u64 {
141+
self.groups
142+
.get(&group_id)
143+
.map_or(0, |node| node.storage().staged_through())
144+
}
145+
146+
/// A ticket for the writes a reply depends on: the group's latest hard
147+
/// state, and every write staged after `mark` when there is one. `None`
148+
/// when those are durable or the group is not mounted. Writes the apply
149+
/// loop staged before `mark` hold no reply back.
150+
pub fn reply_ticket(
151+
&self,
152+
group_id: u64,
153+
mark: u64,
154+
) -> Option<crate::group_disk::DurabilityTicket> {
155+
self.groups.get(&group_id)?.storage().reply_ticket(mark)
156+
}
157+
137158
/// Get the current term and snapshot metadata for a group (for building
138159
/// InstallSnapshot RPCs).
139160
pub fn snapshot_metadata(&self, group_id: u64) -> Result<(u64, u64, u64)> {

0 commit comments

Comments
 (0)