Skip to content

feat: restart the process when a critical background task dies - #678

Merged
MoonBoi9001 merged 18 commits into
mainfrom
mb9/restart-service-when-a-background-task-dies
Jul 23, 2026
Merged

feat: restart the process when a critical background task dies#678
MoonBoi9001 merged 18 commits into
mainfrom
mb9/restart-service-when-a-background-task-dies

Conversation

@MoonBoi9001

@MoonBoi9001 MoonBoi9001 commented Jul 20, 2026

Copy link
Copy Markdown
Member

This PR makes dipper-service exit and restart when any of its background tasks dies unexpectedly, instead of carrying on without it. The service runs about 12 long-lived tasks in one tree (the job worker, admin RPC, chain listener, health endpoint, and the rest); today a task death is logged and forgotten while the process keeps looking healthy from outside, so the service can sit degraded for weeks. A new supervisor treats any unexpected task exit as fatal: it drives the same ordered shutdown a termination signal does, then exits non-zero naming the dead task so Kubernetes restarts the pod.

The teardown is time-bounded: each stop step, the lifecycle-event flush, and the database pool close are capped at 5 seconds, with a 75-second watchdog over the whole drain that aborts and still exits non-zero if anything wedges. The health endpoint stops last so the liveness probe keeps answering through a teardown and a clean shutdown does not read as a probe failure, and the deployment's termination grace period rises to 90 seconds so Kubernetes never SIGKILLs a teardown in progress.

A long-running task that stopped early (a panic or an error return) left the process running
with that task dead: a silent stall no probe caught. It now treats such an exit as fatal, tears
down the rest of the task tree cleanly, and exits non-zero so the orchestrator restarts it.
While shutting down, the supervisor waited without limit for each task, so if the one running
the stop sequence died first the process hung for good; it now gives up after a grace period
with no progress, and a task that fails or panics mid-teardown exits the process non-zero.
The drain picked between its bounded and unbounded wait once per loop, so the shutdown flag
was only re-read after a task completed. A stop sequence that wedged on its first step left
the drain parked forever and the 120-second watchdog never ran. Wake on the flag instead.
The existing stall test lets its stop-sequence stand-in return, which wakes the drain and
lets it notice shutdown, so it passed even while the watchdog could not start on its own.
This variant wedges that stand-in too, leaving the shutdown flag as the only wake-up.
Failing to register the SIGTERM and SIGINT handlers only logged an error before running the
normal stop sequence and returning success, so a host that cannot deliver signals shut the
service down looking healthy. Carry the failure through and report it once everything stops.
The process exited with a flat "a critical task exited unexpectedly" no matter what happened,
so the last line before a restart never said which task died or how. Record the first failure
as it is harvested and report it, keeping the task id and the underlying error chain.
When the watchdog gives up it aborts the stop sequence partway through, so the buffered
lifecycle events never reach the broker and the database pool never closes cleanly. Neither
the log line nor the flush step said so, leaving an operator to work it out from the code.
The module preamble and the supervise doc explained the same design twice, at 14 and 20 lines
against a 3-line limit. Keep each point once, next to the code it explains, moving the reason
the teardown is time-boxed down to the wait itself.
Both ran detached, so if either died the service carried on with no signal: the cache would
serve stale entity counts to fee estimation, and the refresher would freeze the contract
domain used to sign agreements. They now sit in the same supervised tree as everything else.
Every stop step waited indefinitely for its service, so one wedged component could hold the
whole sequence past the pod's grace period and get the process killed mid-teardown. Each step
now gives up after 5 seconds and names the service, putting a ceiling on the sequence.
Without the field Kubernetes applies its 30-second default, which is below what an unlucky
shutdown can take, so the process could be killed part-way through. Allow 90 seconds, leaving
headroom over the service's own bounded teardown so it always exits on its own terms.
Kubernetes probes /health for the whole life of the pod, so tearing that endpoint down at the
start of the stop sequence made every clean shutdown look like a liveness failure. It now stops
after every other service, leaving the probe answering 200 until the process is nearly gone.
The 30s watchdog restarted on every task that finished, so a slow tail of stragglers could
stretch the drain with no end, while a healthy stop sequence taking its full 65s was declared
dead. One deadline now covers the drain, set to 75s, and the stall log claims only what it saw.
The note above terminationGracePeriodSeconds counted 9 stop steps where the sequence has 11, and
treated that sequence as the ceiling on how long the process lives. Draining the task tree runs
on afterwards, so the honest bound is the 75s budget covering the whole teardown.
The one route from a failed signal-handler registration to a non-zero exit is the supervisor
recording a task that returned an error, and nothing tested that arm. Two tests now cover it,
before and during shutdown, so a refactor cannot quietly ship a pod that ignores a stop signal.
When a task death drives shutdown, the branch that wins drops the signal listener, and the
signal library unregisters its handlers without restoring the default. A SIGTERM during the
teardown is then ignored; the process exits within its budget anyway, so the window is left.
The teardown watchdog assumes each step is bounded by the 5s stop-step timeout, but the
event flush waited on a separate 5s limit inside the producer crate. Raising that limit
would push shutdown past the watchdog, so wrap the flush in the same local cap.
@MoonBoi9001
MoonBoi9001 merged commit 432b567 into main Jul 23, 2026
11 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