fix: two copies of one picture imported at once are filed as duplicates, and an archive's row always ends
CI and images / lint (push) Successful in 2s
CI and images / extension-version (push) Successful in 2s
CI and images / extension-test (push) Successful in 20s
CI and images / frontend-build (push) Successful in 23s
CI and images / backend-lint-and-test (push) Successful in 38s
CI and images / integration (push) Successful in 2m31s
CI and images / sign-extension (push) Successful in 2s
CI and images / build-agent (push) Successful in 4s
CI and images / build-web (push) Successful in 1m56s
CI and images / smoke-web (push) Successful in 50s
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 2s
CI and images / extension-version (push) Successful in 2s
CI and images / extension-test (push) Successful in 20s
CI and images / frontend-build (push) Successful in 23s
CI and images / backend-lint-and-test (push) Successful in 38s
CI and images / integration (push) Successful in 2m31s
CI and images / sign-extension (push) Successful in 2s
CI and images / build-agent (push) Successful in 4s
CI and images / build-web (push) Successful in 1m56s
CI and images / smoke-web (push) Successful in 50s
CI and images / promote (push) Successful in 1s
CI and images / refresh-dev (push) Skipped
CI and images / unattended-failure (push) Skipped
The sha256 lookup before inserting an image row can lose a race to another worker importing a second copy of the same picture. The unique index then refused the insert. A media import retried behind the whole import queue, and the recovery sweep ran it a second time. An archive swallowed the error, failed on the dead transaction, and its row stayed 'processing' because the failure mark could not commit either. - Importer._store_new_record inserts under a savepoint; on a conflict the file is filed as a duplicate of the row that won (_file_as_duplicate, now shared by every duplicate branch). _import_media removes its stray library copy. - _import_archive commits its post before extracting; the member loop rolls back a failed member and lets SoftTimeLimitExceeded through. - _mark_failed rolls back before writing the failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -16,6 +16,7 @@ from dataclasses import dataclass, field
|
||||
from enum import StrEnum
|
||||
from pathlib import Path
|
||||
|
||||
from celery.exceptions import SoftTimeLimitExceeded
|
||||
from PIL import Image
|
||||
from sqlalchemy import func, select, update
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
@@ -307,6 +308,44 @@ class Importer:
|
||||
sp.rollback()
|
||||
return self.session.execute(stmt).scalar_one()
|
||||
|
||||
def _store_new_record(self, record: ImageRecord) -> ImageRecord | None:
|
||||
"""Insert `record`, or return the row that already holds its bytes.
|
||||
|
||||
The sha256 lookup before an insert can lose a race: two copies of one
|
||||
picture in different folders, imported side by side by two workers,
|
||||
both find no row, and the unique index refuses the second insert. Only
|
||||
that insert is undone (a savepoint), so the session stays usable and
|
||||
the caller files its file as a duplicate of the winner. None means
|
||||
`record` was stored."""
|
||||
sp = self.session.begin_nested()
|
||||
try:
|
||||
self.session.add(record)
|
||||
self.session.flush()
|
||||
sp.commit()
|
||||
return None
|
||||
except IntegrityError:
|
||||
sp.rollback()
|
||||
winner = self.session.execute(
|
||||
select(ImageRecord).where(ImageRecord.sha256 == record.sha256)
|
||||
).scalar_one_or_none()
|
||||
if winner is None:
|
||||
raise
|
||||
return winner
|
||||
|
||||
def _file_as_duplicate(
|
||||
self, existing: ImageRecord, path: Path, artist: Artist | None,
|
||||
explicit_source: Source | None,
|
||||
) -> ImportResult:
|
||||
"""A byte-identical copy of `existing`: link the copy's post to the
|
||||
image (enrich-on-duplicate), so a picture posted twice shows on both
|
||||
posts, and report the skip."""
|
||||
self._apply_sidecar(existing, path, artist, explicit_source=explicit_source)
|
||||
self.session.commit()
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.duplicate_hash,
|
||||
image_id=existing.id, error="sha256 already present",
|
||||
)
|
||||
|
||||
def _find_or_create_source(
|
||||
self, *, artist_id: int, platform: str, url: str,
|
||||
) -> Source:
|
||||
@@ -577,6 +616,9 @@ class Importer:
|
||||
|
||||
artist_use = artist if artist is not None else self._resolve_artist(source)
|
||||
post = self._post_for_sidecar(source, artist_use, source_row=source_row)
|
||||
# Committed before extraction, so a member that fails (and is rolled
|
||||
# back on its own) can never take the archive's post with it.
|
||||
self.session.commit()
|
||||
member_ids: list[int] = []
|
||||
# Every member image touched (new + superseded + deduped), so the
|
||||
# from_attachment_id stamp below covers files that already existed in the
|
||||
@@ -703,7 +745,15 @@ class Importer:
|
||||
member_record_ids.add(res.image_id)
|
||||
else:
|
||||
counts["failed"] += 1
|
||||
except SoftTimeLimitExceeded:
|
||||
# The task's time budget is spent: stop here so the task marks
|
||||
# its row failed, rather than carrying on until the hard kill.
|
||||
raise
|
||||
except Exception as exc: # noqa: BLE001 — defensive per level; keep going
|
||||
# Each member commits on its own, so this undoes only the one in
|
||||
# progress; without it the next query fails on the dead transaction
|
||||
# and the archive is never stored.
|
||||
self.session.rollback()
|
||||
log.warning(
|
||||
"archive extraction failed for %s (depth %d): %s",
|
||||
archive_path.name, depth, exc,
|
||||
@@ -884,18 +934,8 @@ class Importer:
|
||||
existing = self.session.execute(existing_stmt).scalar_one_or_none()
|
||||
if existing:
|
||||
if not self.deep:
|
||||
# Enrich-on-duplicate (parity with attach_in_place): a re-scanned
|
||||
# file that is a byte-dup of an existing image still links its
|
||||
# post via _apply_sidecar, so a cross-posted image shows on every
|
||||
# post.
|
||||
self._apply_sidecar(
|
||||
existing, attribution_path, path_artist,
|
||||
explicit_source=explicit_source,
|
||||
)
|
||||
self.session.commit()
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.duplicate_hash,
|
||||
image_id=existing.id, error="sha256 already present",
|
||||
return self._file_as_duplicate(
|
||||
existing, attribution_path, path_artist, explicit_source,
|
||||
)
|
||||
return self._deep_rederive(existing, source, attribution_path)
|
||||
|
||||
@@ -981,8 +1021,14 @@ class Importer:
|
||||
origin="imported_filesystem",
|
||||
integrity_status="unknown",
|
||||
)
|
||||
self.session.add(record)
|
||||
self.session.flush()
|
||||
if (winner := self._store_new_record(record)) is not None:
|
||||
# The copy just written is a stray unless it is the winner's own
|
||||
# file (the same source imported twice lands on the same path).
|
||||
if Path(winner.path) != dest:
|
||||
dest.unlink(missing_ok=True)
|
||||
return self._file_as_duplicate(
|
||||
winner, attribution_path, path_artist, explicit_source,
|
||||
)
|
||||
self._phash_cache_append(phash, width, height, record.id)
|
||||
|
||||
# Folder→artist (path_artist resolved up-front above); bind it to the
|
||||
@@ -1310,12 +1356,7 @@ class Importer:
|
||||
# only sets it when NULL. Without this the new post is left with no
|
||||
# image AND (if its other content also deduped) no attachment, i.e. a
|
||||
# bare shell — operator-flagged 2026-06-08 (1589 empty Anduo posts).
|
||||
self._apply_sidecar(existing, path, artist, explicit_source=source)
|
||||
self.session.commit()
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.duplicate_hash,
|
||||
image_id=existing.id, error="sha256 already present",
|
||||
)
|
||||
return self._file_as_duplicate(existing, path, artist, source)
|
||||
|
||||
# phash dedup + supersede
|
||||
phash = None
|
||||
@@ -1395,8 +1436,8 @@ class Importer:
|
||||
)
|
||||
if artist is not None:
|
||||
record.artist_id = artist.id
|
||||
self.session.add(record)
|
||||
self.session.flush()
|
||||
if (winner := self._store_new_record(record)) is not None:
|
||||
return self._file_as_duplicate(winner, path, artist, source)
|
||||
self._phash_cache_append(phash, width, height, record.id)
|
||||
|
||||
# Sidecar provenance (best-effort). When `source` is passed, link
|
||||
|
||||
Reference in New Issue
Block a user