refactor(artwork): drop the unused Bump/wake path from the worker

Worker.Bump had no production callers. Every real bump writes the queue row
through the repository instead: artwork.Refresh (the refresh endpoint and the
uploader), service.enqueue (read-through and stale-absent views), and
radio_repository. None of them wake a pool.

Bump was also the only sender to drainPool.wake, so the select case it fed was
unreachable outside tests, and runPool read as if a bump were picked up
promptly when it always waited for the 5s poll.

The e2e and unit suites used Bump only as a driver, never as the subject, so
they now enqueue with EnqueueBump and exercise the path production takes.
EnqueueBump rather than Refresh because Refresh also clears the state row,
which would change semantics for the re-bump-after-state and dedup specs.

Both harnesses start Run after enqueueing, so a fresh pool drains on its first
iteration and the wake never affected them: core/artwork/e2e averaged 3.26s
before and 3.30s after over 5 runs, within noise.

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2026-07-30 11:33:30 -04:00
parent 97035c4e1c
commit d25b11dd53
5 changed files with 32 additions and 47 deletions

View File

@ -52,6 +52,14 @@ var _ = Describe("Acquisition → serve loop", func() {
return err == nil && ia.Hash == ""
}
}
// Enqueues the way the serving paths do, so the drain is driven by a plain queue row.
bump := func(kind, id string) {
GinkgoHelper()
Expect(ds.ArtworkQueue(ctx).EnqueueBump(model.ArtworkQueueItem{
ItemKind: kind, ItemID: id, ImageType: model.ImageTypePrimary,
Priority: model.ArtworkPriorityBump,
})).To(Succeed())
}
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())
@ -109,7 +117,7 @@ var _ = Describe("Acquisition → serve loop", func() {
It("acquires album folder art and serves the exact bytes under its hash", func() {
seedFolderAlbum("al1")
worker.Bump("al", "al1")
bump("al", "al1")
runWorkerUntil(ctx, worker, itemFound(model.KindAlbumArtwork, "al1"))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al1", model.ImageTypePrimary)
@ -126,7 +134,7 @@ var _ = Describe("Acquisition → serve loop", func() {
It("acquires an artist's uploaded image and serves it", func() {
name := writeUpload(consts.EntityArtist, "artist-e2e.png", artistPngFixture)
artistRepo.SetData(model.Artists{{ID: "ar1", Name: "Artist", UploadedImage: name}})
worker.Bump("ar", "ar1")
bump("ar", "ar1")
runWorkerUntil(ctx, worker, itemFound(model.KindArtistArtwork, "ar1"))
ia, err := artRepo.GetItemArtwork(model.KindArtistArtwork, "ar1", model.ImageTypePrimary)
@ -143,7 +151,7 @@ var _ = Describe("Acquisition → serve loop", func() {
seedFolderAlbum("al1")
plRepo.SetData(model.Playlists{{ID: "pl1", Name: "Playlist"}})
plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"al1"}}
worker.Bump("pl", "pl1")
bump("pl", "pl1")
runWorkerUntil(ctx, worker, itemFound(model.KindPlaylistArtwork, "pl1"))
ia, err := artRepo.GetItemArtwork(model.KindPlaylistArtwork, "pl1", model.ImageTypePrimary)
@ -162,7 +170,7 @@ var _ = Describe("Acquisition → serve loop", func() {
It("acquires a radio station's uploaded image and serves it", func() {
name := writeUpload(consts.EntityRadio, "radio-e2e.jpg", coverFixture)
radioRepo.Data["ra1"] = &model.Radio{ID: "ra1", Name: "Station", UploadedImage: name}
worker.Bump("ra", "ra1")
bump("ra", "ra1")
runWorkerUntil(ctx, worker, itemFound(model.KindRadioArtwork, "ra1"))
ia, err := artRepo.GetItemArtwork(model.KindRadioArtwork, "ra1", model.ImageTypePrimary)
@ -205,7 +213,7 @@ var _ = Describe("Acquisition → serve loop", func() {
It("stores dimensions, mime and a real blurhash alongside the acquired bytes", func() {
seedFolderAlbum("al1")
worker.Bump("al", "al1")
bump("al", "al1")
runWorkerUntil(ctx, worker, itemFound(model.KindAlbumArtwork, "al1"))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al1", model.ImageTypePrimary)
@ -222,7 +230,7 @@ var _ = Describe("Acquisition → serve loop", func() {
It("acquires GIF artwork, whose decoder only core/artwork's blank import registers", func() {
writeUploadedImage(consts.EntityRadio, "station.gif", gifFixture)
radioRepo.Data["ra1"] = &model.Radio{ID: "ra1", Name: "Station", UploadedImage: "station.gif"}
worker.Bump("ra", "ra1")
bump("ra", "ra1")
runWorkerUntil(ctx, worker, itemFound(model.KindRadioArtwork, "ra1"))
ia, err := artRepo.GetItemArtwork(model.KindRadioArtwork, "ra1", model.ImageTypePrimary)
@ -239,8 +247,8 @@ var _ = Describe("Acquisition → serve loop", func() {
{ID: "al1", Name: "Album", FolderIDs: []string{"f1"}, LibraryID: 0},
{ID: "al2", Name: "Same Cover", FolderIDs: []string{"f1"}, LibraryID: 0},
})
worker.Bump("al", "al1")
worker.Bump("al", "al2")
bump("al", "al1")
bump("al", "al2")
runWorkerUntil(ctx, worker, func() bool {
return itemFound(model.KindAlbumArtwork, "al1")() && itemFound(model.KindAlbumArtwork, "al2")()
})
@ -256,7 +264,7 @@ var _ = Describe("Acquisition → serve loop", func() {
It("stops serving a file-backed image once its source file changes underneath", func() {
name := writeUpload(consts.EntityRadio, "radio-stale.jpg", coverFixture)
radioRepo.Data["ra1"] = &model.Radio{ID: "ra1", Name: "Station", UploadedImage: name}
worker.Bump("ra", "ra1")
bump("ra", "ra1")
runWorkerUntil(ctx, worker, itemFound(model.KindRadioArtwork, "ra1"))
ia, err := artRepo.GetItemArtwork(model.KindRadioArtwork, "ra1", model.ImageTypePrimary)
@ -284,7 +292,7 @@ var _ = Describe("Acquisition → serve loop", func() {
It("records an absent state for an entity with no art and reports it unavailable", func() {
albumRepo.SetData(model.Albums{{ID: "alx", Name: "Artless", LibraryID: 0}})
worker.Bump("al", "alx")
bump("al", "alx")
runWorkerUntil(ctx, worker, itemAbsent(model.KindAlbumArtwork, "alx"))
_, err := svc.Get(ctx, model.MustParseArtworkID("al-alx"), 0, false)

View File

@ -139,7 +139,11 @@ func scan() {
func acquire(kind model.Kind, id string) model.ItemArtwork {
GinkgoHelper()
rworker.Bump(kind.Prefix(), id)
// Enqueues the way the serving paths do, so the drain is driven by a plain queue row.
Expect(rds.ArtworkQueue(rctx).EnqueueBump(model.ArtworkQueueItem{
ItemKind: kind.Prefix(), ItemID: id, ImageType: model.ImageTypePrimary,
Priority: model.ArtworkPriorityBump,
})).To(Succeed())
var ia *model.ItemArtwork
runResolutionWorkerUntil(func() bool {
got, err := rds.Artwork(rctx).GetItemArtwork(kind, id, model.ImageTypePrimary)

View File

@ -34,7 +34,6 @@ type drainPool struct {
name string
kinds []string
concurrency int
wake chan struct{}
}
// Worker drains the artwork queue: each external agent is rate-limited and circuit-broken
@ -75,8 +74,8 @@ func newDrainPools() []*drainPool {
// More external slots than the rate allows would only sleep in the limiter.
external := min(max(2, 2*conf.Server.DevArtworkExternalMaxRPS), budget-local)
return []*drainPool{
{name: "local", kinds: localDrainKinds, concurrency: local, wake: make(chan struct{}, 1)},
{name: "external", kinds: externalDrainKinds, concurrency: external, wake: make(chan struct{}, 1)},
{name: "local", kinds: localDrainKinds, concurrency: local},
{name: "external", kinds: externalDrainKinds, concurrency: external},
}
}
@ -120,28 +119,6 @@ func (w *Worker) runPool(ctx context.Context, p *drainPool) {
case <-ctx.Done():
return
case <-ticker.C:
case <-p.wake:
}
}
}
// Bump enqueues an item at the highest priority and wakes the drain loops.
func (w *Worker) Bump(kind, id string) {
item := model.ArtworkQueueItem{
ItemKind: kind,
ItemID: id,
ImageType: model.ImageTypePrimary,
Priority: model.ArtworkPriorityBump,
}
if err := w.proc.ds.ArtworkQueue(context.Background()).Enqueue(item); err != nil {
log.Warn("Artwork: Could not bump queue item", "kind", kind, "id", id, err)
return
}
// Wake every pool: a spurious wake only costs an empty dequeue.
for _, p := range w.pools {
select {
case p.wake <- struct{}{}:
default:
}
}
}

View File

@ -500,15 +500,6 @@ var _ = Describe("Worker", func() {
})
})
Describe("Bump", func() {
It("enqueues at Bump priority and wakes the loop", func() {
w.Bump("al", "al9")
it := findQueued(queueRepo, "al", "al9")
Expect(it).ToNot(BeNil())
Expect(it.Priority).To(Equal(model.ArtworkPriorityBump))
})
})
Describe("gate/breaker", func() {
It("opens after 5 consecutive external errors and short-circuits the step", func() {
var calls int

View File

@ -146,8 +146,13 @@ var _ = Describe("Artwork Serving", Ordered, func() {
})
It("drains the queue: folder art is acquired, the artless album settles absent", func() {
worker.Bump("al", artfulID)
worker.Bump("al", artlessID)
// Enqueues the way the serving paths do, so the drain is driven by a plain queue row.
for _, id := range []string{artfulID, artlessID} {
Expect(ds.ArtworkQueue(ctx).EnqueueBump(model.ArtworkQueueItem{
ItemKind: model.KindAlbumArtwork.Prefix(), ItemID: id,
ImageType: model.ImageTypePrimary, Priority: model.ArtworkPriorityBump,
})).To(Succeed())
}
runWorkerUntil(ctx, worker, func() bool {
found, err := ds.Artwork(ctx).GetItemArtwork(model.KindAlbumArtwork, artfulID, model.ImageTypePrimary)
if err != nil || found.Hash == "" {