From 52d7905c43de5c4e8db459be3edab44706580703 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 25 May 2026 22:25:32 -0400 Subject: [PATCH] =?UTF-8?q?perf(importer):=20cache=20phash=20candidates=20?= =?UTF-8?q?on=20Importer=20to=20fix=20archive-import=20soft-timeout=20(was?= =?UTF-8?q?=20O(M=C3=97N)=20per-member=20SELECTs)=20=E2=80=94=20Co-Authore?= =?UTF-8?q?d-By:=20Claude=20Opus=204.7=20(1M=20context)=20?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- backend/app/services/importer.py | 78 ++++++++++++++++++++++---------- 1 file changed, 54 insertions(+), 24 deletions(-) diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index 309f923..50fe479 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -158,6 +158,49 @@ class Importer: self.settings = settings self.deep = deep self.attachments = AttachmentStore(images_root) + # phash near-dup candidate cache. Archive imports call _import_media + # per-member; without this cache the per-member SELECT *FROM + # image_record WHERE phash IS NOT NULL fetch repeats N times and a + # large library × many-member archive blew past soft_time_limit + # (300s) — operator-flagged 2026-05-25. Loaded lazily on first + # need, appended to on every imported/superseded outcome, never + # invalidated mid-Importer (Importer instances are per-task / + # per-archive-import so cross-instance staleness is harmless). + self._phash_candidates: list[tuple] | None = None + + def _phash_candidates_cache(self) -> list[tuple]: + """Cached `(phash, width, height, id)` rows from image_record. + Loaded on first call, appended-to on subsequent imported/ + superseded outcomes. Soft-timeout pattern: an archive with N + members + a library of M existing rows used to do N × M-row + fetches (operator-flagged 2026-05-25); now it's exactly one. + + The per-task lifecycle of Importer (instantiated fresh by + import_media_file) bounds the cache's staleness window: cross- + process changes (other workers importing concurrently) won't + be reflected, but that's the same race the un-cached version + had — `find_similar` is best-effort anyway.""" + if self._phash_candidates is None: + rows = self.session.execute( + select( + ImageRecord.phash, + ImageRecord.width, + ImageRecord.height, + ImageRecord.id, + ).where(ImageRecord.phash.is_not(None)) + ).all() + self._phash_candidates = [ + (r.phash, r.width or 0, r.height or 0, r.id) for r in rows + ] + return self._phash_candidates + + def _phash_cache_append(self, phash, width, height, image_id) -> None: + """Append a freshly-imported row to the cache so subsequent + members of the same archive can match against it.""" + if self._phash_candidates is not None and phash is not None: + self._phash_candidates.append( + (phash, width or 0, height or 0, image_id) + ) def import_one(self, source: Path) -> ImportResult: """Dispatch by kind. Media → normal pipeline. Archive → extract @@ -356,18 +399,7 @@ class Importer: error=f"PIL load failed during phash compute: {exc}", ) if phash is not None: - cand_rows = self.session.execute( - select( - ImageRecord.phash, - ImageRecord.width, - ImageRecord.height, - ImageRecord.id, - ).where(ImageRecord.phash.is_not(None)) - ).all() - candidates = [ - (c.phash, c.width or 0, c.height or 0, c.id) - for c in cand_rows - ] + candidates = self._phash_candidates_cache() rel, match_id = find_similar( phash, width or 0, height or 0, candidates, self.settings.phash_threshold, @@ -401,6 +433,7 @@ class Importer: ) self.session.add(record) self.session.flush() + self._phash_cache_append(phash, width, height, record.id) # Folder→artist (anchored to attribution_path). artist = None @@ -442,6 +475,9 @@ class Importer: ph = compute_phash(im) if ph is not None: existing.phash = ph + # Promoted from NULL to non-NULL → cache is now stale + # (this row would newly qualify for the candidates set). + self._phash_candidates = None except Exception as exc: log.warning("deep rephash failed for %s: %s", source, exc) @@ -538,18 +574,7 @@ class Importer: except Exception: phash = None if phash is not None: - cand_rows = self.session.execute( - select( - ImageRecord.phash, - ImageRecord.width, - ImageRecord.height, - ImageRecord.id, - ).where(ImageRecord.phash.is_not(None)) - ).all() - candidates = [ - (c.phash, c.width or 0, c.height or 0, c.id) - for c in cand_rows - ] + candidates = self._phash_candidates_cache() rel, match_id = find_similar( phash, width or 0, height or 0, candidates, self.settings.phash_threshold, @@ -584,6 +609,7 @@ class Importer: record.artist_id = artist.id self.session.add(record) self.session.flush() + self._phash_cache_append(phash, width, height, record.id) # Sidecar provenance (best-effort). When `source` is passed, link # the post to that subscription Source instead of creating a new @@ -799,6 +825,10 @@ class Importer: # created_at intentionally preserved; updated_at auto-bumps. self.session.flush() self.session.commit() + # The phash candidate cache (used to avoid N+1 selects during + # archive imports) is now stale for `existing.id` — the row's + # phash/dimensions changed. Invalidate; the next call re-fetches. + self._phash_candidates = None # Sidecar enrichment from the new (larger) file's location. # _apply_sidecar resolves artist from the sidecar itself if the