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.
This commit is contained in:
Deluan Quintão 2026-08-21 20:27:42 -04:00 committed by GitHub
parent 07b6411c0b
commit 295886cb9a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 73 additions and 24 deletions

View File

@ -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 {

View File

@ -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 {

View File

@ -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() {

View File

@ -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 {

View File

@ -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