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