Conversation
julik
force-pushed
the
stop-processes-whose-heartbeats-block
branch
from
September 28, 2026 14:48
09b7ef5 to
7f0ffd1
Compare
rails#778 stops a process whose heartbeats keep failing, but both of its paths hang off a raise. A heartbeat can also never return at all: Process#heartbeat does a real round-trip, and on a half-open socket to a database that stopped answering, the thread blocks in the driver indefinitely. Nothing is raised, so presumed_dead? is never reached. Concurrent::TimerTask does not help either, because it reschedules the task and notifies its observers only after the task returns - so the heartbeat thread goes quiet permanently, having neither succeeded nor failed. Its timeout_interval is a no-op that warns it was never implementable. So record when the heartbeat last returned, in an ensure so that success and failure both count, and have a second timer stop the process once that goes older than the alive threshold. The watchdog reads nothing but memory, so it cannot block the way the heartbeat it watches can. This only covers the case where the run loop is still able to act on being unregistered. A process whose run loop is itself blocked needs something harsher, which I have deliberately left out of this change.
Same defect as the heartbeat watchdog, one level up. Pruning is how supervisors notice dead processes, and launch_maintenance_task runs it in a Concurrent::TimerTask, so a prune blocked on an unresponsive database stops this supervisor pruning ever again - the mechanism meant to notice dead processes is built from the same material as the processes it watches, and fails at exactly the moment they do. Track when maintenance last returned and stop the supervisor once that goes past STALL_FACTOR times the alive threshold, which leaves a full missed cycle of slack since the task's own interval is the alive threshold. Stopping rather than arranging replacement, because a supervisor cannot replace itself - its run loop breaks on stopped?, so whatever runs it gets to start a new one. The way out must not depend on the database that just stopped answering, and must not abandon the supervised processes. So the stalled stop drops the local registration first, turning shutdown's deregister callback into a no-op instead of a round-trip that would block exactly like the prune did - the stale row is left for another supervisor to prune. And a standalone supervisor stops through its own signal pipeline rather than a bare stop, so that handle_signal pairs stop with terminate_gracefully on the supervise thread and the forks get TERMed instead of orphaned; an embedded supervisor never drains its signal queue, but it already terminates its threads from an after_shutdown hook, so a plain stop is enough there. Supervisors need this separately from the heartbeat watchdog in Registrable: stop_to_be_replaced only unregisters and wakes the loop, and unlike Runnable#shutting_down?, Supervisor#supervise never checks registered?.
A supervisor includes Registrable like any other process, so a pruned registration or a tripped heartbeat watchdog reaches stop_to_be_replaced on it too -- but Registrable's version only deregisters locally and wakes the run loop, counting on the supervisor to notice. Nothing supervises a supervisor, and #supervise breaks only on stopped?, never looking at the registration, so on a supervisor both mechanisms were inert: it would run on as a zombie after the rest of the system had declared it dead and failed its children's claimed executions. Override stop_to_be_replaced on the supervisor to stop it instead, through the same route the maintenance watchdog already uses: the signal pipeline when standalone, so the forks are terminated rather than abandoned, and a plain stop when embedded. The maintenance watchdog now funnels into the same override. Once the registration is dropped locally, prune_dead_processes runs with excluding: nil, which excludes nothing: the supervisor may then prune its own stale row. That is benign -- it is exactly what another supervisor would do to that row -- and only reachable while already stopping, so it is left alone.
The dispatcher's maintenance -- semaphore expiry, unblocking blocked executions, sweeping stalled batches -- runs in a Concurrent::TimerTask, which reschedules only once its task returns. A run blocked on an unresponsive database therefore stops maintenance ever running again, silently: nothing is raised for the task's observer to report, and concurrency-limited jobs stall system-wide while the dispatcher keeps polling as if healthy. Record when a maintenance run last returned, and watch it from a timer that reads nothing but memory, so it cannot block the same way. When the gap exceeds twice the maintenance interval, the dispatcher stops to be replaced: its supervisor starts a fresh dispatcher, maintenance task and all, exactly as it would for a stalled heartbeat.
julik
force-pushed
the
stop-processes-whose-heartbeats-block
branch
from
September 28, 2026 15:35
b370cc0 to
fab72c3
Compare
julik
marked this pull request as ready for review
September 28, 2026 15:51
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(Written by Claude on @julik's behalf)
Speculative, for #808. #778 stops a process whose heartbeats keep failing, but both of its paths hang off a raise, and a heartbeat can also never return at all:
Process#heartbeatis a real round-trip, and on a half-open socket the thread blocks in the driver forever.Concurrent::TimerTaskreschedules only after a task returns, so the heartbeat thread goes quiet having neither succeeded nor failed, andtimeout_intervalis a warned no-op.So record when the call last returned, in an
ensureso both outcomes count, and act on staleness from a secondTimerTaskthat reads only memory. Three of them - process heartbeat, supervisor pruning, and dispatcher maintenance, that last one because a blocked run stops semaphore expiry and stalls concurrency-limited jobs everywhere. Theensureis what keeps this orthogonal to #778: that owns "keeps failing", this owns "stopped coming back".Supervisors needed a commit of their own -
supervisebreaks onstopped?and never looks at the registration, so #778'sRecordNotFoundpath and the heartbeat watchdog were both inert there.In fairness,
keepalives/tcp_user_timeoutindatabase.ymlturns the block into a raise and makes #778 work as designed - that, not this, is what fixed our production. Still uncovered: a fork whose run loop is blocked only exits by signal, which needsexit!after a grace period. I would rather hear your view on that shape before writing it.