From 2ee2b2d66fb11bb5d0dd96744e3eea5a9ecc6506 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 25 Jul 2026 10:10:52 -0400 Subject: [PATCH] fix(artwork): restart the retry budget on re-enqueue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conflict clause updated only priority and retry_at, so a row that already existed kept its original attempts and enqueued_at. The worker measures the 12h give-up budget from enqueued_at, so any row that had been pending across a longer gap — a server left off, an upgrade, a laptop asleep — gave up on its very first attempt and settled absent. The manual re-resolve endpoint is the sharpest case: it clears artwork state and re-queues, but inherited the old row's spent window, so a deliberate retry got one shot. A fresh request now gets a fresh budget, with the mock updated to match. --- persistence/artwork_queue_repository.go | 5 ++++- persistence/artwork_queue_repository_test.go | 21 +++++++++++++++++++- tests/mock_artwork_queue_repo.go | 2 ++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/persistence/artwork_queue_repository.go b/persistence/artwork_queue_repository.go index 7fd7cd387..4430efd5c 100644 --- a/persistence/artwork_queue_repository.go +++ b/persistence/artwork_queue_repository.go @@ -26,9 +26,12 @@ func NewArtworkQueueRepository(ctx context.Context, db dbx.Builder) model.Artwor return r } +// Enqueue also restarts the retry budget the worker measures from enqueued_at, so a fresh +// request never inherits an old row's spent window and give up on its first attempt. func (r *artworkQueueRepository) Enqueue(items ...model.ArtworkQueueItem) error { return r.enqueue(`ON CONFLICT (item_kind, item_id, image_type) DO UPDATE SET - priority = MAX(priority, excluded.priority), retry_at = excluded.retry_at`, items) + priority = MAX(priority, excluded.priority), retry_at = excluded.retry_at, + attempts = 0, enqueued_at = excluded.enqueued_at`, items) } // EnqueueBump raises priority like Enqueue but leaves an existing row's retry_at intact, so a diff --git a/persistence/artwork_queue_repository_test.go b/persistence/artwork_queue_repository_test.go index e7c0967a1..402b4274a 100644 --- a/persistence/artwork_queue_repository_test.go +++ b/persistence/artwork_queue_repository_test.go @@ -7,6 +7,7 @@ import ( "github.com/navidrome/navidrome/model" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/pocketbase/dbx" ) var _ = Describe("ArtworkQueueRepository", func() { @@ -96,7 +97,7 @@ var _ = Describe("ArtworkQueueRepository", func() { Expect(repo.MarkFailedIfUnchanged("al", "m1", model.ImageTypePrimary, original, future)).To(Succeed()) got, _ = repo.DequeueBatch(10) Expect(got).To(HaveLen(1), "the fresh re-enqueue stays immediately eligible") - Expect(got[0].Attempts).To(Equal(1), "the stale failure must not bump attempts") + Expect(got[0].Attempts).To(BeZero(), "re-enqueue clears attempts, and the stale failure must not bump them") current := got[0].RetryAt // Failing with the current retry_at applies the backoff and bumps attempts. @@ -107,6 +108,24 @@ var _ = Describe("ArtworkQueueRepository", func() { Expect(all).To(Equal(int64(1))) }) + It("Enqueue restarts the retry budget an existing row had spent", func() { + Expect(repo.Enqueue(item("al", "e1", model.ArtworkPriorityScan))).To(Succeed()) + Expect(repo.MarkFailed("al", "e1", model.ImageTypePrimary, time.Now().Add(-time.Hour))).To(Succeed()) + stale := time.Now().Add(-48 * time.Hour) + _, err := GetDBXBuilder().NewQuery("UPDATE artwork_queue SET enqueued_at = {:t} WHERE item_id = 'e1'"). + Bind(dbx.Params{"t": stale}).Execute() + Expect(err).ToNot(HaveOccurred()) + + Expect(repo.Enqueue(item("al", "e1", model.ArtworkPriorityScan))).To(Succeed()) + + got, err := repo.DequeueBatch(10) + Expect(err).ToNot(HaveOccurred()) + Expect(got).To(HaveLen(1)) + Expect(got[0].EnqueuedAt).To(BeTemporally("~", time.Now(), time.Minute), + "a re-request must not inherit a spent 12h window and give up on its first attempt") + Expect(got[0].Attempts).To(BeZero()) + }) + It("deletes on completion and counts", func() { Expect(repo.Enqueue(item("al", "c1", 0))).To(Succeed()) n, _ := repo.Count() diff --git a/tests/mock_artwork_queue_repo.go b/tests/mock_artwork_queue_repo.go index 19bb3b6e8..896516e02 100644 --- a/tests/mock_artwork_queue_repo.go +++ b/tests/mock_artwork_queue_repo.go @@ -40,6 +40,8 @@ func (m *MockArtworkQueueRepo) Enqueue(items ...model.ArtworkQueueItem) error { if prev, ok := m.Data[k]; ok { prev.Priority = max(prev.Priority, it.Priority) prev.RetryAt = now + prev.Attempts = 0 + prev.EnqueuedAt = now m.Data[k] = prev continue }