Skip to content

fix(hotblocks): drain on SIGTERM behind an unready window#102

Merged
mo4islona merged 1 commit into
masterfrom
fix/hotblocks-graceful-shutdown
Jul 22, 2026
Merged

fix(hotblocks): drain on SIGTERM behind an unready window#102
mo4islona merged 1 commit into
masterfrom
fix/hotblocks-graceful-shutdown

Conversation

@mo4islona

@mo4islona mo4islona commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Mirrors sqd-portal#113 for hotblocks.

Why

Replica replacement is client-visible today. At SIGTERM axum's graceful shutdown closes idle keep-alive connections immediately, while the orchestrator's endpoint removal is still propagating — so the portal posts into sockets we have already dropped and gets a 502 burst. Observed 2026-07-21 09:14:10, one second after the old pod's SIGTERM, with a Ready sibling sitting idle.

The drain that follows has no deadline, and nothing bounds a response (GAP-29: a stalled reader pins its own indefinitely), so P-SHUTDOWN is unbounded from our side and the process waits for the orchestrator's kill.

What

Two phases, same shape as the portal:

  1. Pre-drain grace (--pre-drain-grace-secs, default 25) — /ready flips to 503 immediately while everything else keeps serving normally, so the endpoint is withdrawn before anything closes.
  2. Bounded drain (--drain-timeout-secs, default 25) — axum's graceful shutdown raced against a hard deadline; on timeout the listener closes with the serve future and in-flight connections are aborted on exit.

GET /ready is new (there was no readiness signal at all), and it is routed outside the request middleware. Its 503s are a rotation signal, not a fault; inside the middleware every unlabelled 5xx picks up error_class="UNCLASSIFIED", so a kubelet probing through a 25 s grace window would forge an error burst in the same http_status series error-rate alerting reads — on every termination of every pod. SIGINT is deliberately left on the default handler so Ctrl-C in dev still exits at once.

The exit path, which is where the cap is actually won or lost

The deadline only bounds the exit if nothing downstream of it is unbounded. Two things were.

The runtime. main no longer drops it. The destructor joins whichever spawn_blocking job is mid-flight, and per-dataset compactions run back-to-back on that pool — 45 datasets on mainnet, each with its own compaction_loop — so the exit time was set by a chunk merge, not by the deadline. Measured 3.004 s on a 3 s job, linear in the job. shutdown_background() returns at once and joins nothing.

The database close. Backgrounding the runtime is not enough: rocksdb_close waits out RocksDB's own background flush and compaction jobs, so whoever drops the last Arc<Database> inherits the same unbounded tail. So we never close it — the handle is deliberately leaked after a flush.

That makes every exit crash-equivalent, which is fine and already required: CN-6 counts an uncommitted transaction as lost, SIGKILL and OOM-kill exist regardless, and orphaned SSTs from a cut compaction are what --startup-disk-reclaim already purges. What it is not free of is boot cost — an unclosed database replays its WAL — so the memtables are flushed first (Database::flush_all, all seven column families, since a WAL file lives until the last family that wrote into it has flushed). A flush is bounded by memtable size; the close it replaces is not.

Worth noting this is an improvement on the status quo rather than a new tax: with the deployed 5 s grace and a drain floor equal to P-HEAD-WAIT, production has been taking SIGKILL on every rollout for a long time, so the WAL replay is already priced into the observed P-STARTUP-ACCEPT. The flush is the part that was missing. Its cost is logged as memtables flushed elapsed_ms, symmetric with the existing RocksDB opened elapsed_ms, so the trade is visible from both ends on the first rollout.

What this fixes, and what it does not

P-SHUTDOWN is now genuinely bounded from the inside by pre_drain_grace + drain_timeout.

The 502 burst is deferred, not removed. We are reached through a type: ClusterIP Service by a pooled reqwest client, so withdrawing the endpoint does not retire the portal's established connections: conntrack keeps them pinned here, and reqwest's 90 s idle timeout never fires on a socket polled every second. At t=25 the drain closes those same sockets and hits the same race. What shrinks is the blast radius — the pod is out of Endpoints by then, so the reconnect lands on a live sibling instead of bouncing off this one.

The real fix is client-side replay, landing separately in sqd-portal (fix/hotblocks-connection-replay). A server-side Connection: close during the grace window would retire the pooled connections cleanly and leave the replay nothing to do — deliberately not in this PR.

Deploy — required before this does anything

infra branch feat/hotblocks-graceful-shutdown, behind hotblocksDB.gracefulShutdown (default off). The switch turns on both a /ready readinessProbe and terminationGracePeriodSeconds: 60.

Order matters, per stack: build the image → bump image: → only then flip the switch. No stack runs an image with /ready today (mainnet/morpho 250a9db, internal/testnet-dev 7688e0f), and an ungated probe would 404 on every replica and empty the Service.

Until a stack flips it, the 5 s grace sits below pre_drain_grace, so the drain never starts and the pod is SIGKILLed as before — the burst moves from t=0 to t=5 rather than going away. Inert, not an improvement.

Scope

Only half of GAP-17 closes here. Still open: a stream cut at the deadline is a reset rather than an RP-15 end (only we can fix that — the portal cannot replay a response that already emitted bytes); head waiters still sit out their fixed 5 s timeout instead of being released on shutdown (LIV-4); ingest wind-down is untouched, so the original panic-class cancellation path stands.

Not a gap but a recorded trade: the database is never closed cleanly again, by design. The flush keeps the boot cost near zero in principle, but the residue is unmeasured in production until the first rollout.

Spec updated to match: GAP-17 records what closed and what did not, P-SHUTDOWN gets real observed values, /ready is documented in the binding, and IB-5 is tightened to say a truncation is a server-side end rather than a dropped connection.

Test plan

  • cargo test -p sqd-hotblocks — 51 pass, 6 ignored. New: 6 unit (grace-window ordering, clean drain, deadline path, serve-error propagation, /ready flip, no-join-on-exit) and 2 integration.
  • cargo test -p sqd-storage — 74 pass. New: flush_all empties every memtable; ALL_CFS is pinned against what the database actually opens, so a family added later cannot silently stop retiring the WAL.
  • CT-2 ct2_shutdown drives the real binary: unready before the grace elapses, / still 200 during it, exit inside grace + drain with a success status, and a long-poll held in flight across the deadline is aborted rather than answered. Harness Sut::stop split into signal_shutdown + wait_for_shutdown so the sequence can be observed mid-flight.
  • readiness_metrics — probes through the grace window leave no error_class or status="503" series in /metrics.
  • Measured the exit path: drop(runtime) 3.004 s on a 3 s blocking job, shutdown_background() returns at once.
  • Manual SIGTERM, 3 s grace: /ready 503 at t=0.01 s while / stays 200 for the whole window; clean exit 0 at t=3.26 s.
  • Manual SIGINT with a 30 s grace: exits at t=0.01 s, status 130.
  • Read memtables flushed elapsed_ms and RocksDB opened elapsed_ms off the first rollout — the flush has only ever run against near-empty memtables here.
  • Chart PR merged before this image is rolled out.

🤖 Generated with Claude Code

@mo4islona
mo4islona force-pushed the fix/hotblocks-graceful-shutdown branch from 37eadaa to 172fe70 Compare July 21, 2026 11:54
Replica replacement is client-visible today: at SIGTERM the server closes
idle keep-alive connections at once while the endpoint removal is still
propagating, so the portal posts into sockets we have already dropped and
sees a 502 burst — observed one second after SIGTERM with a Ready sibling
idle. The drain that follows has no deadline either, and nothing bounds a
response (GAP-29), so the process waits for the orchestrator's kill rather
than exiting on its own terms.

Same two-phase shape as sqd-portal#113: report unready first and keep
serving through a grace window, so the endpoint is withdrawn before
anything closes; then drain under a hard cap. SIGINT is left on the default
handler, since a dev Ctrl-C must not sit out the grace.

/ready is routed outside the request middleware. Its 503s are a rotation
signal, not a fault, and the middleware classifies every unlabelled 5xx as
error_class="UNCLASSIFIED" — a kubelet probing through the whole grace
window would otherwise forge an error burst on every termination, in the
same series error-rate alerting reads.

That cap only bounds the exit if the runtime does not outlive it. Its
destructor joins whichever blocking job is mid-flight, and per-dataset
compactions run back-to-back on that pool, so a dropped runtime — not the
deadline — was setting the exit time; measured at 3.0s for a 3s job.
Backgrounding it cuts an uncommitted RocksDB transaction at worst, which
CN-6 already counts as lost, but it also means the database is never
closed: rocksdb_close waits out the background compactions and would put
the unbounded tail straight back. So the memtables are flushed instead —
bounded by their size — and the handle is deliberately leaked. Without that
flush the cost would only move to the next boot's WAL replay, on a start
path with a much tighter budget (GAP-7).

Inert until the chart probes /ready and raises the kill timeout above
pre_drain_grace + drain_timeout. Only half of GAP-17 closes here: a stream
cut at the deadline is still a reset rather than an RP-15 end, head waiters
still sit out their fixed timeout (LIV-4), and ingest wind-down is
untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mo4islona
mo4islona force-pushed the fix/hotblocks-graceful-shutdown branch from 172fe70 to 6f424d5 Compare July 22, 2026 07:46
@mo4islona
mo4islona merged commit 912dcb5 into master Jul 22, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant