fix(explore): let a slow archive node finish, and read the 404 back
Explore's album art was almost entirely missing: 5 of 24 cards on the shelves had a cover, and those five were the ones already on disk. The Cover Art Archive answers `front-250` with a 307 to an Internet Archive storage node, and those nodes are slow. Measured against the twelve albums on Explore's own shelves, a successful fetch took 14-16 s and a failing one 13-17 s, against a client timeout of 10. So every live fetch died, and a timeout writes nothing and says nothing -- which is why this reads as "Explore has no album art" rather than as a slow upstream. The timeout is 30 s, chosen to clear the measured range: the fetch is off the critical path, so waiting costs nothing and giving up early costs the whole page. Two things beside it, both found on the way. `writeCache(mbid, nil)` has recorded "the archive has no art for this" as an empty file since it was written, and nothing has ever read it back: `readCache` returns "" for an empty file, which is indistinguishable from a miss. So every art-less release group was re-fetched from CAA on every render that asked about it. A third of the shelves are art-less, so that was a third of the page spending a live request to be told again what the last one said. `knownMissing` reads it, on both the release-group and the release path. And the frontend marked a failed fetch as permanently answered for the session, so a timed-out cover never retried within it. It drops the marker instead; a genuine 404 is now answered from disk, so re-asking one costs nothing. Measured after: 23 of 24. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MeQt5hgXg5YGoNZQ9ozG7L
This commit is contained in:
@@ -25,8 +25,26 @@ const (
|
||||
// where cached cover art thumbnails are stored.
|
||||
thumbnailDir = CoverArtCacheDirName
|
||||
|
||||
// thumbnailTimeout is the HTTP timeout for fetching a thumbnail.
|
||||
thumbnailTimeout = 10 * time.Second
|
||||
// thumbnailTimeout is the HTTP timeout for fetching a thumbnail,
|
||||
// and it has to cover a redirect the Cover Art Archive does not
|
||||
// serve itself.
|
||||
//
|
||||
// `coverartarchive.org` answers `front-250` with a 307 to an
|
||||
// Internet Archive storage node (`dn######.us.archive.org`), and
|
||||
// those nodes are routinely slow: measured against the twelve
|
||||
// albums on Explore's own shelves, a successful fetch took 14–16 s
|
||||
// and a failing one 13–17 s. At 10 s *every* cover on the page
|
||||
// timed out — 24 cards, 5 of which had art, all of those from the
|
||||
// disk cache — which reads as "Explore has no album art" rather
|
||||
// than as a slow upstream, because a timeout writes nothing and
|
||||
// says nothing.
|
||||
//
|
||||
// 30 s is chosen to clear that measured range with room, not to be
|
||||
// generous: the fetch is off the critical path (each one is its own
|
||||
// goroutine behind an 8/s limiter, and the frontend renders a
|
||||
// placeholder until it lands), so the cost of waiting is nothing
|
||||
// and the cost of giving up early is a blank page.
|
||||
thumbnailTimeout = 30 * time.Second
|
||||
|
||||
// thumbnailMaxSize is the maximum image size to cache (2 MB).
|
||||
thumbnailMaxSize = 2 * 1024 * 1024
|
||||
@@ -97,6 +115,20 @@ func (p *CoverArtProxy) GetThumbnail(
|
||||
return ""
|
||||
}
|
||||
|
||||
// A 404 is an answer, and it is already on disk.
|
||||
//
|
||||
// `writeCache(mbid, nil)` has recorded "the archive has no art for
|
||||
// this" as an empty file since this was written, and nothing has
|
||||
// ever read it back: `readCache` returns "" for an empty file,
|
||||
// which is indistinguishable from a miss, so every art-less release
|
||||
// group was re-fetched from the network on every render that asked
|
||||
// about it. On Explore's shelves a third of the cards are art-less,
|
||||
// so that was a third of the page spending a live CAA request to be
|
||||
// told again what the last one said.
|
||||
if p.knownMissing(releaseGroupMBID) {
|
||||
return ""
|
||||
}
|
||||
|
||||
// Source 3: fetch from Cover Art Archive (slow, cached to disk).
|
||||
url := CoverArtGroupURL(releaseGroupMBID)
|
||||
data, cacheable, err := p.fetch(url)
|
||||
@@ -177,8 +209,9 @@ func (p *CoverArtProxy) GetCandidateThumbnail(
|
||||
}
|
||||
}
|
||||
|
||||
// Network fetch on release group.
|
||||
if releaseGroupMBID != "" {
|
||||
// Network fetch on release group — unless a previous one was told
|
||||
// there is none. See `knownMissing`.
|
||||
if releaseGroupMBID != "" && !p.knownMissing(releaseGroupMBID) {
|
||||
url := CoverArtGroupURL(releaseGroupMBID)
|
||||
data, cacheable, err := p.fetch(url)
|
||||
|
||||
@@ -194,7 +227,7 @@ func (p *CoverArtProxy) GetCandidateThumbnail(
|
||||
}
|
||||
|
||||
// Network fetch on release (fallback).
|
||||
if releaseMBID != "" {
|
||||
if releaseMBID != "" && !p.knownMissing(releaseMBID) {
|
||||
url := CoverArtURL(releaseMBID)
|
||||
data, cacheable, err := p.fetch(url)
|
||||
|
||||
@@ -285,6 +318,17 @@ func (p *CoverArtProxy) cachePath(mbid string) string {
|
||||
return filepath.Join(p.cacheDir, mbid+".jpg")
|
||||
}
|
||||
|
||||
// knownMissing reports whether a previous fetch was told the archive
|
||||
// has no art for this MBID — the empty file `writeCache(mbid, nil)`
|
||||
// leaves behind. It is deliberately separate from `readCache`, which
|
||||
// answers "what are the bytes" and cannot express the difference
|
||||
// between no answer and an answer of none.
|
||||
func (p *CoverArtProxy) knownMissing(mbid string) bool {
|
||||
info, err := os.Stat(p.cachePath(mbid))
|
||||
|
||||
return err == nil && info.Size() == 0
|
||||
}
|
||||
|
||||
func (p *CoverArtProxy) readCache(mbid string) string {
|
||||
path := p.cachePath(mbid)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user