mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-18 07:30:29 +00:00
feat(artwork): let a reprocess target only the absent states that failed
An absent state is reached two ways, and only one is worth retrying: a source answering "no image" will answer the same next time, while an exhausted retry budget will not. --source absent swept both together, so recovering a handful of failures meant re-asking the provider about every artist it has no photo for. failed joins the source vocabulary next to absent, as a pseudo-source matching absent rows that carry a last_failure. Several sources are a union, so asking for absent and failed together is still just absent. It resolves through the filter CountBySource and EnqueueBySource already share, which is what keeps the dry-run preview honest, and the status column now reads through the same call rather than its own query — so the number in the report is the set the command acts on, by construction rather than by agreement.
This commit is contained in:
parent
65b24f63a2
commit
be87c3b179
@ -44,7 +44,8 @@ func init() {
|
||||
artworkReprocessCmd.Flags().StringSliceVar(&artworkKinds, "kind", nil,
|
||||
"kinds to reprocess ("+kindPrefixes(artwork.ReprocessKinds)+"); repeatable")
|
||||
artworkReprocessCmd.Flags().StringSliceVar(&artworkSources, "source", nil,
|
||||
"only items currently resolved from these sources (e.g. folder, external:deezer, absent)")
|
||||
"only items currently resolved from these sources (e.g. folder, external:deezer, absent, "+
|
||||
"or failed for the absent ones that gave up)")
|
||||
artworkReprocessCmd.Flags().BoolVar(&artworkAll, "all", false, "reprocess every kind")
|
||||
artworkReprocessCmd.Flags().BoolVar(&artworkDryRun, "dry-run", false,
|
||||
"report what would be queued and exit without queueing")
|
||||
@ -194,9 +195,9 @@ 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 == "" {
|
||||
failed, err := q.CountAbsentAfterFailure(k)
|
||||
failed, err := q.CountBySource(k, []string{model.ArtworkSourceFailed})
|
||||
if err != nil {
|
||||
return rep, fmt.Errorf("counting absent %s artwork that gave up: %w", k, err)
|
||||
return rep, fmt.Errorf("counting failed %s artwork: %w", k, err)
|
||||
}
|
||||
rep.absent = append(rep.absent, absentCount{kind: k, count: n, afterFailure: failed})
|
||||
}
|
||||
@ -228,12 +229,13 @@ func formatStatus(rep statusReport) string {
|
||||
}
|
||||
|
||||
fmt.Fprintln(w, "\nAbsent (resolved, no image found)")
|
||||
fmt.Fprintln(w, " KIND\tABSENT\tGAVE UP")
|
||||
fmt.Fprintln(w, " KIND\tABSENT\tFAILED")
|
||||
for _, a := range rep.absent {
|
||||
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, " (failed: the retry budget ran out rather than a source answering; retry just those\n"+
|
||||
" with 'artwork reprocess --source failed')")
|
||||
|
||||
fmt.Fprintln(w, "\nConfig")
|
||||
fmt.Fprintf(w, " State:\t%s\n", configState(rep))
|
||||
@ -347,19 +349,31 @@ func selectedKinds(kinds, sources []string, all bool) ([]model.Kind, error) {
|
||||
})
|
||||
}
|
||||
|
||||
// absentSource is how the stored empty source — resolved, no image — is spelled on the CLI.
|
||||
const absentSource = "absent"
|
||||
// absentSource is how the stored empty source — resolved, no image — is spelled on the CLI, and
|
||||
// failedSource the subset of it that gave up rather than being answered.
|
||||
const (
|
||||
absentSource = "absent"
|
||||
failedSource = "failed"
|
||||
)
|
||||
|
||||
func repositorySources(sources []string) []string {
|
||||
return slice.Map(sources, func(s string) string {
|
||||
if s == absentSource {
|
||||
switch s {
|
||||
case absentSource:
|
||||
return ""
|
||||
case failedSource:
|
||||
return model.ArtworkSourceFailed
|
||||
}
|
||||
return s
|
||||
})
|
||||
}
|
||||
|
||||
func displaySource(s string) string { return cmp.Or(s, absentSource) }
|
||||
func displaySource(s string) string {
|
||||
if s == model.ArtworkSourceFailed {
|
||||
return failedSource
|
||||
}
|
||||
return cmp.Or(s, absentSource)
|
||||
}
|
||||
|
||||
type confirmFunc func(out io.Writer, total, external int64) bool
|
||||
|
||||
@ -442,7 +456,8 @@ func validateSources(q model.ArtworkQueueRepository, sources []string) error {
|
||||
}
|
||||
var unknown []string
|
||||
for _, s := range sources {
|
||||
if s != "" && !slices.Contains(inUse, s) { // the reserved absent source is valid even when nothing is absent
|
||||
// The reserved absent and failed sources are valid even when nothing currently matches them.
|
||||
if s != "" && s != model.ArtworkSourceFailed && !slices.Contains(inUse, s) {
|
||||
unknown = append(unknown, displaySource(s))
|
||||
}
|
||||
}
|
||||
|
||||
@ -511,6 +511,11 @@ var _ = Describe("repositorySources", func() {
|
||||
Expect(repositorySources([]string{"absent", "folder"})).To(Equal([]string{"", "folder"}))
|
||||
})
|
||||
|
||||
It("maps the failed name onto the pseudo-source, and back for display", func() {
|
||||
Expect(repositorySources([]string{failedSource})).To(Equal([]string{model.ArtworkSourceFailed}))
|
||||
Expect(displaySource(model.ArtworkSourceFailed)).To(Equal(failedSource))
|
||||
})
|
||||
|
||||
It("keeps an empty selection empty, meaning every source", func() {
|
||||
Expect(repositorySources(nil)).To(BeEmpty())
|
||||
})
|
||||
@ -898,7 +903,7 @@ var _ = Describe("formatStatus", func() {
|
||||
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"))
|
||||
Expect(formatStatus(rep)).To(ContainSubstring("artwork reprocess --source failed"))
|
||||
})
|
||||
|
||||
It("says absent states are never retried on their own, and names the command that does", func() {
|
||||
|
||||
@ -18,6 +18,10 @@ type Artwork struct {
|
||||
|
||||
const ImageTypePrimary = "primary"
|
||||
|
||||
// ArtworkSourceFailed is a pseudo-source selecting absent states that exhausted the retry budget
|
||||
// rather than being answered. The "!" keeps it from colliding with a stored source value.
|
||||
const ArtworkSourceFailed = "!failed"
|
||||
|
||||
// ItemImage is per-entity artwork state hydrated at query time; never persisted.
|
||||
type ItemImage struct {
|
||||
ImageHash string `structs:"-" json:"imageHash,omitempty"`
|
||||
@ -142,13 +146,11 @@ type ArtworkQueueRepository interface {
|
||||
// EnqueueIfMissing inserts only for items with no item_artwork row yet.
|
||||
EnqueueIfMissing(items ...ArtworkQueueItem) error
|
||||
// CountBySource reports how many items of a kind currently resolve from the given sources.
|
||||
// An empty sources slice means every source; "" matches absent state.
|
||||
// An empty sources slice means every source; "" matches absent state, and the pseudo-source
|
||||
// ArtworkSourceFailed matches the absent states that gave up.
|
||||
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)
|
||||
|
||||
@ -102,13 +102,23 @@ func (r *artworkQueueRepository) insertIfNotQueued(with, sql string, args ...any
|
||||
` (`+strings.Join(enqueueColumns, ", ")+`) `+sql+skipIfQueued, args...))
|
||||
}
|
||||
|
||||
// artworkSourceFilter selects item_artwork rows of a kind; no sources means every source, "" the absent state.
|
||||
// artworkSourceFilter selects item_artwork rows of a kind; no sources means every source, "" the
|
||||
// absent state, and ArtworkSourceFailed the absent states that gave up. Several are a union, so
|
||||
// asking for both absent and failed is just absent.
|
||||
func artworkSourceFilter(kind model.Kind, sources []string) Sqlizer {
|
||||
f := And{Eq{"item_kind": kind.Prefix()}}
|
||||
if len(sources) > 0 {
|
||||
f = append(f, Eq{"source": sources})
|
||||
if len(sources) == 0 {
|
||||
return f
|
||||
}
|
||||
return f
|
||||
stored := slices.DeleteFunc(slices.Clone(sources), func(s string) bool { return s == model.ArtworkSourceFailed })
|
||||
var match Or
|
||||
if len(stored) > 0 {
|
||||
match = append(match, Eq{"source": stored})
|
||||
}
|
||||
if len(stored) != len(sources) {
|
||||
match = append(match, And{Eq{"hash": ""}, NotEq{"last_failure": ""}})
|
||||
}
|
||||
return append(f, match)
|
||||
}
|
||||
|
||||
func (r *artworkQueueRepository) CountBySource(kind model.Kind, sources []string) (int64, error) {
|
||||
@ -223,11 +233,4 @@ 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,7 +400,7 @@ var _ = Describe("ArtworkQueueRepository", func() {
|
||||
))
|
||||
})
|
||||
|
||||
It("counts only the absent states that gave up, not those a source answered", func() {
|
||||
It("selects 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: "[]"},
|
||||
@ -410,17 +410,33 @@ var _ = Describe("ArtworkQueueRepository", func() {
|
||||
Expect(awRepo.PutItemArtwork(&ia)).To(Succeed())
|
||||
}
|
||||
|
||||
Expect(repo.CountAbsentAfterFailure(model.KindArtistArtwork)).To(Equal(int64(1)),
|
||||
Expect(repo.CountBySource(model.KindArtistArtwork, []string{model.ArtworkSourceFailed})).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)))
|
||||
Expect(repo.CountBySource(model.KindArtistArtwork, []string{model.ArtworkSourceFailed})).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("unions the failed pseudo-source with a real one, so absent plus failed is just absent", 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: "folder", ImageType: model.ImageTypePrimary, Hash: "hX", Source: "folder"},
|
||||
} {
|
||||
Expect(awRepo.PutItemArtwork(&ia)).To(Succeed())
|
||||
}
|
||||
failedAndAbsent := []string{model.ArtworkSourceFailed, ""}
|
||||
Expect(repo.CountBySource(model.KindArtistArtwork, failedAndAbsent)).To(Equal(int64(2)),
|
||||
"failed is a subset of absent, so asking for both is asking for absent")
|
||||
Expect(repo.CountBySource(model.KindArtistArtwork, []string{model.ArtworkSourceFailed, "folder"})).
|
||||
To(Equal(int64(2)), "a pseudo-source and a stored source combine as a union")
|
||||
})
|
||||
|
||||
It("reports a kind with nothing failed as zero", func() {
|
||||
Expect(repo.CountBySource(model.KindRadioArtwork, []string{model.ArtworkSourceFailed})).To(Equal(int64(0)))
|
||||
})
|
||||
|
||||
It("reports an empty queue as no rows", func() {
|
||||
|
||||
@ -252,14 +252,24 @@ func (m *MockArtworkQueueRepo) EnqueuePreservingBackoff(items ...model.ArtworkQu
|
||||
return nil
|
||||
}
|
||||
|
||||
// matchingSource mirrors the SQL filter: no sources means every source, "" the absent state.
|
||||
// matchingSource mirrors the SQL filter: no sources means every source, "" the absent state, and
|
||||
// ArtworkSourceFailed the absent states that gave up.
|
||||
func (m *MockArtworkQueueRepo) matchingSource(kind model.Kind, sources []string) []model.ItemArtwork {
|
||||
if m.ItemArtworkSource == nil {
|
||||
return nil
|
||||
}
|
||||
matches := func(ia model.ItemArtwork) bool {
|
||||
if len(sources) == 0 {
|
||||
return true
|
||||
}
|
||||
if slices.Contains(sources, ia.Source) {
|
||||
return true
|
||||
}
|
||||
return slices.Contains(sources, model.ArtworkSourceFailed) && ia.Hash == "" && ia.LastFailure != ""
|
||||
}
|
||||
var res []model.ItemArtwork
|
||||
for _, ia := range m.ItemArtworkSource.ItemData {
|
||||
if ia.ItemKind == kind.Prefix() && (len(sources) == 0 || slices.Contains(sources, ia.Source)) {
|
||||
if ia.ItemKind == kind.Prefix() && matches(ia) {
|
||||
res = append(res, ia)
|
||||
}
|
||||
}
|
||||
@ -275,21 +285,6 @@ 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