fix: downloads stop leaving files on disk that the library doesn't use
CI and images / lint (push) Successful in 1s
CI and images / extension-version (push) Successful in 2s
CI and images / extension-test (push) Successful in 19s
CI and images / frontend-build (push) Successful in 28s
CI and images / backend-lint-and-test (push) Successful in 35s
CI and images / integration (push) Successful in 2m25s
CI and images / sign-extension (push) Successful in 3s
CI and images / build-agent (push) Successful in 6s
CI and images / build-web (push) Successful in 2m3s
CI and images / smoke-web (push) Successful in 55s
CI and images / promote (push) Successful in 1s
CI and images / refresh-dev (push) Skipped
CI and images / unattended-failure (push) Skipped
CI and images / lint (push) Successful in 1s
CI and images / extension-version (push) Successful in 2s
CI and images / extension-test (push) Successful in 19s
CI and images / frontend-build (push) Successful in 28s
CI and images / backend-lint-and-test (push) Successful in 35s
CI and images / integration (push) Successful in 2m25s
CI and images / sign-extension (push) Successful in 3s
CI and images / build-agent (push) Successful in 6s
CI and images / build-web (push) Successful in 2m3s
CI and images / smoke-web (push) Successful in 55s
CI and images / promote (push) Successful in 1s
CI and images / refresh-dev (push) Skipped
CI and images / unattended-failure (push) Skipped
- An archive whose images were extracted was kept at its download path as well as in the attachments store. Both download paths now remove the downloaded copy. - A downloaded file the import filters reject (too small, transparent, single colour, unreadable) was left on disk with no record. A later walk found it, imported it again and rejected it again, every time. Rejected files are now removed; the seen ledger keeps them from being downloaded again. - Both paths share one policy, Importer.discard_after_attach, which never removes a file an ImageRecord points at. - An external-link fetch that fails, or whose worker was killed, left partial files (megatools' .megatmp.*) in the link's dir, and a killed fetch left its link 'downloading' for good. A fetch now clears unrecorded leftovers before starting and after failing. The sweep returns a link to retry once it has been 'downloading' past the task's hard limit, using a new external_link.started_at column (migration 0117). - The orphaned temp-file sweep now recognises .megatmp.* as well as .part and .partial. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -24,7 +24,7 @@ from datetime import UTC, datetime, timedelta
|
||||
from pathlib import Path
|
||||
|
||||
import redis
|
||||
from sqlalchemy import delete, select, update
|
||||
from sqlalchemy import delete, or_, select, update
|
||||
|
||||
from ..celery_app import celery
|
||||
from ..config import get_config
|
||||
@@ -54,6 +54,9 @@ _RETENTION_DAYS = 30
|
||||
# than one fetch would have taken.
|
||||
_LOCK_PREFIX = "fc:extdl_lock:"
|
||||
_LOCK_TTL = 2400
|
||||
# A link 'downloading' this long has outlived fetch_external_link's hard limit
|
||||
# (time_limit=3600), so its worker is gone and the sweep recovers it.
|
||||
_STALL_MINUTES = 65
|
||||
_SERIALIZE_COUNTDOWN = 120
|
||||
_MAX_SERIALIZE_WAITS = 30
|
||||
|
||||
@@ -106,7 +109,7 @@ def fetch_external_link(self, link_id: int, _serialize_waits: int = 0) -> dict:
|
||||
ExternalLink.id == link_id,
|
||||
ExternalLink.status.in_(("pending", "failed")),
|
||||
)
|
||||
.values(status="downloading")
|
||||
.values(status="downloading", started_at=datetime.now(UTC))
|
||||
.returning(ExternalLink.id)
|
||||
).first()
|
||||
session.commit()
|
||||
@@ -180,6 +183,10 @@ def fetch_external_link(self, link_id: int, _serialize_waits: int = 0) -> dict:
|
||||
/ str(post.external_post_id) / str(link_id)
|
||||
)
|
||||
try:
|
||||
# Clear what an earlier, interrupted attempt left behind, so this fetch
|
||||
# starts from the link's imported files only.
|
||||
with SessionLocal() as session:
|
||||
_discard_unrecorded(session, post_dir)
|
||||
result = fetch_external(host, url, post_dir)
|
||||
with SessionLocal() as session:
|
||||
link = session.get(ExternalLink, link_id)
|
||||
@@ -190,6 +197,7 @@ def fetch_external_link(self, link_id: int, _serialize_waits: int = 0) -> dict:
|
||||
)
|
||||
_record_failure(link, attempts, result.error or "fetch failed")
|
||||
session.commit()
|
||||
_discard_unrecorded(session, post_dir)
|
||||
return {"link_id": link_id, "error": result.error}
|
||||
|
||||
log.info(
|
||||
@@ -231,6 +239,7 @@ def fetch_external_link(self, link_id: int, _serialize_waits: int = 0) -> dict:
|
||||
if link is not None and link.status == "downloading":
|
||||
_record_failure(link, attempts, f"{type(exc).__name__}: {exc}")
|
||||
session.commit()
|
||||
_discard_unrecorded(session, post_dir)
|
||||
raise
|
||||
finally:
|
||||
if lock is not None:
|
||||
@@ -287,10 +296,9 @@ def _route_files(session, files, post, platform, artist, source) -> list[int]:
|
||||
post.id, f.name, result.status, len(ids),
|
||||
)
|
||||
elif result.status == "attached":
|
||||
# Non-art / archive captured as a PostAttachment (copied into the
|
||||
# store) — drop the on-disk original. An archive captured WITHOUT
|
||||
# extracting any image carries the reason (the recurring "zip but no
|
||||
# images" symptom, #718) — surface it loudly.
|
||||
# Non-art, or an archive with no new images, captured as a
|
||||
# PostAttachment. An archive captured WITHOUT extracting any image
|
||||
# carries the reason, so the "zip but no images" case is visible.
|
||||
if result.error:
|
||||
log.warning(
|
||||
"extdl archive captured UNEXTRACTED: post=%s file=%s reason=%s",
|
||||
@@ -298,51 +306,91 @@ def _route_files(session, files, post, platform, artist, source) -> list[int]:
|
||||
)
|
||||
else:
|
||||
log.info("extdl attachment captured: post=%s file=%s", post.id, f.name)
|
||||
f.unlink(missing_ok=True)
|
||||
elif result.status == "skipped":
|
||||
reason = result.skip_reason.value if result.skip_reason else None
|
||||
log.info(
|
||||
"extdl skipped: post=%s file=%s reason=%s", post.id, f.name, reason,
|
||||
)
|
||||
if reason in ("duplicate_hash", "duplicate_phash"):
|
||||
# Path-safe unlink: external files are imported IN PLACE, so `f`
|
||||
# can BE the canonical record's backing file (same content
|
||||
# re-fetched / a colliding name). Only delete `f` when it is NOT
|
||||
# the existing record's file — otherwise we orphan the record and
|
||||
# playback 404s. (#859)
|
||||
canonical = None
|
||||
if result.image_id is not None:
|
||||
rec = session.get(ImageRecord, result.image_id)
|
||||
if rec is not None:
|
||||
canonical = (IMAGES_ROOT / rec.path).resolve()
|
||||
if canonical != f.resolve():
|
||||
f.unlink(missing_ok=True) # a redundant copy — safe to drop
|
||||
else:
|
||||
log.info(
|
||||
"extdl keep: %s IS record %s's canonical file — not unlinking",
|
||||
f.name, result.image_id,
|
||||
)
|
||||
elif result.status == "failed":
|
||||
log.warning(
|
||||
"extdl import FAILED: post=%s file=%s error=%s",
|
||||
post.id, f.name, result.error,
|
||||
)
|
||||
f.unlink(missing_ok=True)
|
||||
else: # 'refreshed' or any future status — work happened, no new images
|
||||
log.info(
|
||||
"extdl import: post=%s file=%s status=%s", post.id, f.name, result.status,
|
||||
)
|
||||
# Same policy as the subscription download path: drop the file unless
|
||||
# the library needs it at this path. A file imported in place is kept.
|
||||
importer.discard_after_attach(f, result)
|
||||
# The synthesized sidecar has done its job — don't litter the tree.
|
||||
Path(str(f) + ".json").unlink(missing_ok=True)
|
||||
return image_ids
|
||||
|
||||
|
||||
def _discard_unrecorded(session, link_dir: Path) -> int:
|
||||
"""Delete every file in a link's staging dir that no ImageRecord points at,
|
||||
then the dirs left empty. The dir belongs to one link, so anything in it
|
||||
without a record is a partial or abandoned download from an earlier
|
||||
attempt (megatools' `.megatmp.*`, a killed transfer, a rejected file).
|
||||
Images imported in place stay. Returns the number of files removed."""
|
||||
if not link_dir.is_dir():
|
||||
return 0
|
||||
files = [p for p in link_dir.rglob("*") if p.is_file()]
|
||||
if not files:
|
||||
return 0
|
||||
recorded = set(session.execute(
|
||||
select(ImageRecord.path).where(
|
||||
ImageRecord.path.in_([str(p) for p in files])
|
||||
)
|
||||
).scalars())
|
||||
removed = 0
|
||||
for p in files:
|
||||
if str(p) in recorded:
|
||||
continue
|
||||
try:
|
||||
p.unlink(missing_ok=True)
|
||||
removed += 1
|
||||
except OSError as exc:
|
||||
log.warning("extdl could not remove %s: %s", p, exc)
|
||||
for d in sorted((p for p in link_dir.rglob("*") if p.is_dir()), reverse=True):
|
||||
try:
|
||||
d.rmdir()
|
||||
except OSError:
|
||||
pass # not empty: it holds an imported file
|
||||
try:
|
||||
link_dir.rmdir()
|
||||
except OSError:
|
||||
pass
|
||||
if removed:
|
||||
log.info("extdl removed %d leftover file(s) from %s", removed, link_dir)
|
||||
return removed
|
||||
|
||||
|
||||
@celery.task(name="backend.app.tasks.external.sweep_external_links")
|
||||
def sweep_external_links() -> dict:
|
||||
"""Enqueue a bounded batch of actionable links (pending, or failed below the
|
||||
dead-letter threshold). Driven by the post-download hook and a beat tick."""
|
||||
dead-letter threshold). Driven by the post-download hook and a beat tick.
|
||||
|
||||
First it recovers links whose worker died mid-fetch: a link 'downloading'
|
||||
for longer than the task's hard limit has no worker left, so it counts as a
|
||||
failed attempt and goes back to the retry path (dead-lettered at the
|
||||
threshold like any other failure). The next attempt clears its leftovers."""
|
||||
SessionLocal = _sync_session_factory()
|
||||
with SessionLocal() as session:
|
||||
now = datetime.now(UTC)
|
||||
stale = session.execute(
|
||||
select(ExternalLink).where(
|
||||
ExternalLink.status == "downloading",
|
||||
or_(
|
||||
ExternalLink.started_at.is_(None),
|
||||
ExternalLink.started_at < now - timedelta(minutes=_STALL_MINUTES),
|
||||
),
|
||||
)
|
||||
).scalars().all()
|
||||
for link in stale:
|
||||
_record_failure(link, link.attempts, "worker stopped mid-fetch")
|
||||
session.commit()
|
||||
ids = session.execute(
|
||||
select(ExternalLink.id).where(
|
||||
ExternalLink.status.in_(("pending", "failed")),
|
||||
@@ -351,7 +399,7 @@ def sweep_external_links() -> dict:
|
||||
).scalars().all()
|
||||
for link_id in ids:
|
||||
fetch_external_link.delay(link_id)
|
||||
return {"enqueued": len(ids)}
|
||||
return {"enqueued": len(ids), "recovered": len(stale)}
|
||||
|
||||
|
||||
@celery.task(name="backend.app.tasks.external.recover_external_links")
|
||||
|
||||
@@ -88,8 +88,18 @@ TASK_RUN_KEEP_FAILURE_SECONDS = 7 * 24 * 3600 # 7 days
|
||||
# keeps it from deleting an in-flight download's staging file mid-write.
|
||||
IMAGES_ROOT = Path("/images")
|
||||
TEMP_STAGING_SUFFIXES = (".part", ".partial")
|
||||
# megatools stages a Mega download as `.megatmp.<id>` in the target dir.
|
||||
TEMP_STAGING_PREFIXES = (".megatmp.",)
|
||||
ORPHAN_TEMP_MIN_AGE_HOURS = 6
|
||||
|
||||
|
||||
def is_temp_staging_file(path: Path) -> bool:
|
||||
"""True for a downloader's staging file: one that never becomes library
|
||||
content under that name, so a stale one is safe to delete."""
|
||||
return path.suffix in TEMP_STAGING_SUFFIXES or path.name.startswith(
|
||||
TEMP_STAGING_PREFIXES
|
||||
)
|
||||
|
||||
# Audit 2026-06-02: per-entity recovery sweep thresholds. Each must be
|
||||
# > the entity's longest legitimate runtime (its task's time_limit + a
|
||||
# small buffer) so the sweep never flags in-flight work.
|
||||
@@ -373,16 +383,17 @@ def cleanup_old_tasks() -> int:
|
||||
|
||||
@celery.task(name="backend.app.tasks.maintenance.cleanup_orphaned_temp_files")
|
||||
def cleanup_orphaned_temp_files() -> int:
|
||||
"""Delete orphaned .part/.partial staging files under the images root, left by
|
||||
a download/import killed mid-write. Only removes files older than
|
||||
ORPHAN_TEMP_MIN_AGE_HOURS so an in-flight download's staging file is never
|
||||
pulled out from under it. Returns the count removed."""
|
||||
"""Delete orphaned staging files (.part, .partial, megatools' .megatmp.*)
|
||||
under the images root, left by a download/import killed mid-write. Only
|
||||
removes files older than ORPHAN_TEMP_MIN_AGE_HOURS so an in-flight
|
||||
download's staging file is never pulled out from under it. Returns the
|
||||
count removed."""
|
||||
if not IMAGES_ROOT.is_dir():
|
||||
return 0
|
||||
cutoff = datetime.now(UTC).timestamp() - ORPHAN_TEMP_MIN_AGE_HOURS * 3600
|
||||
removed = 0
|
||||
for path in IMAGES_ROOT.rglob("*"):
|
||||
if path.suffix not in TEMP_STAGING_SUFFIXES or not path.is_file():
|
||||
if not is_temp_staging_file(path) or not path.is_file():
|
||||
continue
|
||||
try:
|
||||
if path.stat().st_mtime >= cutoff:
|
||||
|
||||
Reference in New Issue
Block a user