From 07b6411c0bc3ff0c2aafe8c8cfac4d00f2a6ddcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 21 Aug 2026 15:23:07 -0400 Subject: [PATCH] perf(artwork): cap the stale-absent recheck at 100 items per kind per hour (#6007) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(artwork): drip the stale-absent recheck instead of bursting it daily Each hourly housekeeping tick now re-queues at most 100 absent states per kind, oldest attempts first, instead of everything older than 24h at once. External agents see a flat ~100 requests/hour per agent instead of hourly bursts of ~2,000, and the effective recheck interval self-scales with the size of the absent pool (~4 days at 10k absent artists) while small libraries keep the 24h floor. * feat(artwork): trust an absent artwork state for a week before rechecking With the recheck now dripped at 100 items per kind per hour, the 24h floor only governed small libraries, where the drip cap never binds; they still re-asked every agent daily. A 7-day floor cuts that cost 7x and, for large libraries, becomes the binding limit over the drip cycle (~5.7k calls/day instead of ~9.6k at 10k absent artists). Among comparable servers, this is still the second-most-eager recheck: gonic retries misses every 30 days, Jellyfin and Funkwhale never do. * refactor(artwork): state the drip's backpressure contract where it bites Review follow-ups: the recheck limit deliberately caps the *selection*, not the insertions — already-queued rows use up budget, so a stalled drain admits no new work instead of building a recovery burst. Say so in the interface doc, mirror it in the mock by truncating the sorted candidates (matching the SQL's LIMIT-before-ON CONFLICT), and teach `artwork status` and the worker doc the post-drip wording. Also pin the one cmd fixture that still assumed a 24h recheck window. --- cmd/artwork.go | 3 ++- cmd/artwork_test.go | 7 ++++--- core/artwork/housekeeping.go | 8 ++++++-- core/artwork/housekeeping_test.go | 19 +++++++++++++++++-- core/artwork/worker.go | 3 ++- model/artwork.go | 9 +++++---- persistence/artwork_queue_repository.go | 9 +++++---- persistence/artwork_queue_repository_test.go | 19 ++++++++++++++++++- tests/mock_artwork_queue_repo.go | 16 +++++++++++----- 9 files changed, 70 insertions(+), 23 deletions(-) diff --git a/cmd/artwork.go b/cmd/artwork.go index b193abeaf..01de071bb 100644 --- a/cmd/artwork.go +++ b/cmd/artwork.go @@ -238,7 +238,8 @@ func formatStatus(rep statusReport) string { for _, a := range rep.absent { fmt.Fprintf(w, " %s\t%d\t%d\n", a.kind, a.Total, a.Stale) } - fmt.Fprintf(w, " (rechecked once the last attempt is older than %gh)\n", artwork.StaleAbsentAge.Hours()) + fmt.Fprintf(w, " (eligible once the last attempt is older than %gh; re-queued %d per kind per hour, oldest first)\n", + artwork.StaleAbsentAge.Hours(), artwork.StaleAbsentRecheckBatch) fmt.Fprintln(w, "\nBackfill") fmt.Fprintf(w, " State:\t%s\n", backfillState(rep)) diff --git a/cmd/artwork_test.go b/cmd/artwork_test.go index 1d2bdae73..cc63ed86f 100644 --- a/cmd/artwork_test.go +++ b/cmd/artwork_test.go @@ -818,7 +818,7 @@ var _ = Describe("collectStatus", func() { ImageType: model.ImageTypePrimary, Source: source, Hash: hash, AttemptedAt: attempted})).To(Succeed()) } put(model.KindArtistArtwork, "ar-1", "external:deezer", "h1", time.Now()) - put(model.KindArtistArtwork, "ar-2", "", "", time.Now().Add(-48*time.Hour)) + put(model.KindArtistArtwork, "ar-2", "", "", time.Now().Add(-artwork.StaleAbsentAge-time.Hour)) put(model.KindArtistArtwork, "ar-3", "", "", time.Now()) put(model.KindAlbumArtwork, "al-1", "folder", "h2", time.Now()) Expect(queue.Enqueue(model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar-9", @@ -909,8 +909,9 @@ var _ = Describe("formatStatus", func() { 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("states the recheck window and the drip rate the absent counts are bucketed against", func() { + Expect(formatStatus(rep)).To(ContainSubstring("168h")) + Expect(formatStatus(rep)).To(ContainSubstring("100 per kind per hour")) }) It("leads with the queued backlog, which is the finding, not with the fingerprint verdict", func() { diff --git a/core/artwork/housekeeping.go b/core/artwork/housekeeping.go index f2996d044..fed5757c8 100644 --- a/core/artwork/housekeeping.go +++ b/core/artwork/housekeeping.go @@ -18,7 +18,11 @@ import ( ) // StaleAbsentAge is how long an absent state is trusted before a recheck retries it. -const StaleAbsentAge = 24 * time.Hour +const StaleAbsentAge = 7 * 24 * time.Hour + +// StaleAbsentRecheckBatch caps how many absent states each hourly tick re-queues per kind, +// oldest first, so external agents see a flat drip instead of a daily burst. +const StaleAbsentRecheckBatch = 100 // RecheckKinds omits media files: they resolve embedded-only, at scan or on view. var RecheckKinds = []model.Kind{ @@ -125,7 +129,7 @@ func enqueueStaleAbsentAll(ctx context.Context, ds model.DataStore) error { cutoff := time.Now().Add(-StaleAbsentAge) queue := ds.ArtworkQueue(ctx) for _, kind := range RecheckKinds { - if _, err := queue.EnqueueStaleAbsent(kind, cutoff); err != nil { + if _, err := queue.EnqueueStaleAbsent(kind, cutoff, StaleAbsentRecheckBatch); err != nil { return err } } diff --git a/core/artwork/housekeeping_test.go b/core/artwork/housekeeping_test.go index 4ea15ab04..4f229bff8 100644 --- a/core/artwork/housekeeping_test.go +++ b/core/artwork/housekeeping_test.go @@ -2,6 +2,7 @@ package artwork import ( "context" + "fmt" "slices" "time" @@ -235,8 +236,8 @@ var _ = Describe("Housekeeping", func() { }) It("enqueues only absent entries older than the recheck window, across all kinds", func() { - old := time.Now().Add(-48 * time.Hour) - recent := time.Now().Add(-time.Hour) + old := time.Now().Add(-StaleAbsentAge - time.Hour) + recent := time.Now().Add(-StaleAbsentAge + time.Hour) artRepo.ItemData["ar-stale"] = model.ItemArtwork{ItemKind: "ar", ItemID: "ar1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old} artRepo.ItemData["al-stale"] = model.ItemArtwork{ItemKind: "al", ItemID: "al1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old} @@ -259,6 +260,20 @@ var _ = Describe("Housekeeping", func() { Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar2")).To(BeNil()) Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al2")).To(BeNil()) }) + + It("caps each tick at the recheck batch, oldest attempts first", func() { + for i := range StaleAbsentRecheckBatch + 1 { + id := fmt.Sprintf("ar%d", i) + artRepo.ItemData[id] = model.ItemArtwork{ItemKind: "ar", ItemID: id, ImageType: model.ImageTypePrimary, + Hash: "", AttemptedAt: time.Now().Add(-StaleAbsentAge - time.Duration(i+1)*time.Minute)} + } + + Expect(enqueueStaleAbsentAll(ctx, ds)).To(Succeed()) + + Expect(queueRepo.Data).To(HaveLen(StaleAbsentRecheckBatch)) + // ar0 has the newest attempted_at of the cohort, so it is the one left out. + Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar0")).To(BeNil()) + }) }) Describe("EnqueueMissingAll", func() { diff --git a/core/artwork/worker.go b/core/artwork/worker.go index 6271e54bf..0f947f6d7 100644 --- a/core/artwork/worker.go +++ b/core/artwork/worker.go @@ -137,7 +137,8 @@ func (w *Worker) Backfill(ctx context.Context) (bool, error) { return backfill(ctx, w.proc.ds) } -// EnqueueStaleAbsentAll requeues known-absent entries older than StaleAbsentAge. +// EnqueueStaleAbsentAll requeues known-absent entries older than StaleAbsentAge, at most +// StaleAbsentRecheckBatch per kind, oldest first. func (w *Worker) EnqueueStaleAbsentAll(ctx context.Context) error { return enqueueStaleAbsentAll(ctx, w.proc.ds) } diff --git a/model/artwork.go b/model/artwork.go index b484b0500..3c0df209b 100644 --- a/model/artwork.go +++ b/model/artwork.go @@ -134,8 +134,9 @@ type ArtworkQueueRepository interface { // EnqueuePreservingBackoff upserts like Enqueue but preserves an existing row's retry_at, so a // request-triggered read-through never resets a failed resolution's backoff. EnqueuePreservingBackoff(items ...ArtworkQueueItem) error - // EnqueueStaleAbsent inserts queue rows (priority Recheck) for absent states older than cutoff. - EnqueueStaleAbsent(kind Kind, attemptedBefore time.Time) (int64, error) + // EnqueueStaleAbsent inserts queue rows (priority Recheck) for absent states older than cutoff, oldest + // first; limit caps the selection, so already-queued rows use up budget (backpressure when the drain stalls). + EnqueueStaleAbsent(kind Kind, attemptedBefore time.Time, limit int) (int64, error) // EnqueueAllMissing inserts queue rows for all entities with no item_artwork row, at the given priority. EnqueueAllMissing(kind Kind, priority int) (int64, error) // EnqueueIfMissing inserts only for items with no item_artwork row yet. @@ -160,8 +161,8 @@ type ArtworkQueueRepository interface { // CountQueued reports the pending rows matching the kinds and priorities, grouped by both; // an empty filter means every one. CountQueued(kinds []Kind, priorities []int) ([]ArtworkQueueStat, error) - // CountAbsent reports the absent states of a kind, and how many of those EnqueueStaleAbsent - // would pick up at the given cutoff. + // CountAbsent reports the absent states of a kind, and how many are past the given cutoff, + // eligible for EnqueueStaleAbsent (which drains them limit rows per call). CountAbsent(kind Kind, attemptedBefore time.Time) (ArtworkAbsentStat, error) // PurgeDangling removes queue rows whose entity no longer exists. PurgeDangling() (int64, error) diff --git a/persistence/artwork_queue_repository.go b/persistence/artwork_queue_repository.go index 5db122a8b..1c0077fc7 100644 --- a/persistence/artwork_queue_repository.go +++ b/persistence/artwork_queue_repository.go @@ -56,11 +56,12 @@ func (r *artworkQueueRepository) EnqueuePreservingBackoff(items ...model.Artwork priority = MAX(priority, excluded.priority)`, items) } -func (r *artworkQueueRepository) EnqueueStaleAbsent(kind model.Kind, attemptedBefore time.Time) (int64, error) { +func (r *artworkQueueRepository) EnqueueStaleAbsent(kind model.Kind, attemptedBefore time.Time, limit int) (int64, error) { now := time.Now() return r.insertIfNotQueued("", `SELECT item_kind, item_id, image_type, ?, 0, ?, ? - FROM `+itemArtworkTable+` WHERE item_kind = ? AND hash = '' AND attempted_at < ?`, - model.ArtworkPriorityRecheck, now, now, kind.Prefix(), attemptedBefore) + FROM `+itemArtworkTable+` WHERE item_kind = ? AND hash = '' AND attempted_at < ? + ORDER BY attempted_at LIMIT ?`, + model.ArtworkPriorityRecheck, now, now, kind.Prefix(), attemptedBefore, limit) } func (r *artworkQueueRepository) EnqueueAllMissing(kind model.Kind, priority int) (int64, error) { @@ -230,7 +231,7 @@ func (r *artworkQueueRepository) Count() (int64, error) { return res.Count, err } -// CountAbsent matches EnqueueStaleAbsent on hash, so the stale count is what a recheck would queue. +// CountAbsent matches EnqueueStaleAbsent on hash, so the stale count is the pool a recheck drains from. func (r *artworkQueueRepository) CountAbsent(kind model.Kind, attemptedBefore time.Time) (model.ArtworkAbsentStat, error) { var res model.ArtworkAbsentStat err := r.queryOne(Select("count(*) as total"). diff --git a/persistence/artwork_queue_repository_test.go b/persistence/artwork_queue_repository_test.go index 35eebdd6d..1673a5b0e 100644 --- a/persistence/artwork_queue_repository_test.go +++ b/persistence/artwork_queue_repository_test.go @@ -244,7 +244,7 @@ var _ = Describe("ArtworkQueueRepository", func() { Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "fresh1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: time.Now()})).To(Succeed()) Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "found1", ImageType: model.ImageTypePrimary, Hash: "hX", AttemptedAt: old})).To(Succeed()) - n, err := repo.EnqueueStaleAbsent(model.KindArtistArtwork, time.Now().Add(-24*time.Hour)) + n, err := repo.EnqueueStaleAbsent(model.KindArtistArtwork, time.Now().Add(-24*time.Hour), 100) Expect(err).ToNot(HaveOccurred()) Expect(n).To(Equal(int64(1))) @@ -255,6 +255,23 @@ var _ = Describe("ArtworkQueueRepository", func() { Expect(items[0].Priority).To(Equal(model.ArtworkPriorityRecheck)) }) + It("enqueues only the oldest stale absent states up to the limit", func() { + awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder()) + now := time.Now() + Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "oldest", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: now.Add(-72 * time.Hour)})).To(Succeed()) + Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "older", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: now.Add(-60 * time.Hour)})).To(Succeed()) + Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "old", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: now.Add(-48 * time.Hour)})).To(Succeed()) + + n, err := repo.EnqueueStaleAbsent(model.KindArtistArtwork, now.Add(-24*time.Hour), 2) + Expect(err).ToNot(HaveOccurred()) + Expect(n).To(Equal(int64(2))) + + items, err := repo.DequeueBatch(10) + Expect(err).ToNot(HaveOccurred()) + ids := slice.Map(items, func(it model.ArtworkQueueItem) string { return it.ItemID }) + Expect(ids).To(ConsistOf("oldest", "older")) + }) + It("enqueues entities that have no item_artwork row at all", func() { awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder()) Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "al", ItemID: albumSgtPeppers.ID, ImageType: model.ImageTypePrimary, Hash: "hX", AttemptedAt: time.Now()})).To(Succeed()) diff --git a/tests/mock_artwork_queue_repo.go b/tests/mock_artwork_queue_repo.go index 70f7084ff..51ddf4b61 100644 --- a/tests/mock_artwork_queue_repo.go +++ b/tests/mock_artwork_queue_repo.go @@ -272,18 +272,24 @@ func (m *MockArtworkQueueRepo) EnqueuePreservingBackoff(items ...model.ArtworkQu return nil } -func (m *MockArtworkQueueRepo) EnqueueStaleAbsent(kind model.Kind, attemptedBefore time.Time) (int64, error) { +func (m *MockArtworkQueueRepo) EnqueueStaleAbsent(kind model.Kind, attemptedBefore time.Time, limit int) (int64, error) { m.mu.Lock() defer m.mu.Unlock() if m.Err != nil || m.ItemArtworkSource == nil { return 0, m.Err } + var stale []model.ItemArtwork + for _, ia := range m.ItemArtworkSource.ItemData { + if ia.ItemKind == kind.Prefix() && ia.Hash == "" && ia.AttemptedAt.Before(attemptedBefore) { + stale = append(stale, ia) + } + } + slices.SortFunc(stale, func(a, b model.ItemArtwork) int { return a.AttemptedAt.Compare(b.AttemptedAt) }) + // The limit caps the selection, like the SQL's LIMIT before ON CONFLICT: queued rows use up budget. + stale = stale[:min(limit, len(stale))] now := time.Now() var inserted int64 - for _, ia := range m.ItemArtworkSource.ItemData { - if ia.ItemKind != kind.Prefix() || ia.Hash != "" || !ia.AttemptedAt.Before(attemptedBefore) { - continue - } + for _, ia := range stale { k := iaKey(ia.ItemKind, ia.ItemID, ia.ImageType) if _, ok := m.Data[k]; ok { // DO NOTHING: never touch existing queue rows continue