From 258c77dfcd0edc4797ab05930c8c40718aadc612 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 16 Jun 2026 20:54:39 -0400 Subject: [PATCH] fix(maint): raise recovery-sweep threshold for fetch_external_link (#883) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit External file-host fetches run to a 60-min hard limit (time_limit=3600, per-fetch _FETCH_TIMEOUT=3000s), far longer than the recovery sweep's 5-min default. recover_stalled_task_runs was phantom-flagging healthy in-flight fetches as "RecoverySweep: no completion signal received within 5 min" before the task's own timeout/error handling could surface the real error (operator-flagged: target 414 swept at 6.6min). The sweep already has per-queue/per-task overrides for long tasks, but fetch_external_link was never added and its TaskRun records queue='default' (no queue override) despite external.* routing to download. Add a task-name override of 65 min (time_limit 60 + 5 buffer); task-name precedence makes it robust regardless of the recorded queue. No new internal timeout needed — the existing _FETCH_TIMEOUT + soft_time_limit + except-block log.exception already capture the real failure once the sweep stops preempting. Pinned tests: external-fetch override survives a 10-min row / flags a 70-min row on queue='default'; invariant guard asserts override >= hard time_limit. Co-Authored-By: Claude Opus 4.8 --- backend/app/tasks/maintenance.py | 8 +++++ tests/test_maintenance.py | 53 ++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/backend/app/tasks/maintenance.py b/backend/app/tasks/maintenance.py index f53216d..988c9ca 100644 --- a/backend/app/tasks/maintenance.py +++ b/backend/app/tasks/maintenance.py @@ -147,6 +147,14 @@ TASK_STUCK_THRESHOLD_MINUTES: dict[str, int] = { "backend.app.tasks.backup.restore_images_task": 420, # Library audit scans the full library — 2h hard limit. "backend.app.tasks.library_audit.scan_library_for_rule": 130, + # External file-host fetches (mega/gdrive/film packs) legitimately run to + # the task's 60-min hard limit (time_limit=3600; per-fetch _FETCH_TIMEOUT + # is 50min). Its TaskRun records queue='default' (no queue override), so + # without this it fell to the 5-min default and healthy in-flight fetches + # were phantom-flagged 'RecoverySweep' before their own timeout/error could + # surface (operator-flagged 2026-06-17, target 414 swept at 6.6min). A + # task-name override is robust whatever queue the row records. 65 = 60 + 5. + "backend.app.tasks.external.fetch_external_link": 65, } diff --git a/tests/test_maintenance.py b/tests/test_maintenance.py index 5f26ba7..e014c67 100644 --- a/tests/test_maintenance.py +++ b/tests/test_maintenance.py @@ -453,6 +453,59 @@ def test_recover_stalled_task_runs_archive_task_uses_longer_threshold(db_sync): assert _status(archive_stale_id) == "error" +def test_recover_stalled_task_runs_external_fetch_uses_longer_threshold(db_sync): + """fetch_external_link legitimately runs to its 60-min hard limit, but its + TaskRun records queue='default' (no queue override), so before the + task-name override (65 min) it fell to the 5-min default and healthy + in-flight fetches were phantom-flagged 'RecoverySweep' before their own + timeout/error could surface (operator-flagged 2026-06-17, target 414 swept + at 6.6min). A 10-min-old row must survive; a 70-min-old one is flagged.""" + from sqlalchemy import select + + from backend.app.models import TaskRun + from backend.app.tasks.maintenance import recover_stalled_task_runs + + name = "backend.app.tasks.external.fetch_external_link" + now = datetime.now(UTC) + # 10-min-old: stale by the default 5-min rule but fresh by the 65-min + # task-name override. Must survive despite recording queue='default'. + fresh_id = _make_task_run( + db_sync, status="running", queue="default", task_name=name, + started_at=now - timedelta(minutes=10), + ) + # 70-min-old: past even the 65-min override (a genuine hard kill). Flagged. + stale_id = _make_task_run( + db_sync, status="running", queue="default", task_name=name, + started_at=now - timedelta(minutes=70), + ) + db_sync.commit() + + recovered = recover_stalled_task_runs.apply().get() + assert recovered == 1 + + db_sync.expire_all() + def _status(_id): + return db_sync.execute( + select(TaskRun.status).where(TaskRun.id == _id) + ).scalar_one() + assert _status(fresh_id) == "running" + assert _status(stale_id) == "error" + + +def test_external_fetch_stuck_threshold_exceeds_hard_time_limit(): + """Invariant guard (maintenance.py:112): the fetch_external_link task-name + override MUST be ≥ its hard time_limit, else the sweep flags healthy long + fetches. Pins it so a future time_limit bump can't silently re-break it.""" + from backend.app.tasks.external import fetch_external_link + from backend.app.tasks.maintenance import TASK_STUCK_THRESHOLD_MINUTES + + hard_minutes = fetch_external_link.time_limit / 60 + override = TASK_STUCK_THRESHOLD_MINUTES[ + "backend.app.tasks.external.fetch_external_link" + ] + assert override >= hard_minutes + + def test_prune_task_runs_deletes_ok_older_than_24h(db_sync): from sqlalchemy import select