mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
fix(artwork): keep served art when the retry budget runs out
Exhausting the 12h budget called writeAbsent unconditionally, so an entity whose art was already resolved and serving lost it to a long upstream outage: the hash went empty, clients fell back to the placeholder, and the now-unreferenced bytes were freed by the next prune even though nothing about the image had changed. Exhaustion means the source stayed unreachable, not that the cover disappeared, so absent is now recorded only when there is nothing to keep.
This commit is contained in:
parent
381dce0363
commit
2207211997
@ -228,7 +228,9 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) outco
|
||||
}
|
||||
// Retry budget exhausted: stop retrying. A bare failure settles absent so the stale-absent
|
||||
// sweep (and a page view) can still recover it; a stale-found keeps its already-served art.
|
||||
if out == outcomeFailed {
|
||||
// Art we are already serving is kept too: exhaustion means the source stayed unreachable,
|
||||
// not that the entity lost its cover.
|
||||
if out == outcomeFailed && !w.hasResolvedArtwork(ctx, item) {
|
||||
writeAbsent(ctx, w.deps.ds.Artwork(ctx), item)
|
||||
}
|
||||
if err := queue.DeleteIfUnchanged(item.ItemKind, item.ItemID, item.ImageType, item.RetryAt); err != nil {
|
||||
@ -238,6 +240,16 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) outco
|
||||
return out
|
||||
}
|
||||
|
||||
// hasResolvedArtwork reports whether the item already has a hash-bearing state row.
|
||||
func (w *Worker) hasResolvedArtwork(ctx context.Context, item model.ArtworkQueueItem) bool {
|
||||
kind, ok := model.ParseKind(item.ItemKind)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
ia, err := w.deps.ds.Artwork(ctx).GetItemArtwork(kind, item.ItemID, item.ImageType)
|
||||
return err == nil && ia.Hash != ""
|
||||
}
|
||||
|
||||
// precache warms the resize cache for a newly-acquired image at the UI cover size, so the
|
||||
// first UI request is a cache hit. Skipped when disabled; failures are debug-only.
|
||||
func (w *Worker) precache(ctx context.Context, item model.ArtworkQueueItem) {
|
||||
|
||||
@ -336,6 +336,33 @@ var _ = Describe("Worker", func() {
|
||||
Expect(ia.Hash).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("keeps already-served art when the retry budget is exhausted", func() {
|
||||
conf.Server.CoverArtPriority = "external"
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "al10", Name: "Album"}})
|
||||
Expect(artRepo.PutItemArtwork(&model.ItemArtwork{
|
||||
ItemKind: "al", ItemID: "al10", ImageType: model.ImageTypePrimary,
|
||||
Hash: "cafebabe", Source: "external:lastfm",
|
||||
})).To(Succeed())
|
||||
imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")})
|
||||
w = NewWorker(ds, store, ag, ffm, broker, imgCache)
|
||||
Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ItemKind: "al", ItemID: "al10"})).To(Succeed())
|
||||
for k, v := range queueRepo.Data {
|
||||
if v.ItemID == "al10" {
|
||||
v.EnqueuedAt = time.Now().Add(-(giveUpAfter + time.Hour))
|
||||
queueRepo.Data[k] = v
|
||||
}
|
||||
}
|
||||
|
||||
n, err := w.drain(ctx, 1)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(n).To(Equal(1))
|
||||
|
||||
Expect(findQueued(queueRepo, "al", "al10")).To(BeNil())
|
||||
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al10", model.ImageTypePrimary)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(ia.Hash).To(Equal("cafebabe"), "a persistent outage must not discard served art")
|
||||
})
|
||||
|
||||
It("resolves a private playlist under an admin context instead of failing forever", func() {
|
||||
ds.MockedUser = adminUserRepo()
|
||||
vds := &visibilityPlaylistDS{
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user