mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 02:17:25 +02:00
fix(cli): lead the artwork status backfill line with the queued backlog
By the time anyone runs a diagnostic, backfill has usually already stored the new fingerprint, so 'up to date' was printed while thousands of items churned through external providers. The backlog is the finding; the fingerprint is context. Also echoes the config inputs the fingerprint covers, so a change can be traced to the setting that caused it, and pins the rendered rows: the Absent values, the queue TOTAL and a queue-scoped kind/priority pair were all unasserted, so kindName and priorityName were effectively untested. FingerprintInputs is now the single listing ConfigFingerprint hashes; a pinned hash proves the value did not change.
This commit is contained in:
parent
0950f939fb
commit
3ca194c887
5 changed files with 135 additions and 25 deletions
|
|
@ -132,11 +132,11 @@ type statusReport struct {
|
|||
queue []model.ArtworkQueueStat
|
||||
sources []sourceCount
|
||||
absent []absentCount
|
||||
inputs []artwork.FingerprintInput
|
||||
stored string
|
||||
current string
|
||||
}
|
||||
|
||||
// backfillQueued counts what a backfill still has to drain, the cost a config change is charging now.
|
||||
func (r statusReport) backfillQueued() int64 {
|
||||
var n int64
|
||||
for _, s := range r.queue {
|
||||
|
|
@ -179,7 +179,7 @@ func collectStatus(ctx context.Context, ds model.DataStore) (statusReport, error
|
|||
rep.absent = append(rep.absent, absentCount{kind: k, ArtworkAbsentStat: stat})
|
||||
}
|
||||
|
||||
rep.current = artwork.ConfigFingerprint()
|
||||
rep.current, rep.inputs = artwork.ConfigFingerprint(), artwork.FingerprintInputs()
|
||||
if rep.stored, err = ds.Property(ctx).DefaultGet(consts.ArtConfFingerprintPropertyKey, ""); err != nil {
|
||||
return rep, fmt.Errorf("reading the stored artwork fingerprint: %w", err)
|
||||
}
|
||||
|
|
@ -215,25 +215,34 @@ func formatStatus(rep statusReport) string {
|
|||
fmt.Fprintf(w, " (rechecked once the last attempt is older than %gh)\n", artwork.StaleAbsentAge.Hours())
|
||||
|
||||
fmt.Fprintln(w, "\nBackfill")
|
||||
fmt.Fprintf(w, " State:\t%s\n", backfillState(rep))
|
||||
fmt.Fprintf(w, " Stored fingerprint:\t%s\n", cmp.Or(rep.stored, "(none)"))
|
||||
fmt.Fprintf(w, " Current fingerprint:\t%s\n", rep.current)
|
||||
fmt.Fprintf(w, " State:\t%s\n", backfillState(rep))
|
||||
if len(rep.inputs) > 0 {
|
||||
fmt.Fprintln(w, " Fingerprint inputs (changing any of these re-resolves the whole library):")
|
||||
for _, in := range rep.inputs {
|
||||
fmt.Fprintf(w, " %s:\t%s\n", in.Name, in.Value)
|
||||
}
|
||||
}
|
||||
|
||||
w.Flush()
|
||||
return sb.String()
|
||||
}
|
||||
|
||||
// backfillState turns "why is my server re-resolving everything?" into a line: a stored fingerprint
|
||||
// that differs is a pending re-resolve of the whole library, and backfill rows are one already running.
|
||||
// backfillState leads with the queued backlog: by the time anyone runs this, backfill has usually
|
||||
// already stored the new fingerprint, and "up to date" would bury the flood it is still working through.
|
||||
func backfillState(rep statusReport) string {
|
||||
state := "up to date"
|
||||
changed := "fingerprint up to date"
|
||||
if rep.stored != rep.current {
|
||||
state = "fingerprint changed — every artist, album, playlist and radio will be re-enqueued on the next startup"
|
||||
changed = "fingerprint changed"
|
||||
}
|
||||
if n := rep.backfillQueued(); n > 0 {
|
||||
state += fmt.Sprintf("; %d items still queued at backfill priority", n)
|
||||
return fmt.Sprintf("backfill running: %d items queued (%s)", n, changed)
|
||||
}
|
||||
return state
|
||||
if rep.stored != rep.current {
|
||||
return "fingerprint changed — every artist, album, playlist and radio will be re-enqueued on the next startup"
|
||||
}
|
||||
return "up to date"
|
||||
}
|
||||
|
||||
func kindName(prefix string) string {
|
||||
|
|
@ -569,7 +578,7 @@ func formatExplain(rep explainReport) string {
|
|||
if rep.queued == nil {
|
||||
fmt.Fprintln(w, " (not queued)")
|
||||
} else {
|
||||
fmt.Fprintf(w, " Priority:\t%d\n", rep.queued.Priority)
|
||||
fmt.Fprintf(w, " Priority:\t%s (%d)\n", priorityName(rep.queued.Priority), rep.queued.Priority)
|
||||
fmt.Fprintf(w, " Attempts:\t%d\n", rep.queued.Attempts)
|
||||
fmt.Fprintf(w, " Retry at:\t%s\n", formatTime(rep.queued.RetryAt))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -151,7 +151,7 @@ var _ = Describe("formatExplain", func() {
|
|||
Expect(out).To(ContainSubstring("abc123"))
|
||||
Expect(out).To(ContainSubstring("/music/cover.jpg"))
|
||||
Expect(out).To(ContainSubstring("2026-08-13T10:00:00Z"))
|
||||
Expect(out).To(ContainSubstring("50"))
|
||||
Expect(out).To(ContainSubstring("scan (50)"), "a bare 50 makes the operator look the priority up")
|
||||
Expect(out).To(ContainSubstring("resolved from folder"))
|
||||
})
|
||||
|
||||
|
|
@ -502,6 +502,12 @@ var _ = Describe("collectStatus", func() {
|
|||
Expect(rep.stored).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("collects the config inputs the fingerprint is computed from", func() {
|
||||
rep, err := collectStatus(ctx, ds)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(rep.inputs).To(Equal(artwork.FingerprintInputs()))
|
||||
})
|
||||
|
||||
It("queues nothing", func() {
|
||||
_, err := collectStatus(ctx, ds)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
|
@ -526,38 +532,74 @@ var _ = Describe("formatStatus", func() {
|
|||
absent: []absentCount{
|
||||
{kind: model.KindArtistArtwork, ArtworkAbsentStat: model.ArtworkAbsentStat{Total: 2, Stale: 1}},
|
||||
},
|
||||
inputs: []artwork.FingerprintInput{{Name: "Agents", Value: "deezer,lastfm"}},
|
||||
stored: "abc123",
|
||||
current: "abc123",
|
||||
}
|
||||
})
|
||||
|
||||
// block isolates one section, so an assertion cannot be satisfied by a coincidence elsewhere.
|
||||
block := func(out, header string) string {
|
||||
GinkgoHelper()
|
||||
_, after, found := strings.Cut(out, header+"\n")
|
||||
Expect(found).To(BeTrue(), "the %q block must be printed", header)
|
||||
body, _, _ := strings.Cut(after, "\n\n")
|
||||
return body
|
||||
}
|
||||
|
||||
It("prints every block", func() {
|
||||
out := formatStatus(rep)
|
||||
for _, block := range []string{"Queue", "Sources", "Absent", "Backfill"} {
|
||||
Expect(out).To(ContainSubstring(block))
|
||||
for _, header := range []string{"Queue", "Sources", "Absent", "Backfill"} {
|
||||
Expect(out).To(ContainSubstring(header))
|
||||
}
|
||||
Expect(out).To(ContainSubstring("backfill"))
|
||||
Expect(out).To(ContainSubstring("external:deezer"))
|
||||
Expect(out).To(ContainSubstring("abc123"))
|
||||
})
|
||||
|
||||
It("names the empty source as absent", func() {
|
||||
Expect(formatStatus(rep)).To(ContainSubstring("absent"))
|
||||
It("names the kind and the priority of every queued row", func() {
|
||||
queue := block(formatStatus(rep), "Queue")
|
||||
Expect(queue).To(MatchRegexp(`artist\s+backfill\s+2`))
|
||||
Expect(queue).To(MatchRegexp(`album\s+scan\s+1`))
|
||||
})
|
||||
|
||||
It("totals the queue", func() {
|
||||
Expect(block(formatStatus(rep), "Queue")).To(MatchRegexp(`TOTAL\s+3`))
|
||||
})
|
||||
|
||||
It("counts each source, naming the empty one absent", func() {
|
||||
sources := block(formatStatus(rep), "Sources")
|
||||
Expect(sources).To(MatchRegexp(`artist\s+external:deezer\s+5`))
|
||||
Expect(sources).To(MatchRegexp(`artist\s+absent\s+2`))
|
||||
})
|
||||
|
||||
It("prints the absent total and how many are due for recheck", func() {
|
||||
absent := block(formatStatus(rep), "Absent (resolved, no image found)")
|
||||
Expect(absent).To(MatchRegexp(`artist\s+2\s+1`))
|
||||
})
|
||||
|
||||
It("states the recheck window the absent counts are bucketed against", func() {
|
||||
Expect(formatStatus(rep)).To(ContainSubstring("24h"))
|
||||
})
|
||||
|
||||
It("reports a matching fingerprint as up to date, with the backfill still draining", func() {
|
||||
out := formatStatus(rep)
|
||||
Expect(out).To(ContainSubstring("up to date"))
|
||||
Expect(out).To(ContainSubstring("2 items still queued at backfill priority"),
|
||||
"a drained-down backfill is the signal that a config change flooded the queue")
|
||||
It("leads with the queued backlog, which is the finding, not with the fingerprint verdict", func() {
|
||||
out := block(formatStatus(rep), "Backfill")
|
||||
Expect(out).To(MatchRegexp(`State:\s+backfill running: 2 items queued`),
|
||||
"an operator scanning for trouble must not read 'up to date' while 2 items churn")
|
||||
Expect(out).To(ContainSubstring("fingerprint up to date"))
|
||||
})
|
||||
|
||||
It("reports up to date only once the backfill has drained", func() {
|
||||
rep.queue = []model.ArtworkQueueStat{{ItemKind: "al", Priority: model.ArtworkPriorityScan, Count: 1}}
|
||||
|
||||
Expect(block(formatStatus(rep), "Backfill")).To(MatchRegexp(`State:\s+up to date`))
|
||||
})
|
||||
|
||||
It("echoes the config inputs a fingerprint change would have come from", func() {
|
||||
Expect(block(formatStatus(rep), "Backfill")).To(MatchRegexp(`Agents:\s+deezer,lastfm`))
|
||||
})
|
||||
|
||||
It("reports a changed fingerprint as a pending re-resolve of everything", func() {
|
||||
rep.stored = "older"
|
||||
rep.queue, rep.queueTotal = nil, 0
|
||||
|
||||
out := formatStatus(rep)
|
||||
Expect(out).To(ContainSubstring("fingerprint changed"))
|
||||
|
|
|
|||
|
|
@ -6,6 +6,8 @@ import (
|
|||
"encoding/hex"
|
||||
"fmt"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
|
|
@ -33,12 +35,30 @@ func hasRecheckPath(prefix string) bool {
|
|||
// artworkEpoch invalidates all resolution state when bumped; bump it whenever resolution semantics change.
|
||||
const artworkEpoch = 1
|
||||
|
||||
// FingerprintInput is one config value the fingerprint covers, named after the setting it came from.
|
||||
type FingerprintInput struct {
|
||||
Name string
|
||||
Value string
|
||||
}
|
||||
|
||||
// FingerprintInputs is the single listing of what ConfigFingerprint hashes, so a caller can report
|
||||
// which setting a fingerprint change came from without keeping a second copy of the list.
|
||||
func FingerprintInputs() []FingerprintInput {
|
||||
return []FingerprintInput{
|
||||
{"CoverArtPriority", conf.Server.CoverArtPriority},
|
||||
{"ArtistArtPriority", conf.Server.ArtistArtPriority},
|
||||
{"ArtistImageFolder", conf.Server.ArtistImageFolder},
|
||||
{"Agents", conf.Server.Agents},
|
||||
{"EnableExternalServices", strconv.FormatBool(conf.Server.EnableExternalServices)},
|
||||
{"EnableM3UExternalAlbumArt", strconv.FormatBool(conf.Server.EnableM3UExternalAlbumArt)},
|
||||
}
|
||||
}
|
||||
|
||||
// ConfigFingerprint covers the inputs that affect resolution outcomes; a change invalidates stored state.
|
||||
// Exported so the CLI reports the value backfill compares instead of computing one that can drift.
|
||||
func ConfigFingerprint() string {
|
||||
raw := fmt.Sprintf("%s|%s|%s|%s|%t|%t|%d",
|
||||
conf.Server.CoverArtPriority, conf.Server.ArtistArtPriority, conf.Server.ArtistImageFolder,
|
||||
conf.Server.Agents, conf.Server.EnableExternalServices, conf.Server.EnableM3UExternalAlbumArt, artworkEpoch)
|
||||
values := slice.Map(FingerprintInputs(), func(i FingerprintInput) string { return i.Value })
|
||||
raw := fmt.Sprintf("%s|%d", strings.Join(values, "|"), artworkEpoch)
|
||||
sum := md5.Sum([]byte(raw)) //nolint:gosec // fingerprint, not security-sensitive
|
||||
return hex.EncodeToString(sum[:])
|
||||
}
|
||||
|
|
|
|||
|
|
@ -113,6 +113,28 @@ var _ = Describe("Housekeeping", func() {
|
|||
Expect(ConfigFingerprint()).NotTo(Equal(f1))
|
||||
})
|
||||
|
||||
// Pinned: a changed formula re-resolves every library on upgrade, flooding external providers.
|
||||
It("hashes a given config to a stable value", func() {
|
||||
conf.Server.CoverArtPriority = "cover.*, embedded"
|
||||
conf.Server.ArtistArtPriority = "artist.*, external"
|
||||
conf.Server.ArtistImageFolder = ""
|
||||
conf.Server.Agents = "lastfm,spotify"
|
||||
conf.Server.EnableExternalServices = true
|
||||
conf.Server.EnableM3UExternalAlbumArt = false
|
||||
|
||||
Expect(ConfigFingerprint()).To(Equal("7e537a22febc07d3d5ca40546e88da54"))
|
||||
})
|
||||
|
||||
It("reports the config inputs it hashes, so a change can be traced to a setting", func() {
|
||||
conf.Server.Agents = "lastfm,spotify"
|
||||
conf.Server.CoverArtPriority = "cover.*, embedded"
|
||||
|
||||
Expect(FingerprintInputs()).To(ContainElements(
|
||||
FingerprintInput{Name: "Agents", Value: "lastfm,spotify"},
|
||||
FingerprintInput{Name: "CoverArtPriority", Value: "cover.*, embedded"},
|
||||
))
|
||||
})
|
||||
|
||||
It("does not change when the server version changes", func() {
|
||||
original := consts.Version
|
||||
DeferCleanup(func() { consts.Version = original })
|
||||
|
|
|
|||
|
|
@ -415,6 +415,23 @@ var _ = Describe("ArtworkQueueRepository", func() {
|
|||
To(Equal(model.ArtworkAbsentStat{Total: 2, Stale: 1}))
|
||||
})
|
||||
|
||||
// status prints both under one "absent" heading, so a writer that stops clearing hash and
|
||||
// source together would make the two numbers disagree with no other warning.
|
||||
It("agrees with the source-based absent count", func() {
|
||||
awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder())
|
||||
for _, ia := range []model.ItemArtwork{
|
||||
{ItemKind: "ar", ItemID: "absent1", ImageType: model.ImageTypePrimary, Hash: "", Source: "", AttemptedAt: time.Now()},
|
||||
{ItemKind: "ar", ItemID: "absent2", ImageType: model.ImageTypePrimary, Hash: "", Source: "", AttemptedAt: time.Now()},
|
||||
{ItemKind: "ar", ItemID: "found1", ImageType: model.ImageTypePrimary, Hash: "hX", Source: "folder", AttemptedAt: time.Now()},
|
||||
} {
|
||||
Expect(awRepo.PutItemArtwork(&ia)).To(Succeed())
|
||||
}
|
||||
|
||||
stat, err := repo.CountAbsent(model.KindArtistArtwork, time.Now())
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(repo.CountBySource(model.KindArtistArtwork, []string{""})).To(Equal(stat.Total))
|
||||
})
|
||||
|
||||
It("reports a kind with no absent state as zero, not as an error", func() {
|
||||
Expect(repo.CountAbsent(model.KindRadioArtwork, time.Now())).To(Equal(model.ArtworkAbsentStat{}))
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue