Replace terminated forks even if releasing their claimed jobs fails - #781
Open
wintan1418 wants to merge 1 commit into
Open
Replace terminated forks even if releasing their claimed jobs fails#781wintan1418 wants to merge 1 commit into
wintan1418 wants to merge 1 commit into
Conversation
When the database restarts under a running fork supervisor, the workers and dispatcher crash and exit. The supervisor reaps them and calls replace_fork, which released the terminated fork's claimed jobs — a database call — before starting the replacement. With the database still down that call raised, the exception crashed the supervise loop, and the terminated forks were never replaced: the scheduler (which survives enqueue errors) kept enqueueing recurring jobs that nothing would ever process, until the whole thing was restarted by hand. Rescue errors from the release, report them through handle_thread_error and start the replacement regardless. The database being unreachable at that point is likely the very reason the fork died, so replacing it can't depend on the database being back. The terminated fork's claimed jobs aren't lost: its stale registration gets pruned once the database is reachable again, which fails them through the regular path so they can be retried. Fixes rails#683
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
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.
Fixes #683
Problem
When the database restarts under a running fork supervisor, the workers and dispatcher crash and exit (their polling loops raise). The supervisor reaps them and calls
replace_fork, which runsrelease_claimed_jobs_by— a database call — before starting the replacement. With the database still down, that call raises, the exception escapescheck_and_replace_terminated_processes, and the wholesuperviseloop crashes without ever replacing the terminated forks.The scheduler, whose per-task enqueue errors are reported without killing the process, survives and reconnects — so after the database comes back, recurring jobs keep being enqueued with no workers left to run them. I reproduced this end-to-end on
mainwithdocker compose restart mysqlunderbin/jobs: a growing backlog of ready executions, worker heartbeats frozen at the restart timestamp, and a supervisor that never recovers. Full chain of evidence — including the recovered crash backtrace and why the process hangs instead of exiting when thedebuggem is loaded — is written up in a comment on #683.Fix
Rescue errors from
release_claimed_jobs_byinreplace_fork, report them throughhandle_thread_error, and start the replacement regardless. The database being unreachable at that moment is likely the very reason the fork died, so the replacement can't depend on it.The terminated fork's claimed jobs aren't lost by skipping the release: its stale process registration is pruned once the database is reachable again, and pruning fails its claimed executions through the normal
ProcessPrunedErrorpath.Testing
replace_forkstill starts the replacement when releasing claimed jobs raisesConnectionNotEstablished(errors without this change, passes with it).bin/jobsunder adocker compose restart mysqlrecovers by itself — replacement workers crash-loop through the boot-timeout path while the database is down, then re-register and drain the backlog as soon as it's back. Onmain, the same scenario ends with a permanently dead supervisor loop and an ever-growing backlog.