Skip to content

Commit f54a7d6

Browse files
committed
refactor(nodedb): centralize shutdown signalling via LoopRegistry
Replace the ad-hoc Vec<watch::Sender<bool>> in SharedState with a dedicated control::shutdown submodule (watch.rs, receiver.rs, registry.rs, report.rs, spawn.rs). All background loops are now registered at spawn time; on SIGTERM the main shutdown path calls LoopRegistry::shutdown_all with a configurable deadline and logs any laggards. Add ShutdownTuning to TuningConfig (default 900 ms). Migrate the response poller, tenant rate-reset, audit log flush, and event trigger processor to spawn_loop, removing the bare tokio::spawn calls that previously had no shutdown awareness.
1 parent 9600257 commit f54a7d6

14 files changed

Lines changed: 1152 additions & 77 deletions

File tree

nodedb-types/src/config/tuning/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ mod data_plane;
22
mod engines;
33
mod memory;
44
mod network;
5+
mod shutdown;
56

67
pub use data_plane::{DataPlaneTuning, QueryTuning};
78
pub use engines::{
@@ -10,6 +11,7 @@ pub use engines::{
1011
};
1112
pub use memory::MemoryTuning;
1213
pub use network::{BridgeTuning, ClusterTransportTuning, NetworkTuning, WalTuning};
14+
pub use shutdown::ShutdownTuning;
1315

1416
use serde::{Deserialize, Serialize};
1517

@@ -43,6 +45,8 @@ pub struct TuningConfig {
4345
pub cluster_transport: ClusterTransportTuning,
4446
#[serde(default)]
4547
pub memory: MemoryTuning,
48+
#[serde(default)]
49+
pub shutdown: ShutdownTuning,
4650
}
4751

4852
#[cfg(test)]
@@ -84,6 +88,7 @@ mod tests {
8488
assert_eq!(parsed.memory.overflow_initial_bytes, 64 * 1024 * 1024);
8589
assert_eq!(parsed.memory.overflow_max_bytes, 1024 * 1024 * 1024);
8690
assert_eq!(parsed.memory.doc_cache_entries, 4096);
91+
assert_eq!(parsed.shutdown.deadline_ms, 900);
8792
}
8893

8994
#[test]
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//! Shutdown tuning — deadlines applied to the graceful shutdown
2+
//! path by `main.rs` and `control::shutdown::LoopRegistry`.
3+
4+
use serde::{Deserialize, Serialize};
5+
6+
fn default_shutdown_deadline_ms() -> u64 {
7+
// 900ms: the remaining 100ms of a 1s operator budget is
8+
// reserved for transport close + final WAL fsync. Chosen
9+
// so `shutdown_all` has a chance to abort async laggards
10+
// before the OS hard-kills the process on a second SIGTERM.
11+
900
12+
}
13+
14+
/// Tuning knobs for the graceful shutdown path.
15+
#[derive(Debug, Clone, Serialize, Deserialize)]
16+
pub struct ShutdownTuning {
17+
/// Deadline in milliseconds for `LoopRegistry::shutdown_all`.
18+
/// Every registered background loop must exit within this
19+
/// window after the shutdown signal is flipped; laggards
20+
/// are aborted (async) or logged (blocking).
21+
#[serde(default = "default_shutdown_deadline_ms")]
22+
pub deadline_ms: u64,
23+
}
24+
25+
impl Default for ShutdownTuning {
26+
fn default() -> Self {
27+
Self {
28+
deadline_ms: default_shutdown_deadline_ms(),
29+
}
30+
}
31+
}
32+
33+
impl ShutdownTuning {
34+
/// Convert the deadline to a `std::time::Duration`.
35+
pub fn deadline(&self) -> std::time::Duration {
36+
std::time::Duration::from_millis(self.deadline_ms)
37+
}
38+
}
39+
40+
#[cfg(test)]
41+
mod tests {
42+
use super::*;
43+
44+
#[test]
45+
fn default_deadline_is_900ms() {
46+
let t = ShutdownTuning::default();
47+
assert_eq!(t.deadline_ms, 900);
48+
assert_eq!(t.deadline(), std::time::Duration::from_millis(900));
49+
}
50+
51+
#[test]
52+
fn override_deadline() {
53+
let toml_str = r#"deadline_ms = 1500"#;
54+
let t: ShutdownTuning = toml::from_str(toml_str).unwrap();
55+
assert_eq!(t.deadline_ms, 1500);
56+
}
57+
}

nodedb/src/control/event_trigger.rs

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,43 @@ use crate::control::change_stream::{ChangeEvent, ChangeOperation};
1212
use crate::control::planner::context::QueryContext;
1313
use crate::control::state::SharedState;
1414

15-
/// Spawn the event trigger processor as a background tokio task.
16-
///
17-
/// Subscribes to the change stream and evaluates EventDefinitions
18-
/// stored in the catalog for each write event.
15+
/// Spawn the event trigger processor as a background tokio task
16+
/// registered with the shutdown loop registry. The processor
17+
/// exits on shutdown signal or when the change-stream
18+
/// broadcast channel closes.
1919
pub fn spawn_event_trigger_processor(shared: Arc<SharedState>) {
2020
let mut subscription = shared.change_stream.subscribe(None, None);
21-
22-
tokio::spawn(async move {
23-
loop {
24-
match subscription.recv_filtered().await {
25-
Ok(event) => process_event(&shared, &event).await,
26-
Err(tokio::sync::broadcast::error::RecvError::Lagged(n)) => {
27-
warn!(
28-
lagged = n,
29-
"event trigger processor fell behind; skipped {n} events"
30-
);
31-
}
32-
Err(tokio::sync::broadcast::error::RecvError::Closed) => {
33-
debug!("change stream closed; event trigger processor stopping");
34-
break;
21+
let registry = Arc::clone(&shared.loop_registry);
22+
let watch = Arc::clone(&shared.shutdown);
23+
24+
crate::control::shutdown::spawn_loop(
25+
&registry,
26+
&watch,
27+
"event_trigger_processor",
28+
move |mut shutdown| async move {
29+
loop {
30+
tokio::select! {
31+
_ = shutdown.wait_cancelled() => {
32+
debug!("event trigger processor shutting down");
33+
break;
34+
}
35+
msg = subscription.recv_filtered() => match msg {
36+
Ok(event) => process_event(&shared, &event).await,
37+
Err(tokio::sync::broadcast::error::RecvError::Lagged(n)) => {
38+
warn!(
39+
lagged = n,
40+
"event trigger processor fell behind; skipped {n} events"
41+
);
42+
}
43+
Err(tokio::sync::broadcast::error::RecvError::Closed) => {
44+
debug!("change stream closed; event trigger processor stopping");
45+
break;
46+
}
47+
},
3548
}
3649
}
37-
}
38-
});
50+
},
51+
);
3952
}
4053

4154
/// Process a single change event against all matching EventDefinitions.

nodedb/src/control/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub mod scatter_gather;
2525
pub mod security;
2626
pub mod sequence;
2727
pub mod server;
28+
pub mod shutdown;
2829
pub mod state;
2930
pub mod trace_context;
3031
pub mod trigger;

nodedb/src/control/shutdown/mod.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//! Central shutdown coordination: one watch, one registry, one
2+
//! spawn helper. Replaces the ad-hoc `_shutdown_senders: Vec<_>`
3+
//! graveyard that carried no join handles, no laggard detection,
4+
//! and no per-loop names.
5+
//!
6+
//! Every long-running background loop in the Control Plane and
7+
//! Event Plane MUST spawn via [`spawn_loop`] / [`spawn_blocking_loop`]
8+
//! and subscribe to the canonical [`ShutdownWatch`] held on
9+
//! `SharedState`. On `main.rs`'s ctrl-c path, the watch is
10+
//! signaled and [`LoopRegistry::shutdown_all`] awaits every
11+
//! registered handle with a shared deadline, aborting async
12+
//! laggards and logging blocking laggards.
13+
14+
pub mod receiver;
15+
pub mod registry;
16+
pub mod report;
17+
pub mod spawn;
18+
pub mod watch;
19+
20+
pub use receiver::ShutdownReceiver;
21+
pub use registry::{LoopHandle, LoopRegistry, RegistryClosed};
22+
pub use report::{LaggardReport, ShutdownReport};
23+
pub use spawn::{spawn_blocking_loop, spawn_loop};
24+
pub use watch::ShutdownWatch;
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
//! Per-loop consumer side of the shutdown watch.
2+
//!
3+
//! Held by every background loop. Expected usage pattern:
4+
//!
5+
//! ```ignore
6+
//! loop {
7+
//! tokio::select! {
8+
//! _ = shutdown.wait_cancelled() => break,
9+
//! _ = tick.tick() => { /* work */ }
10+
//! }
11+
//! }
12+
//! ```
13+
//!
14+
//! `wait_cancelled` is cancel-safe — dropping the future in a
15+
//! `select!` losing arm does not miss a subsequent signal
16+
//! because the underlying `watch::Receiver::changed` is itself
17+
//! cancel-safe and the state is re-checked on every poll.
18+
19+
use tokio::sync::watch;
20+
21+
/// Consumer handle for [`super::ShutdownWatch`]. One per
22+
/// subscribed loop — clone-cheap but there is no reason to
23+
/// clone: each loop gets its own handle from `subscribe()`.
24+
#[derive(Debug)]
25+
pub struct ShutdownReceiver {
26+
rx: watch::Receiver<bool>,
27+
}
28+
29+
impl ShutdownReceiver {
30+
/// Internal constructor used by [`super::ShutdownWatch::subscribe`].
31+
/// Not public — the canonical way to obtain a receiver is
32+
/// through `ShutdownWatch::subscribe` so the subscribe-fresh
33+
/// guarantee is preserved.
34+
pub(super) fn from_watch(rx: watch::Receiver<bool>) -> Self {
35+
Self { rx }
36+
}
37+
38+
/// Resolves as soon as the watch reports the shutdown flag.
39+
/// Cancel-safe for use inside `tokio::select!`.
40+
///
41+
/// If the watch has already been signaled before this is
42+
/// first polled, it returns immediately — the subscribe-fresh
43+
/// guarantee on `ShutdownWatch` means `*rx.borrow() == true`
44+
/// on the first check fires without waiting for a future
45+
/// `changed()` notification.
46+
pub async fn wait_cancelled(&mut self) {
47+
// Fast path: already signaled.
48+
if *self.rx.borrow() {
49+
return;
50+
}
51+
// Slow path: wait for a state change, then re-check.
52+
// `changed()` can return `Err` if every sender was
53+
// dropped — treat that as "shutdown is no longer
54+
// observable", which in practice only happens during
55+
// destructor races. Returning from the future preserves
56+
// the "loop exits promptly" contract.
57+
while self.rx.changed().await.is_ok() {
58+
if *self.rx.borrow() {
59+
return;
60+
}
61+
}
62+
}
63+
64+
/// Non-blocking check. Useful in the prelude of an expensive
65+
/// operation — "don't start a compaction if we're already
66+
/// shutting down".
67+
pub fn is_cancelled(&self) -> bool {
68+
*self.rx.borrow()
69+
}
70+
}
71+
72+
#[cfg(test)]
73+
mod tests {
74+
use super::super::ShutdownWatch;
75+
use std::time::Duration;
76+
77+
#[tokio::test]
78+
async fn wait_cancelled_returns_on_signal() {
79+
let watch = ShutdownWatch::new();
80+
let mut rx = watch.subscribe();
81+
82+
let handle = tokio::spawn(async move {
83+
rx.wait_cancelled().await;
84+
});
85+
86+
// Let the task park on wait_cancelled.
87+
tokio::time::sleep(Duration::from_millis(10)).await;
88+
assert!(!handle.is_finished());
89+
90+
watch.signal();
91+
tokio::time::timeout(Duration::from_millis(100), handle)
92+
.await
93+
.expect("task did not wake after signal")
94+
.expect("task panicked");
95+
}
96+
97+
#[tokio::test]
98+
async fn wait_cancelled_returns_immediately_if_already_signaled() {
99+
let watch = ShutdownWatch::new();
100+
watch.signal();
101+
102+
let mut rx = watch.subscribe();
103+
tokio::time::timeout(Duration::from_millis(10), rx.wait_cancelled())
104+
.await
105+
.expect("already-signaled receiver did not return immediately");
106+
}
107+
108+
#[tokio::test]
109+
async fn wait_cancelled_survives_select() {
110+
// Prove the drop-on-losing-arm semantics: a signal that
111+
// never arrives leaves the select free to resolve the
112+
// other future. This is the primary usage pattern in
113+
// real loops.
114+
let watch = ShutdownWatch::new();
115+
let mut rx = watch.subscribe();
116+
117+
let result = tokio::select! {
118+
_ = rx.wait_cancelled() => "shutdown",
119+
_ = tokio::time::sleep(Duration::from_millis(20)) => "tick",
120+
};
121+
assert_eq!(result, "tick");
122+
assert!(!rx.is_cancelled());
123+
}
124+
125+
#[tokio::test]
126+
async fn is_cancelled_reflects_state() {
127+
let watch = ShutdownWatch::new();
128+
let rx = watch.subscribe();
129+
assert!(!rx.is_cancelled());
130+
watch.signal();
131+
assert!(rx.is_cancelled());
132+
}
133+
}

0 commit comments

Comments
 (0)