From 295886cb9a432d375afa796ed8a32db0775464af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 21 Aug 2026 20:27:42 -0400 Subject: [PATCH] feat(artwork): report what a config-fingerprint backfill enqueued (#6010) * feat(artwork): report what a config-fingerprint backfill enqueued A backfill re-resolves every entity, and on a large library that is tens of thousands of external agent calls. It announced itself with a single line carrying nothing but an elapsed time, so the size of the job was invisible until the request volume showed up hours later. Log the item count, the per-kind breakdown, and a ceiling on the external lookups the queued work can cost. The ceiling reuses ExternalLookupsPerItem, the same estimator behind the `artwork reprocess` preview, so the two agree on what an item can cost. backfill now returns a summary instead of a bare bool, which keeps the counts assertable without capturing log output. Worker.Backfill keeps its (bool, error) signature, so its caller is unchanged, and it reads the agent count off its own resolver. * refactor(artwork): share the image-agent count and take it lazily Counting image agents was written twice, once in the CLI for the `artwork reprocess` preview and again as a resolver method for the backfill log. Two copies of "which agents count as image agents" can drift, and the CLI estimate and the server log would then disagree silently. Move the derivation next to the type it builds, as NewImageAgentCount, and call it from both. The resolver method goes away with it: hanging the census on the resolver forced two nil guards that its only caller could never trigger, because Worker always builds a resolver with agents. The Worker keeps the *agents.Agents it is already handed instead of reaching through the processor and resolver to find it. Pass the count as a func. Building the agent list constructs every enabled agent (each one an HTTP client and a cache goroutine) only to take its length, and a backfill returns early on an unchanged fingerprint, which is what happens on nearly every restart. * docs(artwork): say what the backfill lookup estimate does not bound The comment called the number a ceiling, which the CLI comment on the same estimate already contradicts: externalEstimate "claims no bound". Both are right about the local-source case and only one of them mentions that a retried item asks its agents again. --- cmd/artwork.go | 7 +----- core/artwork/housekeeping.go | 35 ++++++++++++++++++++------- core/artwork/housekeeping_test.go | 39 ++++++++++++++++++++++++------- core/artwork/resolve.go | 9 +++++++ core/artwork/worker.go | 7 ++++-- 5 files changed, 73 insertions(+), 24 deletions(-) diff --git a/cmd/artwork.go b/cmd/artwork.go index 01de071bb..8b9e28f0e 100644 --- a/cmd/artwork.go +++ b/cmd/artwork.go @@ -339,7 +339,7 @@ func runReprocess(ctx context.Context) { if needsImageAgents(kinds) { mgr := loadPluginAgents(ctx, false) defer func() { _ = mgr.Stop() }() - imageAgents = imageAgentCount(ds, mgr) + imageAgents = artwork.NewImageAgentCount(agents.GetAgents(ds, mgr)) } if err := reprocessArtwork(ctx, ds, kinds, repositorySources(artworkSources), imageAgents, @@ -397,11 +397,6 @@ func externalLookupLine(n int64) string { return fmt.Sprintf("External lookups: %s.", externalEstimate(n)) } -func imageAgentCount(ds model.DataStore, mgr *plugins.Manager) artwork.ImageAgentCount { - ag := agents.GetAgents(ds, mgr) - return artwork.ImageAgentCount{Artist: len(ag.ArtistImageAgents()), Album: len(ag.AlbumImageAgents())} -} - // loadPluginAgents loads the plugins named in Agents, so the CLI resolves through the same agents a // running server would. A load failure is reported, not fatal: the built-in agents still answer. func loadPluginAgents(ctx context.Context, runInit bool) *plugins.Manager { diff --git a/core/artwork/housekeeping.go b/core/artwork/housekeeping.go index fed5757c8..9456a5584 100644 --- a/core/artwork/housekeeping.go +++ b/core/artwork/housekeeping.go @@ -72,18 +72,27 @@ func ConfigFingerprint() string { return fmt.Sprintf("%016x", xxh3.Hash([]byte(raw))) } +// backfillSummary is what a backfill enqueued. MaxExternalLookups is an upper estimate for one +// attempt per item, not a bound: a local hit ends the walk, and a retry asks the agents again. +type backfillSummary struct { + Ran bool + PerKind map[string]int64 + Items int64 + MaxExternalLookups int64 +} + // backfill enqueues artwork resolution for every entity when the config fingerprint changed. -func backfill(ctx context.Context, ds model.DataStore) (bool, error) { +func backfill(ctx context.Context, ds model.DataStore, agentCount func() ImageAgentCount) (backfillSummary, error) { start := time.Now() ctx = auth.WithAdminUser(ctx, ds) current := ConfigFingerprint() props := ds.Property(ctx) stored, err := props.DefaultGet(consts.ArtConfFingerprintPropertyKey, "") if err != nil { - return false, err + return backfillSummary{}, err } if stored == current { - return false, nil + return backfillSummary{}, nil } // Artists first: few entities, most external-dependent, so they get a queue headstart. @@ -96,21 +105,31 @@ func backfill(ctx context.Context, ds model.DataStore) (bool, error) { {model.KindPlaylistArtwork, func() ([]string, error) { return ds.Playlist(ctx).GetAllIDs() }}, {model.KindRadioArtwork, func() ([]string, error) { return ds.Radio(ctx).GetAllIDs() }}, } + // Counted here, not by the caller: building the agent list constructs every enabled agent, and + // an unchanged fingerprint returns above without ever needing the number. + agents := agentCount() + summary := backfillSummary{Ran: true, PerKind: map[string]int64{}} for _, k := range kinds { ids, err := k.fetch() if err != nil { - return false, err + return backfillSummary{}, err } if err := enqueueBackfillKind(ctx, ds, k.kind, ids); err != nil { - return false, err + return backfillSummary{}, err } + n := int64(len(ids)) + summary.PerKind[k.kind.Prefix()] = n + summary.Items += n + summary.MaxExternalLookups += n * ExternalLookupsPerItem(k.kind, agents) } if err := props.Put(consts.ArtConfFingerprintPropertyKey, current); err != nil { - return false, err + return backfillSummary{}, err } - log.Info(ctx, "Artwork: Config fingerprint changed, backfill enqueued", "elapsed", time.Since(start)) - return true, nil + log.Info(ctx, "Artwork: Config fingerprint changed, backfill enqueued", "items", summary.Items, + "byKind", summary.PerKind, "maxExternalLookups", summary.MaxExternalLookups, + "elapsed", time.Since(start)) + return summary, nil } func enqueueBackfillKind(ctx context.Context, ds model.DataStore, kind model.Kind, ids []string) error { diff --git a/core/artwork/housekeeping_test.go b/core/artwork/housekeeping_test.go index 4f229bff8..7aecd2760 100644 --- a/core/artwork/housekeeping_test.go +++ b/core/artwork/housekeeping_test.go @@ -39,6 +39,8 @@ func adminUserRepo() *tests.MockedUserRepo { return repo } +func noAgents() ImageAgentCount { return ImageAgentCount{} } + // orderTrackingQueueRepo records the item kind of each Enqueue call, so tests can // assert phase ordering (artists-first) that same-priority timestamps can't guarantee. type orderTrackingQueueRepo struct { @@ -164,9 +166,14 @@ var _ = Describe("Housekeeping", func() { seedEntities() Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, ConfigFingerprint())).To(Succeed()) - did, err := backfill(ctx, ds) + counted := false + s, err := backfill(ctx, ds, func() ImageAgentCount { + counted = true + return ImageAgentCount{Artist: 3, Album: 2} + }) Expect(err).ToNot(HaveOccurred()) - Expect(did).To(BeFalse()) + Expect(s).To(Equal(backfillSummary{})) + Expect(counted).To(BeFalse(), "building the agent list constructs every agent; an unchanged fingerprint must not pay for it") count, err := queueRepo.Count() Expect(err).ToNot(HaveOccurred()) @@ -176,9 +183,9 @@ var _ = Describe("Housekeeping", func() { It("runs the backfill when no fingerprint was ever stored", func() { seedEntities() - did, err := backfill(ctx, ds) + s, err := backfill(ctx, ds, noAgents) Expect(err).ToNot(HaveOccurred()) - Expect(did).To(BeTrue()) + Expect(s.Ran).To(BeTrue()) count, err := queueRepo.Count() Expect(err).ToNot(HaveOccurred()) @@ -197,9 +204,9 @@ var _ = Describe("Housekeeping", func() { tracks: &tests.MockPlaylistTrackRepo{}, } - did, err := backfill(ctx, vds) + s, err := backfill(ctx, vds, noAgents) Expect(err).ToNot(HaveOccurred()) - Expect(did).To(BeTrue()) + Expect(s.Ran).To(BeTrue()) Expect(findQueued(queueRepo.MockArtworkQueueRepo, "pl", "plPrivate")).ToNot(BeNil()) }) @@ -207,9 +214,9 @@ var _ = Describe("Housekeeping", func() { seedEntities() Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, "stale-fingerprint")).To(Succeed()) - did, err := backfill(ctx, ds) + s, err := backfill(ctx, ds, noAgents) Expect(err).ToNot(HaveOccurred()) - Expect(did).To(BeTrue()) + Expect(s.Ran).To(BeTrue()) Expect(queueRepo.callKinds).ToNot(BeEmpty()) firstOther := slices.IndexFunc(queueRepo.callKinds, func(k string) bool { return k != "ar" }) @@ -224,6 +231,22 @@ var _ = Describe("Housekeeping", func() { Expect(it.ItemKind).To(BeElementOf("ar", "al", "pl", "ra")) } }) + + It("reports what it enqueued, per kind and as an external-lookup ceiling", func() { + conf.Server.ArtistArtPriority = "artist.*, external" + conf.Server.CoverArtPriority = "cover.*, external" + conf.Server.EnableM3UExternalAlbumArt = false + seedEntities() + + s, err := backfill(ctx, ds, func() ImageAgentCount { return ImageAgentCount{Artist: 3, Album: 2} }) + Expect(err).ToNot(HaveOccurred()) + Expect(s.Ran).To(BeTrue()) + + Expect(s.PerKind).To(Equal(map[string]int64{"ar": 2, "al": 1, "pl": 1, "ra": 1})) + Expect(s.Items).To(Equal(int64(5))) + // 2 artists x 3 agents, 1 album x 2, 1 playlist grid x 2, and radios never fetch. + Expect(s.MaxExternalLookups).To(Equal(int64(6 + 2 + PlaylistGridSamples*2))) + }) }) Describe("EnqueueStaleAbsentAll", func() { diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index f42beb9f1..6663679fa 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -137,6 +137,15 @@ func MayFetchExternal(kind model.Kind) bool { // ImageAgentCount is how many enabled agents provide artist and album images. type ImageAgentCount struct{ Artist, Album int } +// NewImageAgentCount counts what an external step would consult, so an estimate and the gate that +// guards it cannot disagree about which agents exist. +func NewImageAgentCount(ag *agents.Agents) ImageAgentCount { + if ag == nil { + return ImageAgentCount{} + } + return ImageAgentCount{Artist: len(ag.ArtistImageAgents()), Album: len(ag.AlbumImageAgents())} +} + // ExternalLookupsPerItem reports what resolving one item of this kind can cost: every image agent is // tried, and a zero count still bills one, so agents the caller cannot see never read as free. func ExternalLookupsPerItem(kind model.Kind, agents ImageAgentCount) int64 { diff --git a/core/artwork/worker.go b/core/artwork/worker.go index 0f947f6d7..0358708c0 100644 --- a/core/artwork/worker.go +++ b/core/artwork/worker.go @@ -40,6 +40,7 @@ type drainPool struct { // independently, and pruneMu serializes prune against the store-write window. type Worker struct { proc *processor + agents *agents.Agents cache cache.FileCache ffmpeg ffmpeg.FFmpeg broker events.Broker @@ -54,6 +55,7 @@ type Worker struct { func NewWorker(ds model.DataStore, store *ImageStore, ag *agents.Agents, ffmpeg ffmpeg.FFmpeg, broker events.Broker, imgCache cache.FileCache) *Worker { w := &Worker{ proc: &processor{ds: ds, store: store}, + agents: ag, cache: imgCache, ffmpeg: ffmpeg, broker: broker, @@ -132,9 +134,10 @@ func (w *Worker) RunPrune(ctx context.Context) error { } // Backfill enqueues every entity for re-resolution when the artwork config fingerprint changed, -// artists first. It reports whether anything was enqueued. +// artists first. It reports whether the backfill ran. func (w *Worker) Backfill(ctx context.Context) (bool, error) { - return backfill(ctx, w.proc.ds) + s, err := backfill(ctx, w.proc.ds, func() ImageAgentCount { return NewImageAgentCount(w.agents) }) + return s.Ran, err } // EnqueueStaleAbsentAll requeues known-absent entries older than StaleAbsentAge, at most