stats: normalize image URLs at cache-build time, not per /api/stats/cached read (#935)

reported (radoslav-orlov): the Stats page hangs ~20s on a 16GB/HDD box, GET /api/stats/cached
?range=7d -> 200 in ~20000ms, while it's instant on Boulder's SSD. the endpoint is documented
"instant" — it reads 3 small precomputed metadata blobs — but it then ran every top
artist/album/track image through normalize_image_url ON THE READ. that fixer calls
cache_url_for, which does a SQLite INSERT/UPDATE + commit under a global lock PER image. on an
HDD each commit fsyncs and contends with the background image fetcher -> ~20s for ~50 images.
(this is exactly the "image caching when we open a page" ramonskie flagged in the thread.)

fix: do the normalization in the background ListeningStatsWorker when it builds the cache
(_enrich_stats_items), so the cache stores browser-ready /api/image-cache/... URLs and the read
does ZERO per-image work. the read-path fixer stays as a cheap no-op (normalize_image_url early-
returns on already-proxied URLs), so an old raw-URL cache self-heals on the next rebuild with no
broken art in between. the registration writes now happen once per rebuild, off the hot path.

2 tests (existing enrich test relaxed to truthy since the value is normalized now; new
deterministic test stubs the fixer to prove every section's url is fixed at build time).
136 stats/listening/image tests green.
This commit is contained in:
BoulderBadgeDad 2026-06-28 09:32:48 -07:00
parent d62afbb2df
commit 77e3673c9c
2 changed files with 42 additions and 4 deletions

View file

@ -291,6 +291,13 @@ class ListeningStatsWorker:
if not (top_artists or top_albums or top_tracks):
return
# Normalize image URLs HERE, at cache-build time, not on every /api/stats/cached
# read. normalize_image_url registers each URL in the image cache (a SQLite write
# under a lock) — doing that per-request made the "instant" stats endpoint take ~20s
# on HDD-backed installs (#935). Done once per background rebuild it's off the hot path,
# and the read just returns the already-browser-safe URLs.
from core.metadata import normalize_image_url as _fix_image
conn = None
try:
conn = self.db._get_connection()
@ -324,7 +331,7 @@ class ListeningStatsWorker:
key = (artist.get('name') or '').lower()
r = artist_rows.get(key)
if r:
artist['image_url'] = r[1] or None
artist['image_url'] = _fix_image(r[1]) or None
artist['id'] = r[2]
artist['global_listeners'] = r[3]
artist['global_playcount'] = r[4]
@ -356,7 +363,7 @@ class ListeningStatsWorker:
key = (album.get('name') or '').lower()
r = album_rows.get(key)
if r:
album['image_url'] = r[1] or None
album['image_url'] = _fix_image(r[1]) or None
album['id'] = r[2]
album['artist_id'] = r[3]
@ -395,7 +402,7 @@ class ListeningStatsWorker:
(track.get('artist') or '').lower())
r = track_rows.get(key)
if r:
track['image_url'] = r[2] or None
track['image_url'] = _fix_image(r[2]) or None
track['id'] = r[3]
track['artist_id'] = r[4]
except Exception as e:

View file

@ -218,13 +218,44 @@ class TestEnrichStatsItems:
album_by_name = {a["name"]: a for a in cache["top_albums"]}
assert album_by_name["First Album"]["id"] == "al1"
assert album_by_name["First Album"]["image_url"] == "http://img/al1.jpg"
# image_url is normalized at build time now (#935) — it's enriched (truthy);
# exact form depends on the image-cache config, so don't pin the raw value.
assert album_by_name["First Album"]["image_url"]
track_by_name = {t["name"]: t for t in cache["top_tracks"]}
assert track_by_name["Alpha"]["id"] == "t1"
assert track_by_name["Bravo"]["id"] == "t2"
assert track_by_name["Alpha"]["artist_id"] == "a1"
def test_image_urls_normalized_at_build_time(self, db, worker, monkeypatch):
"""#935: image URLs are run through the fixer HERE, at cache-build time, so the
/api/stats/cached read does zero per-image image-cache writes on the hot path
(those writes were the ~20s stats hang on HDD-backed installs). Deterministic
via a stub fixer so it doesn't depend on the real image-cache config."""
_insert_track(db, "t1", "Alpha", "a1", "Band One", "al1", "First Album")
seen = []
def _fake_fix(url):
seen.append(url)
return f"/api/image-cache/fixed::{url}"
monkeypatch.setattr("core.metadata.normalize_image_url", _fake_fix)
cache = {
"top_artists": [{"name": "Band One"}],
"top_albums": [{"name": "First Album"}],
"top_tracks": [{"name": "Alpha", "artist": "Band One"}],
}
worker._enrich_stats_items(cache)
# every section's raw thumb_url was passed through the fixer, and the cached
# value is the fixed (browser-safe) url — so the read path has nothing to do.
assert cache["top_albums"][0]["image_url"] == "/api/image-cache/fixed::http://img/al1.jpg"
assert cache["top_artists"][0]["image_url"].startswith("/api/image-cache/fixed::")
assert cache["top_tracks"][0]["image_url"].startswith("/api/image-cache/fixed::")
assert any("al1" in str(s) for s in seen)
def test_unknown_entries_left_untouched(self, db, worker):
_insert_track(db, "t1", "Real", "a1", "Real Band", "al1", "Real Album")