mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-18 07:30:29 +00:00
feat(artwork): report which absent states gave up
An absent state is now terminal, so the operator needs to know how each one was reached. "Every source answered, there is no image" and "the retry budget ran out" both land in the same row, and only the second is worth a reprocess. artwork status splits the absent total by whether last_failure is set, which recordGiveUp writes only on exhaustion and a later success clears. Counted on absent rows alone. An item that kept its previous art through a failure is not missing artwork, and folding it in would have doubled the artist figure on a test library with a flaky provider.
This commit is contained in:
parent
8dd778239b
commit
65b24f63a2
@ -149,6 +149,8 @@ type sourceCount struct {
|
||||
type absentCount struct {
|
||||
kind model.Kind
|
||||
count int64
|
||||
// afterFailure is the subset that gave up rather than being told "no image".
|
||||
afterFailure int64
|
||||
}
|
||||
|
||||
type statusReport struct {
|
||||
@ -192,7 +194,11 @@ func collectStatus(ctx context.Context, ds model.DataStore) (statusReport, error
|
||||
rep.sources = append(rep.sources, sourceCount{kind: k, source: s, count: n})
|
||||
// An absent state is exactly a row with no source, so it needs no second query.
|
||||
if s == "" {
|
||||
rep.absent = append(rep.absent, absentCount{kind: k, count: n})
|
||||
failed, err := q.CountAbsentAfterFailure(k)
|
||||
if err != nil {
|
||||
return rep, fmt.Errorf("counting absent %s artwork that gave up: %w", k, err)
|
||||
}
|
||||
rep.absent = append(rep.absent, absentCount{kind: k, count: n, afterFailure: failed})
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -222,11 +228,12 @@ func formatStatus(rep statusReport) string {
|
||||
}
|
||||
|
||||
fmt.Fprintln(w, "\nAbsent (resolved, no image found)")
|
||||
fmt.Fprintln(w, " KIND\tABSENT")
|
||||
fmt.Fprintln(w, " KIND\tABSENT\tGAVE UP")
|
||||
for _, a := range rep.absent {
|
||||
fmt.Fprintf(w, " %s\t%d\n", a.kind, a.count)
|
||||
fmt.Fprintf(w, " %s\t%d\t%d\n", a.kind, a.count, a.afterFailure)
|
||||
}
|
||||
fmt.Fprintln(w, " (never retried on their own; run 'artwork reprocess --source absent')")
|
||||
fmt.Fprintln(w, " (gave up: the retry budget ran out rather than a source answering; most likely to resolve)")
|
||||
|
||||
fmt.Fprintln(w, "\nConfig")
|
||||
fmt.Fprintf(w, " State:\t%s\n", configState(rep))
|
||||
|
||||
@ -862,7 +862,7 @@ var _ = Describe("formatStatus", func() {
|
||||
{kind: model.KindArtistArtwork, source: "", count: 2},
|
||||
},
|
||||
absent: []absentCount{
|
||||
{kind: model.KindArtistArtwork, count: 2},
|
||||
{kind: model.KindArtistArtwork, count: 2, afterFailure: 1},
|
||||
},
|
||||
inputs: []artwork.FingerprintInput{{Name: "Agents", Value: "deezer,lastfm"}},
|
||||
stored: "abc123",
|
||||
@ -895,6 +895,12 @@ var _ = Describe("formatStatus", func() {
|
||||
Expect(sources).To(MatchRegexp(`artist\s+absent\s+2`))
|
||||
})
|
||||
|
||||
It("splits the absent total by how it was reached", func() {
|
||||
out := block(formatStatus(rep), "Absent (resolved, no image found)")
|
||||
Expect(out).To(MatchRegexp(`artist\s+2\s+1`), "2 absent, of which 1 gave up")
|
||||
Expect(formatStatus(rep)).To(ContainSubstring("the retry budget ran out"))
|
||||
})
|
||||
|
||||
It("says absent states are never retried on their own, and names the command that does", func() {
|
||||
Expect(formatStatus(rep)).To(ContainSubstring("never retried on their own"))
|
||||
Expect(formatStatus(rep)).To(ContainSubstring("artwork reprocess --source absent"))
|
||||
|
||||
@ -146,6 +146,9 @@ type ArtworkQueueRepository interface {
|
||||
CountBySource(kind Kind, sources []string) (int64, error)
|
||||
// SourcesInUse lists the distinct sources items of a kind currently resolve from, "" included.
|
||||
SourcesInUse(kind Kind) ([]string, error)
|
||||
// CountAbsentAfterFailure reports absent states a kind recorded because the retry budget ran
|
||||
// out, not because every source answered. Those are the ones a reprocess is likely to resolve.
|
||||
CountAbsentAfterFailure(kind Kind) (int64, error)
|
||||
// EnqueueBySource inserts queue rows for items of a kind whose current source matches.
|
||||
// It does not clear existing artwork state: the current image stays until it is replaced.
|
||||
EnqueueBySource(kind Kind, sources []string, priority int) (int64, error)
|
||||
|
||||
@ -223,4 +223,11 @@ func (r *artworkQueueRepository) Count() (int64, error) {
|
||||
return res.Count, err
|
||||
}
|
||||
|
||||
func (r *artworkQueueRepository) CountAbsentAfterFailure(kind model.Kind) (int64, error) {
|
||||
var res struct{ Count int64 }
|
||||
err := r.queryOne(Select("count(*) as count").From(itemArtworkTable).
|
||||
Where(And{Eq{"item_kind": kind.Prefix(), "hash": ""}, NotEq{"last_failure": ""}}), &res)
|
||||
return res.Count, err
|
||||
}
|
||||
|
||||
var _ model.ArtworkQueueRepository = (*artworkQueueRepository)(nil)
|
||||
|
||||
@ -400,6 +400,29 @@ var _ = Describe("ArtworkQueueRepository", func() {
|
||||
))
|
||||
})
|
||||
|
||||
It("counts only the absent states that gave up, not those a source answered", func() {
|
||||
awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder())
|
||||
for _, ia := range []model.ItemArtwork{
|
||||
{ItemKind: "ar", ItemID: "gaveup", ImageType: model.ImageTypePrimary, LastFailure: "[]"},
|
||||
{ItemKind: "ar", ItemID: "toldno", ImageType: model.ImageTypePrimary},
|
||||
{ItemKind: "ar", ItemID: "hasart", ImageType: model.ImageTypePrimary, Hash: "hX", LastFailure: "[]"},
|
||||
} {
|
||||
Expect(awRepo.PutItemArtwork(&ia)).To(Succeed())
|
||||
}
|
||||
|
||||
Expect(repo.CountAbsentAfterFailure(model.KindArtistArtwork)).To(Equal(int64(1)),
|
||||
"an item still serving art is not absent, however its last attempt went")
|
||||
|
||||
// A later success rewrites the row, clearing the record.
|
||||
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "gaveup",
|
||||
ImageType: model.ImageTypePrimary, Hash: "hZ"})).To(Succeed())
|
||||
Expect(repo.CountAbsentAfterFailure(model.KindArtistArtwork)).To(Equal(int64(0)))
|
||||
})
|
||||
|
||||
It("reports a kind that never gave up as zero", func() {
|
||||
Expect(repo.CountAbsentAfterFailure(model.KindRadioArtwork)).To(Equal(int64(0)))
|
||||
})
|
||||
|
||||
It("reports an empty queue as no rows", func() {
|
||||
Expect(repo.CountQueued(nil, nil)).To(BeEmpty())
|
||||
})
|
||||
|
||||
@ -275,6 +275,21 @@ func (m *MockArtworkQueueRepo) CountBySource(kind model.Kind, sources []string)
|
||||
return int64(len(m.matchingSource(kind, sources))), nil
|
||||
}
|
||||
|
||||
func (m *MockArtworkQueueRepo) CountAbsentAfterFailure(kind model.Kind) (int64, error) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
if m.Err != nil || m.ItemArtworkSource == nil {
|
||||
return 0, m.Err
|
||||
}
|
||||
var n int64
|
||||
for _, ia := range m.ItemArtworkSource.ItemData {
|
||||
if ia.ItemKind == kind.Prefix() && ia.Hash == "" && ia.LastFailure != "" {
|
||||
n++
|
||||
}
|
||||
}
|
||||
return n, nil
|
||||
}
|
||||
|
||||
func (m *MockArtworkQueueRepo) SourcesInUse(kind model.Kind) ([]string, error) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user