slice-5: drain deadline covers the send; doc fixes (final review)
All checks were successful
All checks were successful
The shutdown drain send could block past the deadline on a wedged
media thread with a full command channel — same whole-round-trip
principle as readyz's 5ce18bf. Plus session_map doc coherence and a
plan deviation note pointing snippet-copiers at the landed code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012QndwfhjyTiZcUYp87dwW8
Signed-off-by: Aaron D. Lee <himself@adlee.work>
This commit is contained in:
@@ -76,15 +76,24 @@ async fn main() {
|
||||
shutdown_signal().await;
|
||||
if !drain_deadline.is_zero() {
|
||||
let (reply, drained) = tokio::sync::oneshot::channel();
|
||||
if drain_cmd_tx
|
||||
.send(rutster::media_thread::MediaCmd::Drain { reply })
|
||||
.await
|
||||
.is_ok()
|
||||
{
|
||||
match tokio::time::timeout(drain_deadline, drained).await {
|
||||
Ok(_) => info!("drain complete; proceeding to shutdown"),
|
||||
Err(_) => info!(?drain_deadline, "drain deadline hit; shutting down anyway"),
|
||||
// One deadline bounds the WHOLE drain round trip — send AND
|
||||
// completion. A wedged media thread can stall the send itself
|
||||
// (full command channel), and the shutdown promise is "bounded
|
||||
// by the deadline" against exactly that node. Same principle
|
||||
// as readyz's whole-round-trip timeout (commit 5ce18bf).
|
||||
let outcome = tokio::time::timeout(drain_deadline, async {
|
||||
if drain_cmd_tx
|
||||
.send(rutster::media_thread::MediaCmd::Drain { reply })
|
||||
.await
|
||||
.is_ok()
|
||||
{
|
||||
let _ = drained.await;
|
||||
}
|
||||
})
|
||||
.await;
|
||||
match outcome {
|
||||
Ok(()) => info!("drain complete; proceeding to shutdown"),
|
||||
Err(_) => info!(?drain_deadline, "drain deadline hit; shutting down anyway"),
|
||||
}
|
||||
}
|
||||
let _ = http_stop_tx.send(());
|
||||
|
||||
@@ -80,13 +80,16 @@ impl AppState {
|
||||
let _ = rx.await;
|
||||
}
|
||||
|
||||
/// Spawn the dedicated media thread for this state.
|
||||
/// Spawn the dedicated media thread with default options.
|
||||
///
|
||||
/// Returns the state with the real command-channel sender wired in,
|
||||
/// plus the `MediaThread` handle. Routes on the returned `AppState`
|
||||
/// will send `MediaCmd` to the running media thread.
|
||||
/// Spawn with defaults — the test-facing convenience; production goes
|
||||
/// through `spawn_media_thread_with` so env config reaches the thread.
|
||||
/// Test-facing convenience: most tests don't care about addressing,
|
||||
/// admission caps, or event sinks, so this skips straight to
|
||||
/// `MediaThreadOpts::default()`. Production always goes through
|
||||
/// [`AppState::spawn_media_thread_with`] instead, so that env-resolved
|
||||
/// config actually reaches the thread. Returns the state with the real
|
||||
/// command-channel sender wired in, plus the `MediaThread` handle;
|
||||
/// routes on the returned `AppState` will send `MediaCmd` to the
|
||||
/// running media thread.
|
||||
pub fn spawn_media_thread(
|
||||
self,
|
||||
tokio_handle: tokio::runtime::Handle,
|
||||
@@ -97,6 +100,15 @@ impl AppState {
|
||||
)
|
||||
}
|
||||
|
||||
/// Spawn the dedicated media thread with explicit options — the
|
||||
/// production path.
|
||||
///
|
||||
/// Unlike [`AppState::spawn_media_thread`], this threads a caller-built
|
||||
/// `MediaThreadOpts` (media addressing, admission cap, event sink) all
|
||||
/// the way to the media thread, so `main` can resolve those from env
|
||||
/// vars and have them actually take effect. Returns the state with the
|
||||
/// real command-channel sender installed, plus the `MediaThread`
|
||||
/// handle.
|
||||
pub fn spawn_media_thread_with(
|
||||
mut self,
|
||||
opts: crate::media_thread::MediaThreadOpts,
|
||||
|
||||
@@ -1150,6 +1150,11 @@ git commit -s -m "slice-5: drain lifecycle — SIGTERM bleeds out calls (review
|
||||
|
||||
### Task 5: `GET /healthz` + `GET /readyz`
|
||||
|
||||
> **Deviation note (post-review):** the snippets below contain two bugs found and fixed
|
||||
> in commit 5ce18bf during execution — the readyz timeout must wrap the SEND as well as
|
||||
> the reply await, and the integration test must shut its `MediaThread` down. Copy from
|
||||
> the landed code, not these snippets.
|
||||
|
||||
**Files:**
|
||||
- Modify: `crates/rutster/src/routes.rs` (two routes + handlers)
|
||||
- Test: `routes.rs` inline (default-AppState zombie case) + `crates/rutster/tests/api_integration.rs` (live case)
|
||||
|
||||
Reference in New Issue
Block a user