From 9ba3db75fd19643981c035a5a272729e516149b7 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 10 Jun 2026 11:57:31 -0400 Subject: [PATCH] fix(maintenance): download queue needs a sweep threshold above its 25-min time_limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit recover_stalled_task_runs used the 5-min default for the download queue, but download_source legitimately walks up to DOWNLOAD_HARD_TIME_LIMIT (1500s = 25m). Healthy in-flight Patreon/gallery-dl walks were flagged as phantom 'RecoverySweep' failures — visible in System Activity but absent from the Subscriptions view (the download finished ok, reset the source's consecutive_failures; only the orphaned task_run kept the stamp, since _finalize only updates rows still 'running'). Add download:30 to QUEUE_STUCK_THRESHOLD_MINUTES — clears the 25-min hard limit with buffer and matches DOWNLOAD_STALL_THRESHOLD_MINUTES so a real hard kill is swept by the task-run and event sweeps together. Restores the documented invariant (every override >= task time_limit). Regression test pins the threshold above the hard limit so a future limit bump can't silently re-break it. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/app/tasks/maintenance.py | 9 +++++ tests/test_maintenance.py | 56 ++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/backend/app/tasks/maintenance.py b/backend/app/tasks/maintenance.py index 7e81786..f53216d 100644 --- a/backend/app/tasks/maintenance.py +++ b/backend/app/tasks/maintenance.py @@ -120,6 +120,15 @@ IMPORT_BATCH_KEEP_DAYS = 30 # files); time_limit=2100. QUEUE_STUCK_THRESHOLD_MINUTES: dict[str, int] = { "ml": 25, + # download_source legitimately walks 5-25 min (Patreon/gallery-dl + # deep creators); its hard time_limit is DOWNLOAD_HARD_TIME_LIMIT + # (1500s = 25m). The 5-min default flagged healthy in-flight walks as + # phantom 'RecoverySweep' failures (System Activity showed errors the + # Subscriptions view correctly didn't — the download finished ok and + # reset the source). 30 clears the 25-min limit with buffer and lines + # up with DOWNLOAD_STALL_THRESHOLD_MINUTES (30) so a genuine hard kill + # is swept by the task-run AND event sweeps together. Audit 2026-06-10. + "download": 30, # Audit 2026-06-02 — maintenance/scan queues run tasks that # legitimately exceed the 5-min default (verify_integrity at 70m # hard, scan_directory at 70m hard, apply_allowlist_tags / diff --git a/tests/test_maintenance.py b/tests/test_maintenance.py index 9873d4d..5f26ba7 100644 --- a/tests/test_maintenance.py +++ b/tests/test_maintenance.py @@ -350,6 +350,62 @@ def test_recover_stalled_task_runs_ml_queue_uses_longer_threshold(db_sync): assert ml_stale_status == "error" +def test_recover_stalled_task_runs_download_queue_uses_longer_threshold(db_sync): + """download_source legitimately walks 5-25 min (Patreon/gallery-dl). + The 5-min default flagged healthy in-flight walks as phantom + 'RecoverySweep' failures — visible in System Activity but absent from + the Subscriptions view because the download actually finished ok. + The 30-min download override (QUEUE_STUCK_THRESHOLD_MINUTES) must + protect a 10-min-old download row while still flagging a 35-min one. + Audit 2026-06-10.""" + from sqlalchemy import select + + from backend.app.models import TaskRun + from backend.app.tasks.maintenance import recover_stalled_task_runs + + now = datetime.now(UTC) + # 10-min-old download row: stale by the default 5-min rule but fresh + # by the 30-min download override. Must survive the sweep. + dl_fresh_id = _make_task_run( + db_sync, status="running", queue="download", + task_name="backend.app.tasks.download.download_source", + started_at=now - timedelta(minutes=10), + ) + # 35-min-old download row: past even the 30-min override (a genuine + # hard kill). Must be flagged. + dl_stale_id = _make_task_run( + db_sync, status="running", queue="download", + task_name="backend.app.tasks.download.download_source", + started_at=now - timedelta(minutes=35), + ) + db_sync.commit() + + recovered = recover_stalled_task_runs.apply().get() + assert recovered == 1 + + db_sync.expire_all() + dl_fresh_status = db_sync.execute( + select(TaskRun.status).where(TaskRun.id == dl_fresh_id) + ).scalar_one() + dl_stale_status = db_sync.execute( + select(TaskRun.status).where(TaskRun.id == dl_stale_id) + ).scalar_one() + assert dl_fresh_status == "running" + assert dl_stale_status == "error" + + +def test_download_stuck_threshold_exceeds_hard_time_limit(): + """Invariant guard (maintenance.py:112): every queue override MUST be + ≥ the relevant task's hard time_limit, else the sweep flags in-flight + work. download_source is the one that regressed — pin it so a future + DOWNLOAD_HARD_TIME_LIMIT bump can't silently re-break it.""" + from backend.app.tasks.download import DOWNLOAD_HARD_TIME_LIMIT + from backend.app.tasks.maintenance import QUEUE_STUCK_THRESHOLD_MINUTES + + hard_minutes = DOWNLOAD_HARD_TIME_LIMIT / 60 + assert QUEUE_STUCK_THRESHOLD_MINUTES["download"] >= hard_minutes + + def test_recover_stalled_task_runs_archive_task_uses_longer_threshold(db_sync): """import_archive_file shares the 'import' queue with fast single-file import_media_file, so it gets a per-task-name override