fix(celery): dead-letter OOM-loop tasks by keying worker-loss cap on stable Mongo task id - #1296
fix(celery): dead-letter OOM-loop tasks by keying worker-loss cap on stable Mongo task id#1296ocervell wants to merge 1 commit into
Conversation
…er OOM loops A task whose worker OOM-kills or is evicted is redelivered by the broker under task_acks_late + task_reject_on_worker_lost. The redelivery cap in run_command counted deliveries keyed on self.request.id, assuming the Celery id is stable across redeliveries. It is not for the fan-in zombies seen in prod: those redelivered via pure broker redelivery (redelivered=true, retries=0) presenting a fresh/rotating Celery id each cycle, so bump_worker_loss_count reset to 1 every delivery and the cap never tripped -> one worker pod OOM-killed per cycle, forever, even after the parent scan FAILED. Key the counter on the stable Mongo task id instead (worker_loss_key: prefer context['task_chunk_id'] / context['task_id'] minted at on_build and baked into the serialized signature, which rides inside the message body and is identical on every redelivery; fall back to the Celery id for bare tasks with no doc, preserving prior behaviour). The count now accumulates on the logical task regardless of Celery-id rotation and abandon_task dead-letters it after N deliveries, forwarding results so the surrounding chord still completes. The bump runs first thing in run_command, before forward_results/task setup, so it counts even when the OOM happens during setup. Test: same logical task redelivered with differing Celery ids abandons on the 5th delivery (max_retries=3), and asserts the old per-Celery-id keying never accumulates past 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MtTyzcUmPYxM5nfp7MnMVd
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughWorker-loss retry counting now uses stable task or chunk identifiers from task context, falling back to the Celery request ID. Redelivery tests cover identifier priority and abandonment after the configured cap despite rotating Celery IDs. ChangesWorker-loss retry identity
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem (RC#6 — dead-letter broker-redelivery OOM loops)
A task that OOM-kills its worker (or is node-evicted) has its message requeued by the broker under
task_acks_late+task_reject_on_worker_lost, and re-run. Secator already caps this inrun_commandviabump_worker_loss_count+abandon_task, gated ontask_max_retries— but the counter was keyed onself.request.id, assuming the Celery id is stable across redeliveries.For the fan-in "zombies" seen in prod (52,337 and 303,499-result fan-ins), that assumption broke: they redelivered via pure broker redelivery (
redelivered=true, retries=0) presenting a fresh/rotating Celery id each cycle. Sobump_worker_loss_count(self.request.id)reset to1on every delivery, the cap never tripped, and one worker pod was OOM-killed per cycle indefinitely — even after the parent scan had FAILED.Fix
Key the counter on the stable Mongo task id instead of the rotating Celery id:
worker_loss_key(context, celery_id)→ preferscontext['task_chunk_id']/context['task_id'](minted aton_build, baked into the child signature's serializedcontext, so it rides inside the message body and is identical on every redelivery). Falls back to the Celery id for bare top-level tasks that never went throughon_build(those keep a stable Celery id underreject_on_worker_lost, so prior behaviour is preserved).abandon_taskdead-letters it after N deliveries — forwarding results (forward_results+chain_results) so the surrounding chord/workflow still completes instead of hanging.run_command, beforeforward_results/task setup, so it counts even when the OOM happens during setup.No config or behavioural change when the cap is disabled (
task_max_retries=-1) or when a task already keeps a stable Celery id.Native-Celery note (for the reader)
Celery has no built-in delivery-count / dead-letter cap for the Redis broker.
task_reject_on_worker_lostonly re-queues on worker loss (it does not count or cap);Task.retry()is a different, in-task path (retriesheader) not involved here (retries=0); AMQPx-death/ dead-letter exchanges are RabbitMQ-only and unavailable on the Redis broker + result-backend counting secator uses. So the counting + cap must live in application code — this PR keeps it there and only corrects the key. See the postmortem RC#6 research summary for detail.Test
tests/unit/test_celery.py::TestWorkerLossRetryCap:test_redeliveries_with_rotating_celery_ids_abandon_after_cap— same logical task redelivered 7× with a different Celery id each time abandons on the 5th delivery (max_retries=3), and asserts the old per-Celery-id keying never accumulates past1.test_worker_loss_key_prefers_mongo_id_over_rotating_celery_id— precedence: chunk id → task id → Celery id fallback.secator test unit --test test_celery→ 25 passed, 2 skipped.secator test lintclean.🤖 Generated with Claude Code
https://claude.ai/code/session_01MtTyzcUmPYxM5nfp7MnMVd
Summary by CodeRabbit
Bug Fixes
Tests