fix(celery): reschedule in-flight tasks on worker shutdown#488
Merged
Conversation
When a worker is reclaimed mid-step (SIGTERM / spot-node drain), its in-flight task message was only redelivered after the Redis broker visibility_timeout (~1h, time-based). The parent workflow stayed SUSPENDED that whole time, so the user-facing flow silently hung with no output. Re-enqueue a worker's in-flight tasks the moment its broker consumer is torn down, so a live worker's blocking BRPOP resumes them within seconds (event-based) instead of waiting out visibility_timeout. How it works: - Children record what they're executing in a per-pod Redis hash (pyworkflow:inflight:<pod>) on task_prerun, and clear it on task_postrun. - A consumer bootstep (RescheduleConsumerStep, requires Tasks) fires as the consumer is cancelled during shutdown. There is no Celery signal for that instant: worker_shutting_down fires before the consumer is cancelled (so an immediate re-publish gets self-re-grabbed back into `unacked`), and worker_shutdown fires only after the pool drains the long task (too late). - The bootstep cancels this worker's task consumer itself (guaranteeing it stops fetching), then re-enqueues: it releases the singleton lock and re-publishes to the same queue, so the message lands in the *ready* list for another worker rather than this dying one. - Guarded by celery.worker.state.should_stop/should_terminate (identity checks, since the flag is the exit code and 0 == False) so it only runs on real shutdown, not a transient consumer restart (broker reconnect). Safe against double execution: only SingletonWorkflowTask tasks with unique_on are rescheduled, and execute_step_task short-circuits on an existing STEP_COMPLETED event. Gated by PYWORKFLOW_RESCHEDULE_ON_SIGTERM (default on). Tests: unit (logic + bootstep wiring + state guard), integration against real Redis (registry hash, lock release/re-acquire, real broker enqueue lands in the ready list, dedup-bypass), and an end-to-end test with real prefork worker subprocesses + real SIGTERM that proves a fresh worker resumes the workflow in seconds (broker visibility_timeout is 3600s, so completion proves the reschedule did it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fixes the CI "Run Ruff formatter check" failure (ruff format --check). No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Problem
When a worker is reclaimed mid-step (SIGTERM / AWS spot-node drain), its in-flight task message was only redelivered after the Redis broker
visibility_timeout(~1h, time-based). The parent workflow stayedSUSPENDEDthat whole time, so the user-facing flow silently hung with no output.Observed in production: a chatbot
DeepAgentrun satcancelled/suspendedfor ~2h withrecovery_attempts: 0, because the dispatched step's worker was reclaimed and nothing re-drove the run until the visibility window elapsed.Fix
Re-enqueue a worker's in-flight tasks the moment its broker consumer is torn down, so a live worker's blocking
BRPOPresumes them within seconds (event-based) instead of waiting outvisibility_timeout.pyworkflow:inflight:<pod>ontask_prerun, and clear it ontask_postrun.RescheduleConsumerStep,requiresTasks) fires as the consumer is cancelled during shutdown. There is no Celery signal for that instant:worker_shutting_downfires before the consumer is cancelled — an immediate re-publish gets self-re-grabbed back into Redisunacked.worker_shutdownfires only after the pool drains the long task — too late.celery.worker.state.should_stop/should_terminate(identity checks — the flag is the exit code and0 == False) so it only runs on real shutdown, not a transient consumer restart (broker reconnect).Safe against double execution: only
SingletonWorkflowTasktasks withunique_onare rescheduled, andexecute_step_taskshort-circuits on an existingSTEP_COMPLETEDevent. Gated byPYWORKFLOW_RESCHEDULE_ON_SIGTERM(default on).Tests
tests/unit/test_reschedule.py, +test_singleton.py): tracking rules, re-enqueue/lock-release, queue resolution, env gate, the consumer-stop hook + the shutdown-state guard, andrelease_lockkey reproduction.tests/integration/test_reschedule.py): registry hash round-trip, real singleton lock release + re-acquire, real broker enqueue lands in the ready list, and the dedup-bypass proof.tests/integration/test_reschedule_e2e.py): boots real prefork worker subprocesses, runs a blocking step, sends a realSIGTERM, hard-kills that worker so only a fresh worker can finish, and asserts the workflowcompletedwithin seconds. The brokervisibility_timeoutis 3600s, so completion proves the reschedule did it — not redelivery. (@integration/@slow, skipped without Redis.)All green;
ruffclean.🤖 Generated with Claude Code