fix(external): recovery-sweep threshold + queue recording + split fetch timeouts (#883) #116

Merged

bvandeusen merged 4 commits from dev into main 2026-06-16 21:24:31 -04:00

Conversation 0 Commits 4 Files Changed 8

+212 -37

bvandeusen commented 2026-06-16 21:24:25 -04:00

Owner

Copy Link

Copy Source

External file-host fetch hardening, all CI-green on dev (run 1193, ee1b45f).

1. Phantom RecoverySweep failures

fetch_external_link runs to a 60-min hard limit but was swept at the 5-min default, phantom-flagging healthy in-flight fetches before their own error could surface. Added a task-name override (65 min) to TASK_STUCK_THRESHOLD_MINUTES.

2. Queue-recording divergence

celery_signals._queue_for (the hand-maintained mirror of task_routes) was missing the external. prefix, so external rows recorded queue='default' even though they route to download. Dashboard filters + per-queue thresholds missed them. Fixed + pinned a test that the mirror agrees with the route.

3. Read-vs-total timeout split

The single _FETCH_TIMEOUT=3000s was a total for mega (subprocess) but only a per-read idle timeout for HTTP hosts — a stalled HTTP socket tied up a worker slot + the per-host lock for ~50 min. Split into read_timeout=60s (stalled sockets fail fast) and a total_timeout=30min wall-clock budget enforced across chunks; mega keeps the total as its subprocess cap. Per-host lock TTL lowered 3600→2400.

Each external link is already one Celery task, so these budgets are per-link.

🤖 Generated with Claude Code

External file-host fetch hardening, all CI-green on dev (run 1193, ee1b45f). ## 1. Phantom RecoverySweep failures `fetch_external_link` runs to a 60-min hard limit but was swept at the 5-min default, phantom-flagging healthy in-flight fetches before their own error could surface. Added a task-name override (65 min) to `TASK_STUCK_THRESHOLD_MINUTES`. ## 2. Queue-recording divergence `celery_signals._queue_for` (the hand-maintained mirror of `task_routes`) was missing the `external.` prefix, so external rows recorded `queue='default'` even though they route to `download`. Dashboard filters + per-queue thresholds missed them. Fixed + pinned a test that the mirror agrees with the route. ## 3. Read-vs-total timeout split The single `_FETCH_TIMEOUT=3000s` was a total for mega (subprocess) but only a per-read idle timeout for HTTP hosts — a stalled HTTP socket tied up a worker slot + the per-host lock for ~50 min. Split into `read_timeout=60s` (stalled sockets fail fast) and a `total_timeout=30min` wall-clock budget enforced across chunks; mega keeps the total as its subprocess cap. Per-host lock TTL lowered 3600→2400. Each external link is already one Celery task, so these budgets are per-link. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

bvandeusen added 4 commits 2026-06-16 21:24:25 -04:00

fix(maint): raise recovery-sweep threshold for fetch_external_link (#883) ... 258c77dfcd

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 <noreply@anthropic.com>

fix(activity): record external.* TaskRun.queue as download, not default (#883) ... 25e1e098fb

celery_signals._queue_for is a hand-maintained mirror of task_routes that
stamps TaskRun.queue in the prerun signal. It was missing the
backend.app.tasks.external. prefix, so external fetches recorded
queue='default' even though celery routes external.* → download and runs
them on the download worker. The dashboard's per-queue filters and the
per-queue recovery-sweep threshold therefore missed them — the same
'queue column lies default' gap the 2026-06-02 audit fixed for
backup/admin/library_audit.

Map external.* → download in _queue_for. Composes with the fetch_external_link
task-name sweep override (#883), which wins by precedence regardless of the
recorded queue. Pinned test asserts the mirror agrees with the actual route.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

fix(external): split fetch timeout into read (60s) + total (30m) budgets (#883) ... 4272a19d40

The single _FETCH_TIMEOUT=3000s meant different things per host: a TOTAL
wall-clock for mega (subprocess), but only a per-read socket timeout for HTTP
hosts (requests' timeout is the idle gap between bytes, never a total). So a
stalled HTTP connection tied up a download-worker slot AND the per-host
serialize lock for ~50 min before failing (operator-flagged 2026-06-17).

Split into two limits in external_fetch:
- read timeout (_READ_TIMEOUT=60s, with _CONNECT_TIMEOUT=30s) → requests gets
  (connect, read); a stalled socket now fails in ~60s.
- total budget (_TOTAL_TIMEOUT=30min) → enforced as a wall-clock deadline
  across chunks in _stream_to_file (HTTP has no total-download timeout), and
  passed as the subprocess total for mega.
fetch_external() signature: timeout= → read_timeout=/total_timeout=. gdrive
(gdown) self-manages; the celery hard limit is the outer backstop.

Also lowered the per-host lock TTL 3600→2400 so a worker that dies holding it
can't wedge a host's links much past one fetch's budget.

Each external link is already one Celery task (sweep enqueues one
fetch_external_link.delay per link), so these budgets are per-link.

Tests: total-budget-exceeded cleans the .part; HTTP gets (connect, read);
mega gets the total. Worker fakes updated to **kwargs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

test(external): fix third fake_fetch stub still requiring timeout= ... ee1b45f8cc

test_downloaded_archive_gets_provenance_and_tagging's fake_fetch still had the
old `*, timeout` signature; the task now calls fetch_external() without it, so
the stub raised TypeError in the integration lane (run 1191). Switch it to
**kwargs like the other two.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

bvandeusen merged commit 18123fb9cb into main 2026-06-16 21:24:31 -04:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/FabledCurator#116