Prevent false worker pruning from leaking concurrency permits#11
Prevent false worker pruning from leaking concurrency permits#11tblais1224 wants to merge 3 commits into
Conversation
* Keep stale supervised workers while their supervisor heartbeat is healthy * Make claimed execution finalization own and release permits exactly once * Add regression coverage and document pruning behavior
How the
|
jpcamara
left a comment
There was a problem hiding this comment.
I’ve left the requested review feedback inline, with proposed fixes on each relevant line.
* Split unblock_next_blocked_job into release_concurrency_permit and promote_next_blocked_job so promotion runs after the claim transaction commits, matching the failure isolation from 873760d * Returning the permit inside the claim transaction keeps ownership atomic; promoting outside it means a promotion failure can no longer roll back a job that already finished or failed, and avoids holding the semaphore lock while waiting on the blocked row's lock
* Deregister a supervised process's row as soon as its supervisor reaps it, mirroring Process::Prunable#prune, so a dead child no longer stays permanently protected from pruning just because its supervisor is alive * Update the two existing tests whose expected process counts depended on that dead row lingering * Add non-transactional, multi-connection tests that exercise the actual FOR UPDATE serialization in ClaimedExecution finalization: both race outcomes (pruner wins, performer wins) across finish/fail/release paths, and a use_skip_locked=false regression guarding against the reverse lock order between the semaphore and the blocked row
What does this PR do? (required)
Prevents a stale Solid Queue heartbeat from turning one concurrency-limited execution into multiple overlapping executions and permanently corrupting the semaphore count.
Incident and failure sequence
The production investigation started with
Sync::ExternalCalendar::SyncJob, configured with a concurrency limit of one, running up to 11 times concurrently for the same calendar configuration.One confirmed execution followed this sequence:
SSL error: unexpected eof while reading.process_alive_threshold, another supervisor pruned the worker record as dead.ClaimedExecutionfinalized the job and returned the same permit again.The immediate database/network cause of the SSL EOF remains unknown. This PR instead makes that transient heartbeat failure safe.
Safeguards
Corroborate worker liveness with its supervisor
A supervised worker with a stale heartbeat is no longer pruned while its owning supervisor has a fresh heartbeat. The supervisor already detects actual fork or thread exits directly and fails those claims with
ProcessExitError, which is stronger evidence than a failed database heartbeat.If both worker and supervisor heartbeats become stale, the existing pruning recovery remains available. This intentionally favors preventing concurrent side effects while the owning supervisor is known to be alive.
Make terminal claim ownership exact-once
Finishing, failing, and graceful release now lock the claimed-execution row before changing job state. Only the actor that still owns that row can delete the claim or return its concurrency permit.
Job state changes, claim deletion, semaphore signaling, and blocked-job promotion occur in one transaction. If pruning already removed the claim, the stale in-memory performer performs no terminal bookkeeping and cannot signal the semaphore a second time.
Together, these changes prevent the observed false prune when the supervisor remains healthy and prevent permanent concurrency amplification even if pruning races with a live completion.
Link to Basecamp to-do, Trello card, New Relic or Honeybadger (required)
N/A — production investigation for external calendar configuration
58070.QA
What platforms should be included in QA?
QA steps
Automated regression coverage verifies:
Verification completed:
Screenshots (if appropriate)
N/A
Docs
The process lifecycle documentation now explains that a stale supervised process is protected while its supervisor heartbeat remains fresh.