fix(import): split archive imports into their own task + budget; archive-aware recovery sweeps
Operator-flagged 2026-05-28: import_media_file on target 1645019 hit SoftTimeLimitExceeded at exactly 5.0 min. Their diagnosis was correct — the timeout covered the WHOLE archive, not per object. Importer._import_archive (importer.py:409) runs the full per-member pipeline (sha256 + pHash + dedup query + copy + provenance) for EVERY media member inline, all under import_media_file's single 300s soft limit. A single media file is sub-second; a multi-hundred-member archive blows the budget. They shared one task name and one timeout. **Split archive into its own task** - New `import_archive_file` task: same body as import_media_file (dispatch is by file-kind inside Importer.import_one) but soft=30min / hard=35min. Shared `_run_import_task` helper holds the flip-to-processing + resilience-contract wrapper; both tasks call it. - New `enqueue_import(task_id, task_type)` router — single source of truth for media-vs-archive dispatch. Used by all three enqueue sites: scan_directory, /api/import/retry-failed, recover_interrupted_tasks. - scan_directory now sets ImportTask.task_type = "archive" when is_archive(entry) (the model field already existed, anticipating this; scan was hardcoding "media"). - import_archive_file routes to the existing 'import' queue via the task_routes `import_file.*` wildcard — no worker config change. **Archive-aware recovery sweeps** Both sweeps would otherwise preempt a legitimately-running archive: - recover_interrupted_tasks (ImportTask 'processing' sweep): now task-type-aware. Media stays at STUCK_THRESHOLD_MINUTES (5); archives get ARCHIVE_STUCK_THRESHOLD_MINUTES (40 = 5-min buffer past the 35-min hard limit). Single UPDATE with an OR predicate over the two (task_type, cutoff) pairs; requeue routes via enqueue_import. - recover_stalled_task_runs (TaskRun 'running' sweep): now supports per-task-name overrides (TASK_STUCK_THRESHOLD_MINUTES) layered above the per-queue overrides added for ml. import_archive_file gets 40 min while the 'import' queue stays at the 5-min default for single-file imports. Precedence: task_name → queue → default, each pass excluding rows claimed by a higher-precedence pass so every row is touched once. **Tests** - test_import_archive_file_registered - test_recover_stalled_task_runs_archive_task_uses_longer_threshold — pins that a 10-min archive task-run survives, a 50-min one is flagged, and a same-queue 10-min media import is flagged at the default. - _make_task_run gains queue= + task_name= params. After deploy: archive imports get a 30-min budget and aren't preempted by either sweep; single-file imports keep their tight 5-min detection.
This commit is contained in:
@@ -64,30 +64,13 @@ def _mark_failed(session, task, error_msg: str) -> None:
|
||||
pass
|
||||
|
||||
|
||||
@celery.task(
|
||||
name="backend.app.tasks.import_file.import_media_file",
|
||||
bind=True,
|
||||
autoretry_for=(OperationalError, DBAPIError, OSError),
|
||||
retry_backoff=5,
|
||||
retry_backoff_max=60,
|
||||
retry_jitter=True,
|
||||
max_retries=3,
|
||||
soft_time_limit=300,
|
||||
time_limit=360,
|
||||
)
|
||||
def import_media_file(self, import_task_id: int) -> dict:
|
||||
"""Returns a dict so the eager-mode tests can assert without DB.
|
||||
|
||||
Decorator notes:
|
||||
- autoretry_for: transient DB / filesystem errors retry with
|
||||
exponential backoff (5s base, jitter, max 3 attempts). On final
|
||||
give-up the task raises and acks_late=True (set globally on the
|
||||
Celery app) does NOT redeliver — the recovery sweep catches the
|
||||
row instead.
|
||||
- soft_time_limit (300s) raises SoftTimeLimitExceeded in this
|
||||
process so the task can mark its row failed before being killed.
|
||||
- time_limit (360s) is the hard cap; SIGKILL if the soft signal
|
||||
was swallowed.
|
||||
def _run_import_task(import_task_id: int) -> dict:
|
||||
"""Shared body for import_media_file + import_archive_file. The two
|
||||
tasks differ ONLY in their Celery time limits (a single media file
|
||||
is sub-second; an archive runs the full per-member pipeline inline
|
||||
for every member and can take many minutes). Both flip the row to
|
||||
'processing', dispatch to `_do_import`, and honor the
|
||||
flip-to-terminal resilience contract.
|
||||
"""
|
||||
SessionLocal = _sync_session_factory()
|
||||
with SessionLocal() as session:
|
||||
@@ -103,19 +86,85 @@ def import_media_file(self, import_task_id: int) -> dict:
|
||||
try:
|
||||
return _do_import(session, task, import_task_id)
|
||||
except SoftTimeLimitExceeded:
|
||||
_mark_failed(session, task, "soft_time_limit exceeded (>300s)")
|
||||
_mark_failed(session, task, "soft_time_limit exceeded")
|
||||
raise
|
||||
except (OperationalError, DBAPIError, OSError):
|
||||
# Retryable per the decorator; do NOT mark failed (let
|
||||
# autoretry have a clean go at it). If autoretry exhausts,
|
||||
# the row stays 'processing' and the maintenance sweep
|
||||
# flips it within 5 min.
|
||||
# flips it.
|
||||
raise
|
||||
except Exception as exc: # noqa: BLE001 — pipeline crash, mark + re-raise
|
||||
_mark_failed(session, task, f"{type(exc).__name__}: {exc}")
|
||||
raise
|
||||
|
||||
|
||||
@celery.task(
|
||||
name="backend.app.tasks.import_file.import_media_file",
|
||||
bind=True,
|
||||
autoretry_for=(OperationalError, DBAPIError, OSError),
|
||||
retry_backoff=5,
|
||||
retry_backoff_max=60,
|
||||
retry_jitter=True,
|
||||
max_retries=3,
|
||||
soft_time_limit=300,
|
||||
time_limit=360,
|
||||
)
|
||||
def import_media_file(self, import_task_id: int) -> dict:
|
||||
"""Import ONE media file (or non-media → PostAttachment). Sub-second
|
||||
for the common case; the tight 5-min soft limit keeps a genuinely
|
||||
stuck single-file import detectable fast.
|
||||
|
||||
Decorator notes:
|
||||
- autoretry_for: transient DB / filesystem errors retry with
|
||||
exponential backoff (5s base, jitter, max 3 attempts). On final
|
||||
give-up the task raises and acks_late=True (set globally on the
|
||||
Celery app) does NOT redeliver — the recovery sweep catches the
|
||||
row instead.
|
||||
- soft_time_limit (300s) raises SoftTimeLimitExceeded in-process
|
||||
so the task can mark its row failed before being killed.
|
||||
- time_limit (360s) is the hard SIGKILL cap.
|
||||
"""
|
||||
return _run_import_task(import_task_id)
|
||||
|
||||
|
||||
@celery.task(
|
||||
name="backend.app.tasks.import_file.import_archive_file",
|
||||
bind=True,
|
||||
autoretry_for=(OperationalError, DBAPIError, OSError),
|
||||
retry_backoff=5,
|
||||
retry_backoff_max=60,
|
||||
retry_jitter=True,
|
||||
max_retries=3,
|
||||
# Archives run the full per-member pipeline (sha256 + pHash + dedup
|
||||
# query + copy + provenance) for EVERY media member inline, under a
|
||||
# single task budget. A multi-hundred-member archive blows the
|
||||
# 5-min media limit. soft=30min / hard=35min sizes for a large
|
||||
# archive. Operator-flagged 2026-05-28 (target 1645019 hit the old
|
||||
# shared 300s soft limit). The recovery sweep gives this task its
|
||||
# own 40-min threshold via maintenance.TASK_STUCK_THRESHOLD_MINUTES
|
||||
# so it isn't preempted while legitimately grinding through members.
|
||||
soft_time_limit=1800,
|
||||
time_limit=2100,
|
||||
)
|
||||
def import_archive_file(self, import_task_id: int) -> dict:
|
||||
"""Import an archive: extract + run the per-member media pipeline for
|
||||
every member inline, then preserve the archive as a PostAttachment.
|
||||
Same body as import_media_file (dispatch is by file kind inside
|
||||
Importer.import_one); split out purely for the larger time budget."""
|
||||
return _run_import_task(import_task_id)
|
||||
|
||||
|
||||
def enqueue_import(task_id: int, task_type: str) -> None:
|
||||
"""Route an ImportTask to the right Celery task by its task_type.
|
||||
Single source of truth for the media-vs-archive dispatch so the
|
||||
scan, retry, and recovery-requeue paths stay in sync."""
|
||||
if task_type == "archive":
|
||||
import_archive_file.delay(task_id)
|
||||
else:
|
||||
import_media_file.delay(task_id)
|
||||
|
||||
|
||||
def _do_import(session, task, import_task_id: int) -> dict:
|
||||
"""Actual work, called from inside the resilience wrapper."""
|
||||
settings = session.execute(
|
||||
|
||||
Reference in New Issue
Block a user