relay: fuzz WebSocket 1012 restart-close timing on graceful drain (BUZZ_DRAIN_JITTER_MS)

[bradseiler]

Problem

On SIGTERM the relay sends every live WebSocket a 1012 Service Restart close frame via ConnectionManager::drain_all() — all in the same instant (crates/buzz-relay/src/main.rs shutdown task → state.rs::drain_all). On a pod holding thousands of sessions, that makes every client reconnect simultaneously: the thundering-herd reconnect behind the DB pool-timeout bursts observed on each rolling deploy.

Change

Add BUZZ_DRAIN_JITTER_MS (default 0 = unchanged all-at-once behavior). When set, a new ConnectionManager::drain_all_jittered(jitter_ms) spreads each connection's restart close over an independent uniform delay in [0, jitter_ms], smearing reconnects across the window. The well-attributed 1012 close is preserved, and the existing 30s hard-drain timeout still backstops the window.

• config.rs — drain_jitter_ms field, non-negative parse mirroring BUZZ_REPLICA_READ_MAX_AGE_MS (default 0, junk fails loudly).
• state.rs — drain_all_jittered: sets the sticky draining flag synchronously before returning, then schedules each captured connection's close in a detached task after its random delay. jitter_ms == 0 delegates to drain_all for identical synchronous behavior and no task spawns.
• main.rs — shutdown task calls drain_all_jittered(config.drain_jitter_ms).

Safety

• Default off. jitter_ms == 0 is byte-for-byte the current path. Safe to deploy dark and dial up.
• Shutdown-boundary race preserved. The sticky flag is set immediately, so a registration landing after the snapshot self-signals its close with no jitter (a client arriving mid-shutdown is closed at once) — same guarantee drain_all already provides.
• Backstopped. Per-connection closes run in detached tasks; the 30s hard-drain process::exit remains the ceiling. Keep the jitter well under terminationGracePeriodSeconds (60s).

Tests

• config::tests::drain_jitter_defaults_off_and_rejects_junk
• state::tests::drain_all_jittered_zero_is_synchronous_drain_all
• state::tests::drain_all_jittered_defers_close_until_within_jitter_window (paused-time: verifies deferred close fires within the window, 1012 preserved, and a late registration self-signals unaffected by jitter)

Local validation at 344bfdc48: cargo build -p buzz-relay clean; cargo clippy -p buzz-relay --all-targets -- -D warnings clean; buzz-relay --lib = 831 passed, 10 failed — all 10 are pre-existing DB-backed integration tests (Sqlx(PoolTimedOut) / media/admin/mesh) that need a live Postgres+Redis not provisioned locally; none touch drain/shutdown/config. Push hooks (rust-tests, desktop-tauri-checks) passed.

Rollout

Ship with default 0, then set BUZZ_DRAIN_JITTER_MS (e.g. 10000–20000) on staging first, watch the roll-window pool-timeout metric, then production. Complements the preStop sleep (which stops routing before close but doesn't spread within a pod).

[tlongwell-block]

Reviewed at 344bfdc486537f5d0036263a5d5ff4cfa40acda3.

Verified locally (this SHA, same shell):

• cargo clippy -p buzz-relay --all-targets -- -D warnings — clean.
• Full cargo test -p buzz-relay --lib — 840 passed, 1 failed: api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo (504 vs 200). Reproduced the identical failure on origin/main @ 027a74a61, so it is pre-existing and unrelated to this PR.
• Traced the call path: main.rs shutdown task → drain_all_jittered → per-conn detached task → restart_close_frame() + cancel(). The store-then-iterate sticky-flag ordering matches drain_all's existing contract with register's insert-then-check, and the late-registration self-signal is untouched and covered by the paused-time test.

One real issue — the phantom cap 👻:

The config.rs doc for drain_jitter_ms says "Values are capped at shutdown to leave headroom under the 30s hard-drain timeout", and the parse-site comment repeats "Bound is enforced (capped) at shutdown." No such cap exists — main.rs:1230 passes config.drain_jitter_ms straight through, and drain_all_jittered uses it unclamped.

Consequence: an operator who sets BUZZ_DRAIN_JITTER_MS=45000 (trusting the documented cap) gets every connection whose draw lands past ~30s killed by the hard-drain process::exit(1) — a TCP reset with no 1012 frame, which is precisely the silent-death failure mode this drain path exists to prevent. Default-0 is safe, but the doc promises a guardrail the code doesn't have.

Fix is one line either way:

1. (preferred) actually cap at the call site, e.g. drain_jitter_ms.min(25_000) with a comment tying it to the 30s hard timeout — makes the config doc true; or
2. fix the doc to say the value is not capped and must be kept well under 30s (the state.rs docstring already says this correctly — the two docs currently contradict each other).

Nits (no action required):

• self: &Arc<Self> receiver on drain_all_jittered — nothing in the spawned tasks captures self; &self would do.
• Docstring says the delay is drawn from [0, jitter_ms]; % jitter_ms is [0, jitter_ms). The inline comment has it right.

Everything else is genuinely tight: default-off byte-identical path, race guarantee preserved and tested under paused time, uses the existing relay jitter idiom, 239 lines mostly docs+tests. Fix the phantom cap (or its documentation) and this is ready.

[tlongwell-block]

Correction to my review above — the jitter mechanism does not survive shutdown. Downgrading from "fix the cap and ship" to "needs a redesign of task lifetime."

Max ran the real relay process (200 NIP-42-authenticated sockets, BUZZ_DRAIN_JITTER_MS=2000): the relay logged connections=200, then 0/200 received the 1012 frame — all 200 got abnormal 1006 closes at ~5.018s. The jitter=0 control delivered 1012 to 100/100.

Root cause, confirmed in the code: drain_all_jittered spawns detached timer tasks and returns immediately without cancelling any connection. With no cancel() fired synchronously, axum::serve(...).with_graceful_shutdown (main.rs:1281) completes as soon as the watch signal fires, serve() returns at main.rs:1076, main falls through the audit drain and returns — and #[tokio::main]'s runtime drop destroys every parked jitter timer and the 30s hard-timeout task. The frames never send; clients get a TCP-level 1006. The jitter=0 path only works because drain_all queues the frame and cancels synchronously before the graceful-shutdown signal propagates.

The paused-time ConnectionManager unit tests can't see this — they never tear the runtime down. That's why they pass while the live process fails at even 2s of jitter.

What a fix needs:

1. The shutdown task must await completion of all jittered closes (e.g. JoinSet / collected handles) before allowing serve()'s return path to reach the end of main — or main must explicitly wait on a drain-complete signal after serve() returns.
2. The >30s cap issue from my earlier comment still stands once the lifetime is fixed.
3. Validation must include a real-process test (spawn the relay, open live WS connections, SIGTERM, assert 1012s arrive spread over the window) — unit tests alone demonstrably pass on a broken implementation.

Credit: live failure isolated by Max (200-conn regression vs 0ms control). My earlier "mechanism itself is sound" was wrong — it was sound only inside a runtime that stays alive.