Skip to content

Retry finalizing claimed executions on transient errors - #788

Open
wintan1418 wants to merge 1 commit into
rails:mainfrom
wintan1418:retry-claimed-execution-finalization
Open

Retry finalizing claimed executions on transient errors#788
wintan1418 wants to merge 1 commit into
rails:mainfrom
wintan1418:retry-claimed-execution-finalization

Conversation

@wintan1418

Copy link
Copy Markdown
Contributor

Fixes #748

Problem

When a job completes, the worker finalizes it: a small transaction that deletes the ClaimedExecution and marks the Job finished (or failed). If that transaction raises — e.g. a transient DB connection drop, exactly the scenario reproduced in #748 — the error is swallowed by the pool's thread error handler and the claimed execution stays claimed forever:

  • Every recovery mechanism (release on deregistration, failing orphaned executions on supervisor boot, failing pruned processes' claims) assumes the claiming process is gone.
  • Here the worker is alive and healthy, so nothing ever picks the execution up again — even though the job already ran to completion.

Fix

Finalization is idempotent: it takes a row lock on the claimed execution and no-ops if it's already gone (unless_already_finalized). That makes it safe to retry. ClaimedExecution#perform now retries the finalization of both outcomes (finished and failed) through a few increasingly spaced attempts (0.5s, 1s, 2s, 4s) before giving up and letting the error propagate as before.

This handles transient instability, which is the case reported in #748 (and what its reproduction script simulates). A persistent DB outage is a different problem with different machinery (#549, #683): once nothing can talk to the database, heartbeats fail too and the process-pruning recovery path takes over when connectivity returns.

Each retry emits a retry_finalization.solid_queue event (logged at warn) with the job, process, attempt number, and error, so operators can see the instability instead of it being invisible.

Tests

  • Finishing and failing paths each retry past a simulated transient ActiveRecord::ConnectionNotEstablished and finalize correctly on the next attempt.
  • When retries are exhausted, the error propagates as before (no behavior change for persistent failures).
  • Log subscriber coverage for the new event.

All three regression tests reproduce the stuck-claimed state on main and pass with the fix.

When a job finishes, the worker deletes its claimed execution and marks
the job finished (or failed) in a small transaction. If that transaction
raises — say a transient DB connection drop — the error is swallowed by
the pool's thread error handler and the claimed execution stays claimed
forever: every recovery mechanism (releasing on deregistration, failing
orphaned or pruned claims) assumes the claiming process is gone, but this
worker is alive and well, so nothing ever picks the execution up again,
even though the job already ran to completion.

Finalization is idempotent — it locks the claimed execution row and
no-ops if it's already gone — so it's safe to retry. Wait out the hiccup
with a few increasingly spaced attempts before giving up and letting the
error propagate as before. Each retry emits a retry_finalization.solid_queue
event, logged at warn level, so operators can see the instability.

Fixes rails#748
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.

ClaimedExecution can be stuck in claimed state forever when #finished raises

1 participant