[Backend] Add Celery acks_late and a stuck-task watchdog across every async pipeline #398

Closed
opened 2026-08-25 20:44:38 +00:00 by claude-bot · 2 comments
Contributor

Severity: CRITICAL

Found in the August 2026 session lifecycle review (#319).

A routine deploy or an OOM-killed worker permanently strands a session mid-pipeline with no self-service way out, because Celery acknowledges tasks the instant they're received rather than after they finish. If the worker process dies while transcribing a multi-hour recording, Celery never redelivers the task — it's just gone — but the session row was already flipped to processing before the work started, and nothing ever flips it back. The GM's own retry button, and even the admin console's retry button, refuse to act unless the status is failed, so the only way out is a manual database edit.

Evidence

  • webapp/backend/app/tasks/celery_app.py:27-60celery_app.conf.update(...) sets no task_acks_late and no task_reject_on_worker_lost; a repo-wide grep confirms these settings appear nowhere, so Celery's default (ack-on-receipt) applies to every task, including process_audio.
  • webapp/backend/app/tasks/reminder_tasks.py:1984process_audio is declared with max_retries=0, so even a task that does fail with an exception (e.g. a transient Whisper timeout on a 3-hour job) gets no automatic retry.
  • webapp/backend/app/routers/sessions.py:891-897 and webapp/backend/app/routers/admin.py:174-183 — both the GM and admin retry endpoints require audio_processing_status == AudioProcessingStatus.failed; a session stuck at processing 409s on both.
  • webapp/backend/app/tasks/reminder_tasks.py:2705-2712enforce_retention's pass over on-disk audio explicitly continues past any session whose status is neither successfully processed nor failed — it deliberately skips stuck-processing sessions rather than acting on them; no watchdog exists.
  • webapp/backend/app/tasks/reminder_tasks.py:2043-2060 — the WAV-discovery/speakers.json parsing code (which can raise FileNotFoundError or a JSON error) runs before async with task_session() as db: opens, so an exception there never reaches the status-setting failure path either — same stuck-processing outcome from a different cause.
  • The same failure class is confirmed for the lore pipeline (extracting/matching after a SIGKILL — the UI hides the retry button for every in-progress state) and for wiki drafts (generating with no failure path, only a destructive discard_draft escape) and Workbench GenerationResult (stuck pending, frontend gives up polling after ~60s).

Failure scenario
An operator runs the routine docker compose up -d --build deploy while a 3-hour transcription job is mid-flight on the worker. The container is sent SIGTERM/SIGKILL, the in-flight process_audio task vanishes with it, and the session (already flipped to processing on intake) never moves again. The GM sees "Processing…" forever; clicking retry 409s; admin's retry 409s too. The only fix is a support engineer running a manual UPDATE sessions SET audio_processing_status = 'failed' WHERE id = ... so the retry button lights up.

Proposed fix
Set task_acks_late=True and task_reject_on_worker_lost=True in celery_app.conf for process_audio and the lore/draft/workbench tasks. This is safe only once the "[Recording] Guard against re-submitting audio..." issue lands, since acks_late means a task can now be redelivered and re-run — without that guard in place first, a redelivery would clobber edits exactly like an unguarded manual re-submission would. Add a Celery Beat watchdog task that scans for processing/pending/generating/extracting/matching rows older than a configurable threshold (e.g. 3-6 hours) and flips them to failed with a "worker lost" error, which makes the existing retry buttons work for this class of failure without new UI. Separately, raise process_audio's max_retries above 0 for genuinely transient exceptions (network/timeout), distinct from permanent ones (bad path, malformed input). (This watchdog is generalised across all four pipelines in the "[Backend] Build a generic stuck-task watchdog..." issue; this issue is the audio-pipeline-specific defect that motivates it.)

Acceptance criteria

  • task_acks_late=True and task_reject_on_worker_lost=True are set for process_audio.
  • This change ships only after the re-submission state guard (tracked in the "[Recording] Guard against re-submitting audio..." issue) is in place.
  • A Beat watchdog transitions stale processing sessions older than a configured threshold to failed with a descriptive error.
  • The existing GM/admin retry endpoints succeed against sessions the watchdog has flipped to failed, with no manual SQL required.
  • process_audio retries at least once on transient exceptions instead of max_retries=0.
  • Regression test simulates a worker loss (task never acks/never completes) and asserts the watchdog eventually unsticks the session.

Scope: every pipeline, not just process_audio

Merged from a companion finding. The same "worker died, no exit" shape appears in four independent pipelines, so the fix should be shared infrastructure rather than four bespoke patches:

  • Audio processingprocess_audio, status processing
  • Lore generation — statuses extracting / matching, with no on_failure coverage for a SIGKILL
  • Wiki draft generationgenerate_lore_entry_draft has no on_failure base at all
  • WorkbenchGenerationResult rows have no server-side timeout; only the frontend gives up polling

Apply task_acks_late and task_reject_on_worker_lost consistently across all of them, and add a single Celery Beat watchdog that scans for any row in an in-progress state (processing, pending, extracting, matching, generating) older than a configurable threshold and transitions it to a terminal failed state with a "worker lost" error.

That one mechanism closes the majority of the eleven enumerated stuck states at once.

Ordering constraint: the audio re-submission guard must land first. Enabling acks_late means tasks get redelivered after a worker loss, and until re-submission is guarded a redelivery will clobber GM-edited transcripts, summaries and curated highlights.

Added acceptance criteria

  • task_acks_late and task_reject_on_worker_lost applied to all four pipelines, not only process_audio
  • A generic watchdog covers all five in-progress states
  • The watchdog's age threshold is configurable and its actions are logged and auditable
  • Re-submission guard confirmed landed before acks_late is enabled
**Severity: CRITICAL** Found in the August 2026 session lifecycle review (#319). A routine deploy or an OOM-killed worker permanently strands a session mid-pipeline with no self-service way out, because Celery acknowledges tasks the instant they're received rather than after they finish. If the worker process dies while transcribing a multi-hour recording, Celery never redelivers the task — it's just gone — but the session row was already flipped to `processing` before the work started, and nothing ever flips it back. The GM's own retry button, and even the admin console's retry button, refuse to act unless the status is `failed`, so the only way out is a manual database edit. **Evidence** - `webapp/backend/app/tasks/celery_app.py:27-60` — `celery_app.conf.update(...)` sets no `task_acks_late` and no `task_reject_on_worker_lost`; a repo-wide grep confirms these settings appear nowhere, so Celery's default (ack-on-receipt) applies to every task, including `process_audio`. - `webapp/backend/app/tasks/reminder_tasks.py:1984` — `process_audio` is declared with `max_retries=0`, so even a task that *does* fail with an exception (e.g. a transient Whisper timeout on a 3-hour job) gets no automatic retry. - `webapp/backend/app/routers/sessions.py:891-897` and `webapp/backend/app/routers/admin.py:174-183` — both the GM and admin retry endpoints require `audio_processing_status == AudioProcessingStatus.failed`; a session stuck at `processing` 409s on both. - `webapp/backend/app/tasks/reminder_tasks.py:2705-2712` — `enforce_retention`'s pass over on-disk audio explicitly `continue`s past any session whose status is neither successfully processed nor `failed` — it deliberately skips stuck-`processing` sessions rather than acting on them; no watchdog exists. - `webapp/backend/app/tasks/reminder_tasks.py:2043-2060` — the WAV-discovery/`speakers.json` parsing code (which can raise `FileNotFoundError` or a JSON error) runs before `async with task_session() as db:` opens, so an exception there never reaches the status-setting failure path either — same stuck-`processing` outcome from a different cause. - The same failure class is confirmed for the lore pipeline (`extracting`/`matching` after a SIGKILL — the UI hides the retry button for every in-progress state) and for wiki drafts (`generating` with no failure path, only a destructive `discard_draft` escape) and Workbench `GenerationResult` (stuck `pending`, frontend gives up polling after ~60s). **Failure scenario** An operator runs the routine `docker compose up -d --build` deploy while a 3-hour transcription job is mid-flight on the worker. The container is sent SIGTERM/SIGKILL, the in-flight `process_audio` task vanishes with it, and the session (already flipped to `processing` on intake) never moves again. The GM sees "Processing…" forever; clicking retry 409s; admin's retry 409s too. The only fix is a support engineer running a manual `UPDATE sessions SET audio_processing_status = 'failed' WHERE id = ...` so the retry button lights up. **Proposed fix** Set `task_acks_late=True` and `task_reject_on_worker_lost=True` in `celery_app.conf` for `process_audio` and the lore/draft/workbench tasks. This is safe only once the "[Recording] Guard against re-submitting audio..." issue lands, since acks_late means a task can now be redelivered and re-run — without that guard in place first, a redelivery would clobber edits exactly like an unguarded manual re-submission would. Add a Celery Beat watchdog task that scans for `processing`/`pending`/`generating`/`extracting`/`matching` rows older than a configurable threshold (e.g. 3-6 hours) and flips them to `failed` with a "worker lost" error, which makes the existing retry buttons work for this class of failure without new UI. Separately, raise `process_audio`'s `max_retries` above 0 for genuinely transient exceptions (network/timeout), distinct from permanent ones (bad path, malformed input). (This watchdog is generalised across all four pipelines in the "[Backend] Build a generic stuck-task watchdog..." issue; this issue is the audio-pipeline-specific defect that motivates it.) **Acceptance criteria** - [ ] `task_acks_late=True` and `task_reject_on_worker_lost=True` are set for `process_audio`. - [ ] This change ships only after the re-submission state guard (tracked in the "[Recording] Guard against re-submitting audio..." issue) is in place. - [ ] A Beat watchdog transitions stale `processing` sessions older than a configured threshold to `failed` with a descriptive error. - [ ] The existing GM/admin retry endpoints succeed against sessions the watchdog has flipped to `failed`, with no manual SQL required. - [ ] `process_audio` retries at least once on transient exceptions instead of `max_retries=0`. - [ ] Regression test simulates a worker loss (task never acks/never completes) and asserts the watchdog eventually unsticks the session. --- ## Scope: every pipeline, not just `process_audio` Merged from a companion finding. The same "worker died, no exit" shape appears in four independent pipelines, so the fix should be shared infrastructure rather than four bespoke patches: - **Audio processing** — `process_audio`, status `processing` - **Lore generation** — statuses `extracting` / `matching`, with no `on_failure` coverage for a SIGKILL - **Wiki draft generation** — `generate_lore_entry_draft` has no `on_failure` base at all - **Workbench** — `GenerationResult` rows have no server-side timeout; only the frontend gives up polling Apply `task_acks_late` and `task_reject_on_worker_lost` consistently across all of them, and add a single Celery Beat watchdog that scans for any row in an in-progress state (`processing`, `pending`, `extracting`, `matching`, `generating`) older than a configurable threshold and transitions it to a terminal `failed` state with a "worker lost" error. That one mechanism closes the majority of the eleven enumerated stuck states at once. **Ordering constraint:** the audio re-submission guard must land *first*. Enabling `acks_late` means tasks get redelivered after a worker loss, and until re-submission is guarded a redelivery will clobber GM-edited transcripts, summaries and curated highlights. ## Added acceptance criteria - [ ] `task_acks_late` and `task_reject_on_worker_lost` applied to all four pipelines, not only `process_audio` - [ ] A generic watchdog covers all five in-progress states - [ ] The watchdog's age threshold is configurable and its actions are logged and auditable - [ ] Re-submission guard confirmed landed before `acks_late` is enabled
Author
Contributor

Implemented in PR #448 (fix/398-survive-a-lost-worker), awaiting CI.

Precondition confirmed first: #397's re-submission guard is in place (reminder_tasks.py:2175), so a redelivered process_audio refuses to overwrite an existing transcript instead of clobbering GM edits. acks_late would have been actively unsafe before that landed.

One thing the issue didn't specify, shipped in the same change. There were no Celery time limits configured at all. With acks_late and no limit, a task that kills its worker is redelivered and kills the next one, forever — the fix would have traded a stuck session for an unavailable worker pool. So task_soft_time_limit=6h / task_time_limit=6h10m and worker_prefetch_multiplier=1 are part of this, not a follow-up. Six hours sits just above max_recording_hours, so it bounds pathology without ever cutting a legitimate full-length transcription.

Watchdog scope: hourly, 8h threshold (above the 6h task limit, so anything past it cannot still be running). Covers all four pipelines that flip a row to in-progress on intake — audio processing, lore pending/extracting/matching, LoreEntryDraft.generating, GenerationResult pending. It only ever moves rows to failed, since that is the one state the retry endpoints accept; no files are touched and audio_file_path is left intact, so the recording is still there to reprocess. Writes a tasks.unstuck audit entry with counts.

process_audio also retries twice with backoff on transport errors specifically — not a blanket Exception, which would only delay the honest error the admin panel needs to show.

Verification: mutation-checked both halves — reverting acks_late fails 1 test, stubbing the watchdog fails 3 including this issue's own scenario. The idempotency test initially passed against the stub (vacuously — "the second pass changed nothing" is also true of a watchdog that never ran) and was tightened to assert the first pass did the work. 1,417 backend tests pass, up from 1,409.

Will close once CI is green and this is merged.

Implemented in PR #448 (`fix/398-survive-a-lost-worker`), awaiting CI. **Precondition confirmed first:** #397's re-submission guard is in place (`reminder_tasks.py:2175`), so a redelivered `process_audio` refuses to overwrite an existing transcript instead of clobbering GM edits. `acks_late` would have been actively unsafe before that landed. **One thing the issue didn't specify, shipped in the same change.** There were **no Celery time limits configured at all**. With `acks_late` and no limit, a task that kills its worker is redelivered and kills the next one, forever — the fix would have traded a stuck session for an unavailable worker pool. So `task_soft_time_limit=6h` / `task_time_limit=6h10m` and `worker_prefetch_multiplier=1` are part of this, not a follow-up. Six hours sits just above `max_recording_hours`, so it bounds pathology without ever cutting a legitimate full-length transcription. **Watchdog scope:** hourly, 8h threshold (above the 6h task limit, so anything past it cannot still be running). Covers all four pipelines that flip a row to in-progress on intake — audio `processing`, lore `pending`/`extracting`/`matching`, `LoreEntryDraft.generating`, `GenerationResult` `pending`. It only ever moves rows to `failed`, since that is the one state the retry endpoints accept; no files are touched and `audio_file_path` is left intact, so the recording is still there to reprocess. Writes a `tasks.unstuck` audit entry with counts. `process_audio` also retries twice with backoff on transport errors specifically — not a blanket `Exception`, which would only delay the honest error the admin panel needs to show. **Verification:** mutation-checked both halves — reverting `acks_late` fails 1 test, stubbing the watchdog fails 3 including this issue's own scenario. The idempotency test initially passed against the stub (vacuously — "the second pass changed nothing" is also true of a watchdog that never ran) and was tightened to assert the first pass did the work. 1,417 backend tests pass, up from 1,409. Will close once CI is green and this is merged.
Author
Contributor

Merged in PR #448 (CI green). Closing.

One addition after the first mutation run: the terminal-row test walked all four watchdog queries, but only the audio and lore assignments ever executed — a wrong column name in the draft or workbench branch would have passed CI and failed the first time a worker actually died. Added two tests that seed a stranded draft and a stranded generation result, which also assert the accumulated work survives (draft body, iteration_count, generation params) since preserving it is the reason the watchdog fails rows instead of discarding them. Stubbing the watchdog now fails five tests rather than three.

Final: 1,419 backend tests pass.

Merged in PR #448 (CI green). Closing. One addition after the first mutation run: the terminal-row test walked all four watchdog queries, but only the audio and lore *assignments* ever executed — a wrong column name in the draft or workbench branch would have passed CI and failed the first time a worker actually died. Added two tests that seed a stranded draft and a stranded generation result, which also assert the accumulated work survives (draft body, `iteration_count`, generation params) since preserving it is the reason the watchdog fails rows instead of discarding them. Stubbing the watchdog now fails five tests rather than three. Final: 1,419 backend tests pass.
Sign in to join this conversation.
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rbrooks/Quest-Board#398
No description provided.