Skip to content

Commit 5f6abeb

Browse files
committed
test(cluster): replace fixed sleeps with readiness polling and fix SWIM probe test
Replace fixed tokio::time::sleep calls in tests with bounded polling loops that break as soon as the expected condition is met. This makes tests deterministic under heavy parallel load (500+ unit tests sharing the same CPU pool) rather than depending on timing assumptions. In the SWIM indirect_ack_saves_target test, remove start_paused — paused time auto-advances timeouts before channel-woken tasks are polled, making the indirect path race its own timeout. With real time the 40 ms probe window is ample for in-memory delivery. Add a recv loop on the local endpoint to resolve inflight probes when Acks arrive, and handle both Ping and PingReq in the helper so the test is correct regardless of which node the scheduler picks as the direct target. In the cluster harness, replace the 200 ms sleep before peers join with a topology-readiness poll, and serialise node 2 joining before node 3 to avoid overwhelming the bootstrap leader's join handler under load. Extend the descriptor lease planner wait budget from 3 s to 10 s to match the extended election timeout range used in tests.
1 parent dbe01c9 commit 5f6abeb

7 files changed

Lines changed: 147 additions & 59 deletions

File tree

nodedb-cluster/src/raft_loop/loop_core.rs

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -580,35 +580,38 @@ mod tests {
580580
let sr1h = shutdown_tx.subscribe();
581581
tokio::spawn(async move { t1.serve(rl1_h, sr1h).await });
582582

583-
tokio::time::sleep(Duration::from_millis(200)).await;
584-
585-
assert!(
586-
rl1.applier.count() >= 1,
587-
"node 1 should have committed at least the no-op, got {}",
588-
rl1.applier.count()
589-
);
583+
// Poll until node 1 commits at least the no-op (election done).
584+
let deadline = tokio::time::Instant::now() + Duration::from_secs(5);
585+
loop {
586+
if rl1.applier.count() >= 1 {
587+
break;
588+
}
589+
assert!(
590+
tokio::time::Instant::now() < deadline,
591+
"node 1 should have committed at least the no-op, got {}",
592+
rl1.applier.count()
593+
);
594+
tokio::time::sleep(Duration::from_millis(20)).await;
595+
}
590596

591597
let (_gid, idx) = rl1.propose(0, b"distributed-cmd".to_vec()).unwrap();
592598
assert!(idx >= 2);
593599

594-
tokio::time::sleep(Duration::from_millis(200)).await;
595-
596-
assert!(
597-
rl1.applier.count() >= 2,
598-
"node 1: expected >= 2 applied, got {}",
599-
rl1.applier.count()
600-
);
601-
602-
assert!(
603-
rl2.applier.count() >= 1,
604-
"node 2: expected >= 1 applied, got {}",
605-
rl2.applier.count()
606-
);
607-
assert!(
608-
rl3.applier.count() >= 1,
609-
"node 3: expected >= 1 applied, got {}",
610-
rl3.applier.count()
611-
);
600+
// Poll until all nodes replicate the proposed command.
601+
let deadline = tokio::time::Instant::now() + Duration::from_secs(5);
602+
loop {
603+
if rl1.applier.count() >= 2 && rl2.applier.count() >= 1 && rl3.applier.count() >= 1 {
604+
break;
605+
}
606+
assert!(
607+
tokio::time::Instant::now() < deadline,
608+
"replication timed out: n1={}, n2={}, n3={}",
609+
rl1.applier.count(),
610+
rl2.applier.count(),
611+
rl3.applier.count()
612+
);
613+
tokio::time::sleep(Duration::from_millis(20)).await;
614+
}
612615

613616
shutdown_tx.send(true).unwrap();
614617
}

nodedb-cluster/src/swim/detector/probe_round.rs

Lines changed: 64 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -413,13 +413,16 @@ mod tests {
413413
);
414414
}
415415

416-
#[tokio::test(start_paused = true)]
416+
#[tokio::test]
417417
async fn indirect_ack_saves_target() {
418+
// No `start_paused` — paused time auto-advances timeouts
419+
// before polling channel-woken tasks, making the indirect
420+
// path race the timeout. With real time, the 40ms probe
421+
// timeout is ample for the in-memory fabric (sub-µs delivery).
418422
let fab = TransportFabric::new();
419-
let local = Arc::new(fab.bind(addr(7000)).await) as Arc<dyn Transport>;
420-
// Target bound but silent on the direct channel.
423+
let local = Arc::new(fab.bind(addr(7000)).await);
421424
let _silent = fab.bind(addr(7001)).await;
422-
let helper = fab.bind(addr(7002)).await;
425+
let helper = Arc::new(fab.bind(addr(7002)).await);
423426
let list = membership_with_peers(
424427
"local",
425428
7000,
@@ -432,38 +435,74 @@ mod tests {
432435
let mut sched = ProbeScheduler::with_seed(1);
433436
let inflight = Arc::new(InflightProbes::new());
434437

435-
// Helper task: forwards any PingReq it sees into an Ack via the
436-
// inflight registry. Paused-runtime auto-advance drives the
437-
// direct-ping timeout on the main task.
438-
let inflight_helper = Arc::clone(&inflight);
438+
// Helper task: respond to Ping (direct probe) with Ack and to
439+
// PingReq (indirect probe) with a forwarded Ack — mirrors the
440+
// production runner recv-loop + handle_ping_req path. The
441+
// scheduler may pick n2 as direct target or as indirect helper
442+
// depending on the shuffle seed, so both must be handled.
443+
let helper_t: Arc<dyn Transport> = helper.clone();
439444
let responder = tokio::spawn(async move {
440445
loop {
441-
let (_from, msg) = match helper.recv().await {
446+
let (from, msg) = match helper_t.recv().await {
442447
Ok(v) => v,
443448
Err(_) => return,
444449
};
445-
if let SwimMessage::PingReq(req) = msg {
446-
inflight_helper
447-
.resolve(
448-
req.probe_id,
449-
SwimMessage::Ack(Ack {
450-
probe_id: req.probe_id,
451-
from: req.target.clone(),
452-
incarnation: Incarnation::new(9),
453-
piggyback: vec![],
454-
}),
455-
)
456-
.await;
457-
return;
450+
match msg {
451+
SwimMessage::Ping(ping) => {
452+
let _ = helper_t
453+
.send(
454+
from,
455+
SwimMessage::Ack(Ack {
456+
probe_id: ping.probe_id,
457+
from: NodeId::new("n2"),
458+
incarnation: Incarnation::new(9),
459+
piggyback: vec![],
460+
}),
461+
)
462+
.await;
463+
return;
464+
}
465+
SwimMessage::PingReq(req) => {
466+
let _ = helper_t
467+
.send(
468+
from,
469+
SwimMessage::Ack(Ack {
470+
probe_id: req.probe_id,
471+
from: req.target.clone(),
472+
incarnation: Incarnation::new(9),
473+
piggyback: vec![],
474+
}),
475+
)
476+
.await;
477+
return;
478+
}
479+
_ => {}
458480
}
459481
}
460482
});
461483

484+
// Recv-loop on the local endpoint: resolves inflight probes
485+
// when Acks arrive — mirrors the production runner recv-loop.
486+
let recv_t: Arc<dyn Transport> = local.clone();
487+
let recv_inflight = Arc::clone(&inflight);
488+
let recv_loop = tokio::spawn(async move {
489+
loop {
490+
let (_from, msg) = match recv_t.recv().await {
491+
Ok(v) => v,
492+
Err(_) => return,
493+
};
494+
if let SwimMessage::Ack(ref ack) = msg {
495+
recv_inflight.resolve(ack.probe_id, msg).await;
496+
}
497+
}
498+
});
499+
500+
let local_dyn: Arc<dyn Transport> = local.clone();
462501
let dissemination = Arc::new(DisseminationQueue::new());
463502
let outcome = ProbeRound {
464503
scheduler: &mut sched,
465504
membership: &list,
466-
transport: &local,
505+
transport: &local_dyn,
467506
inflight: &inflight,
468507
dissemination: &dissemination,
469508
probe_timeout: cfg().probe_timeout,
@@ -476,9 +515,8 @@ mod tests {
476515
.execute()
477516
.await
478517
.expect("run");
479-
let _ = responder.await;
480-
// Either direct (unlikely — n1 is silent) or indirect ack via n2.
481-
// Whichever path fires, the outcome must be Acked.
518+
responder.abort();
519+
recv_loop.abort();
482520
assert!(matches!(outcome, ProbeOutcome::Acked { .. }));
483521
}
484522

nodedb-cluster/tests/common/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,8 @@ impl TestNode {
187187
max_backoff_secs: 2,
188188
},
189189
swim_udp_addr: None,
190+
election_timeout_min: std::time::Duration::from_millis(150),
191+
election_timeout_max: std::time::Duration::from_millis(300),
190192
};
191193

192194
let lifecycle = ClusterLifecycleTracker::new();

nodedb/tests/cluster_execute_request.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,19 @@ async fn execute_request_cross_node_dispatch() {
162162
.await
163163
.expect("create collection");
164164

165-
// Give the metadata applier on all nodes a moment to replicate.
166-
tokio::time::sleep(Duration::from_millis(400)).await;
165+
// Wait for the collection to be visible on every node.
166+
common::cluster_harness::wait_for(
167+
"cross_node_kv visible on all nodes",
168+
Duration::from_secs(10),
169+
Duration::from_millis(50),
170+
|| {
171+
cluster
172+
.nodes
173+
.iter()
174+
.all(|n| n.cached_collection_count() >= 1)
175+
},
176+
)
177+
.await;
167178

168179
// Node 2 sends the request; node 1 (bootstrap leader) receives it.
169180
let sender_transport = cluster.nodes[1]

nodedb/tests/common/cluster_harness/cluster.rs

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,18 @@ impl TestCluster {
1717
/// via node 1's pre-bound address. Waits until every node sees
1818
/// topology_size == 3 (10s deadline).
1919
pub async fn spawn_three() -> Result<Self, Box<dyn std::error::Error + Send + Sync>> {
20-
Self::spawn_three_with_tuning(ClusterTransportTuning::default()).await
20+
Self::spawn_three_with_tuning(ClusterTransportTuning {
21+
// Fast health pings so the HealthMonitor re-broadcasts
22+
// topology within ~1s if the initial join broadcast was missed.
23+
health_ping_interval_secs: 1,
24+
// Fast election timeouts so the metadata Raft group elects a
25+
// leader well within the 10s convergence deadline, even under
26+
// heavy parallel test load.
27+
election_timeout_min_secs: 1,
28+
election_timeout_max_secs: 2,
29+
..ClusterTransportTuning::default()
30+
})
31+
.await
2132
}
2233

2334
/// Spawn a 3-node cluster with a custom `ClusterTransportTuning`.
@@ -29,12 +40,34 @@ impl TestCluster {
2940
) -> Result<Self, Box<dyn std::error::Error + Send + Sync>> {
3041
let node1 = TestClusterNode::spawn_with_tuning(1, vec![], tuning.clone()).await?;
3142

32-
// Give node 1's transport + raft loop a moment to start
33-
// accepting before peers dial in.
34-
tokio::time::sleep(Duration::from_millis(200)).await;
43+
// Wait until node 1 has bootstrapped (topology shows itself)
44+
// before peers try to join. The old fixed 200ms sleep was too
45+
// short under heavy host load (e.g. 500+ parallel unit tests
46+
// sharing the same CPU pool), causing peers to dial before
47+
// node 1's transport was ready — failing topology convergence.
48+
let deadline = std::time::Instant::now() + Duration::from_secs(10);
49+
while node1.topology_size() < 1 {
50+
if std::time::Instant::now() >= deadline {
51+
return Err("node 1 failed to bootstrap within 10s".into());
52+
}
53+
tokio::time::sleep(Duration::from_millis(20)).await;
54+
}
3555

3656
let seeds = vec![node1.listen_addr];
3757
let node2 = TestClusterNode::spawn_with_tuning(2, seeds.clone(), tuning.clone()).await?;
58+
59+
// Wait for node 2's join to be reflected before spawning node 3.
60+
// Under load, spawning both peers simultaneously can overwhelm the
61+
// bootstrap leader's join handler, causing neither join to complete
62+
// within the topology convergence deadline.
63+
let deadline = std::time::Instant::now() + Duration::from_secs(10);
64+
while node1.topology_size() < 2 {
65+
if std::time::Instant::now() >= deadline {
66+
return Err("node 2 failed to join within 10s".into());
67+
}
68+
tokio::time::sleep(Duration::from_millis(20)).await;
69+
}
70+
3871
let node3 = TestClusterNode::spawn_with_tuning(3, seeds, tuning).await?;
3972

4073
let cluster = Self {
@@ -44,7 +77,7 @@ impl TestCluster {
4477
wait_for(
4578
"all 3 nodes report topology_size == 3",
4679
Duration::from_secs(10),
47-
Duration::from_millis(100),
80+
Duration::from_millis(50),
4881
|| cluster.nodes.iter().all(|n| n.topology_size() == 3),
4982
)
5083
.await;

nodedb/tests/common/cluster_harness/node.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ impl TestClusterNode {
129129
&cluster_settings,
130130
transport.clone(),
131131
&data_dir_path,
132+
&tuning,
132133
)
133134
.await?;
134135

nodedb/tests/descriptor_lease_planner_integration.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use common::cluster_harness::{TestCluster, wait_for};
1616
use nodedb_cluster::{DescriptorId, DescriptorKind};
1717

1818
const TENANT: u32 = 1;
19-
const WAIT_BUDGET: Duration = Duration::from_secs(3);
19+
const WAIT_BUDGET: Duration = Duration::from_secs(10);
2020
const POLL: Duration = Duration::from_millis(20);
2121

2222
fn coll_id(name: &str) -> DescriptorId {

0 commit comments

Comments
 (0)