diff --git a/cmd/artwork.go b/cmd/artwork.go index 2fb696ca4..72291460c 100644 --- a/cmd/artwork.go +++ b/cmd/artwork.go @@ -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)) diff --git a/cmd/artwork_test.go b/cmd/artwork_test.go index b24fd5d06..4bee2f203 100644 --- a/cmd/artwork_test.go +++ b/cmd/artwork_test.go @@ -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")) diff --git a/model/artwork.go b/model/artwork.go index d4413ec9c..c6bfadf1d 100644 --- a/model/artwork.go +++ b/model/artwork.go @@ -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) diff --git a/persistence/artwork_queue_repository.go b/persistence/artwork_queue_repository.go index 3ad368772..2889bddaa 100644 --- a/persistence/artwork_queue_repository.go +++ b/persistence/artwork_queue_repository.go @@ -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) diff --git a/persistence/artwork_queue_repository_test.go b/persistence/artwork_queue_repository_test.go index a4a1ea76f..2b936eca2 100644 --- a/persistence/artwork_queue_repository_test.go +++ b/persistence/artwork_queue_repository_test.go @@ -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()) }) diff --git a/tests/mock_artwork_queue_repo.go b/tests/mock_artwork_queue_repo.go index a1b7b3e3d..f0e09d4cf 100644 --- a/tests/mock_artwork_queue_repo.go +++ b/tests/mock_artwork_queue_repo.go @@ -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()