discography 'add to wishlist': batch the per-track ownership check (fixes ~15-30s/track)
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.
This commit is contained in:
parent
88b68b8073
commit
50c17ec9f7
3 changed files with 65 additions and 1 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue