From 50c17ec9f73c763104e410078b519faf284d9b52 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 27 Jun 2026 19:51:28 -0700 Subject: [PATCH] discography 'add to wishlist': batch the per-track ownership check (fixes ~15-30s/track) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clicking Download Discography → Add all to wishlist added ~1 track every 15-30s. trace: the endpoint's per-track library-ownership check (track_already_owned → check_track_exists) ran the LEGACY path — firing search_tracks for every title-variation × artist-variation, per track. on a large library and an artist you own NOTHING of, STRATEGY-1 (indexed LIKE) always missed and fell through to the fuzzy fallback (full-table scan), ~10-15 scans/track = the 15-30s. metadata fetch was never the bottleneck (deezer returns each album in ~1s). fix: pre-fetch the artist's owned tracks ONCE (get_candidate_albums_for_artist → get_candidate_tracks_for_albums) and pass candidate_tracks to check_track_exists's batched in-memory path — the same path the discography backfill job + completion-stream already use. pass an EMPTY list (not None) when nothing is owned so the owns-nothing case still takes the fast path → instant. per-track cost drops from ~20s to ~1ms. safe: track_already_owned's only real caller is this endpoint; the new param is optional (None = unchanged legacy behaviour for any other caller). normal ownership still detected (same artist-variation breadth); the one divergence is a track owned ONLY via a compilation → a harmless redundant wishlist add, which is the endpoint's explicitly-accepted failure mode and already how the backfill job behaves. 4 new tests; 1134 discog/metadata/wishlist tests green. --- core/metadata/discography_filters.py | 11 ++++++++ tests/metadata/test_discography_filters.py | 31 ++++++++++++++++++++++ web_server.py | 24 ++++++++++++++++- 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/core/metadata/discography_filters.py b/core/metadata/discography_filters.py index c5985fdf..0761d6d0 100644 --- a/core/metadata/discography_filters.py +++ b/core/metadata/discography_filters.py @@ -167,6 +167,7 @@ def track_already_owned( album_name: str, server_source: Optional[str], confidence_threshold: float = 0.7, + candidate_tracks: Optional[List[Any]] = None, ) -> bool: """Return True if the track is already in the user's library. @@ -186,6 +187,15 @@ def track_already_owned( original deleted) matches just fine — track_name + artist + album don't change with format. + ``candidate_tracks`` (when not None) is the artist's library tracks, + pre-fetched ONCE by the caller, so the check scores in-memory instead + of firing per-track fuzzy SQL scans against the whole library. Pass an + empty list for an artist the user owns nothing of — it still routes + through the fast in-memory path (scores against zero candidates → + instant "not owned") rather than the slow per-track search. None + preserves the original per-track-SQL behaviour for callers that don't + pre-fetch. + Returns False on any exception so a transient DB hiccup doesn't silently nuke a discography fetch — a redundant wishlist add is much cheaper to recover from than a missed track. @@ -198,6 +208,7 @@ def track_already_owned( confidence_threshold=confidence_threshold, server_source=server_source, album=album_name or None, + candidate_tracks=candidate_tracks, ) except Exception: return False diff --git a/tests/metadata/test_discography_filters.py b/tests/metadata/test_discography_filters.py index 8c297d05..cb6ba194 100644 --- a/tests/metadata/test_discography_filters.py +++ b/tests/metadata/test_discography_filters.py @@ -301,6 +301,37 @@ class TestTrackAlreadyOwned: track_already_owned(db, 'Track', 'Artist', 'Album X', 'plex') assert db.calls[0]['album'] == 'Album X' + def test_candidate_tracks_threaded_to_batched_path(self): + """The discography endpoint pre-fetches the artist's owned tracks once + and passes them so check_track_exists scores in-memory instead of firing + per-track fuzzy SQL — the fix for ~15-30s/track on a large library.""" + owned = [SimpleNamespace(title='Owned')] + db = _FakeDB((object(), 0.9)) + track_already_owned( + db, 'Owned', 'Artist', 'Album', 'plex', candidate_tracks=owned, + ) + # The pre-fetched candidates must reach check_track_exists verbatim. + assert db.calls[0]['candidate_tracks'] is owned + + def test_empty_candidate_list_still_uses_batched_path(self): + """Owns-nothing case: an EMPTY list (not None) must be forwarded so the + check takes the fast in-memory path (scores against zero candidates → + instant 'not owned') instead of falling back to the slow per-track SQL.""" + db = _FakeDB((None, 0.0)) + result = track_already_owned( + db, 'Anything', 'Artist', 'Album', 'plex', candidate_tracks=[], + ) + assert result is False + # [] is forwarded (not coerced to None) — that's what keeps it fast. + assert db.calls[0]['candidate_tracks'] == [] + + def test_default_omits_candidate_tracks_for_legacy_callers(self): + """Callers that don't pre-fetch get None → check_track_exists keeps its + original per-track-SQL behaviour. No other caller is forced to change.""" + db = _FakeDB((object(), 0.9)) + track_already_owned(db, 'Track', 'Artist', 'Album', 'plex') + assert db.calls[0]['candidate_tracks'] is None + def test_passes_server_source_to_check(self): """Active media server scopes the lookup so the skip check only fires on tracks the user can actually see in their diff --git a/web_server.py b/web_server.py index 2a470c71..34aa2044 100644 --- a/web_server.py +++ b/web_server.py @@ -9714,6 +9714,27 @@ def download_discography(artist_id): except Exception as e: logger.debug("active media server lookup failed: %s", e) + # Pre-fetch the artist's owned library tracks ONCE so the per-track + # ownership check scores in-memory instead of firing fuzzy SQL scans + # against the whole library for every track (which, on a large library + # and an artist the user owns nothing of, was ~15-30s PER TRACK — every + # title/artist variation fell through to a full-table fuzzy fallback). + # Same batched path the discography backfill job + completion-stream use. + # Crucially we pass an empty list (not None) when nothing is owned, so the + # owns-nothing case still takes the fast in-memory path → instant. + owned_candidate_tracks = [] + try: + cand_albums = db.get_candidate_albums_for_artist( + artist_name, server_source=active_server + ) + if cand_albums: + owned_candidate_tracks = db.get_candidate_tracks_for_albums( + [a.id for a in cand_albums] + ) or [] + except Exception as _cand_err: + logger.debug("Discography: candidate pre-fetch failed for %s: %s", artist_name, _cand_err) + owned_candidate_tracks = [] + total_added = 0 total_skipped = 0 total_skipped_artist = 0 @@ -9803,7 +9824,8 @@ def download_discography(artist_id): # Same library-ownership check the discography # backfill repair job uses. Format-agnostic so # Blasphemy mode (FLAC→MP3) doesn't false-miss. - if track_already_owned(db, track_name, hint_artist, album_name, active_server): + if track_already_owned(db, track_name, hint_artist, album_name, active_server, + candidate_tracks=owned_candidate_tracks): skipped_owned += 1 continue