mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
tune(artwork): 5s backoff base + 12h give-up, drop the cap
Retry backoff now starts at 5s (was 15s) so a transient failure recovers on essentially the next drain, and jitter widens to ±40% so a wave of correlated failures doesn't re-clump into one poll. Add a 12h give-up budget measured from enqueued_at: once the next backoff would land past it, the worker stops retrying instead of grinding at a cap forever. A bare failure settles absent (handed to the 24h stale-absent sweep, and still recoverable on a page view); a found-stale keeps its already-served art. The budget bounds the tail, so the separate 48h backoffCap is removed.
This commit is contained in:
parent
8c7a6f25f8
commit
c3eea27b4f
@ -22,10 +22,12 @@ import (
|
||||
|
||||
const (
|
||||
workerPollInterval = 5 * time.Second
|
||||
backoffBase = 15 * time.Second
|
||||
backoffCap = 48 * time.Hour
|
||||
breakerThreshold = 5
|
||||
breakerProbeAfter = time.Minute
|
||||
backoffBase = 5 * time.Second
|
||||
// giveUpAfter bounds the retry budget from enqueue: past it the worker stops retrying and
|
||||
// hands the item to the periodic stale-absent recheck (settling absent on a bare failure).
|
||||
giveUpAfter = 12 * time.Hour
|
||||
breakerThreshold = 5
|
||||
breakerProbeAfter = time.Minute
|
||||
)
|
||||
|
||||
var errBreakerOpen = errors.New("artwork: external circuit breaker open")
|
||||
@ -215,11 +217,22 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) outco
|
||||
log.Warn(ctx, "artwork: could not delete processed queue item", "kind", item.ItemKind, "id", item.ItemID, err)
|
||||
}
|
||||
case outcomeFoundStale, outcomeFailed:
|
||||
// MarkFailedIfUnchanged, not MarkFailed: a scan that re-enqueued this row mid-flight reset
|
||||
// retry_at, so stale backoff must not stomp its fresh, immediate eligibility.
|
||||
retryAt := time.Now().Add(backoff(item.Attempts))
|
||||
if err := queue.MarkFailedIfUnchanged(item.ItemKind, item.ItemID, item.ImageType, item.RetryAt, retryAt); err != nil {
|
||||
log.Warn(ctx, "artwork: could not reschedule failed queue item", "kind", item.ItemKind, "id", item.ItemID, err)
|
||||
if retryAt.Before(item.EnqueuedAt.Add(giveUpAfter)) {
|
||||
// MarkFailedIfUnchanged, not MarkFailed: a scan that re-enqueued this row mid-flight reset
|
||||
// retry_at, so stale backoff must not stomp its fresh, immediate eligibility.
|
||||
if err := queue.MarkFailedIfUnchanged(item.ItemKind, item.ItemID, item.ImageType, item.RetryAt, retryAt); err != nil {
|
||||
log.Warn(ctx, "artwork: could not reschedule failed queue item", "kind", item.ItemKind, "id", item.ItemID, err)
|
||||
}
|
||||
break
|
||||
}
|
||||
// Retry budget exhausted: stop retrying. A bare failure settles absent so the stale-absent
|
||||
// sweep (and a page view) can still recover it; a stale-found keeps its already-served art.
|
||||
if out == outcomeFailed {
|
||||
writeAbsent(ctx, w.deps.ds.Artwork(ctx), item)
|
||||
}
|
||||
if err := queue.DeleteIfUnchanged(item.ItemKind, item.ItemID, item.ImageType, item.RetryAt); err != nil {
|
||||
log.Warn(ctx, "artwork: could not remove exhausted queue item", "kind", item.ItemKind, "id", item.ItemID, err)
|
||||
}
|
||||
}
|
||||
return out
|
||||
@ -314,14 +327,14 @@ func (w *Worker) gateFor(name string) *extGate {
|
||||
return g
|
||||
}
|
||||
|
||||
// backoffFor returns min(5m×4^n, 48h) scaled by (1+jitter), with jitter in [-0.2, 0.2].
|
||||
// backoffFor returns min(5s×4^n, giveUpAfter) scaled by (1+jitter), with jitter in [-0.4, 0.4].
|
||||
func backoffFor(attempts int, jitter float64) time.Duration {
|
||||
d := math.Min(float64(backoffBase)*math.Pow(4, float64(attempts)), float64(backoffCap))
|
||||
d := math.Min(float64(backoffBase)*math.Pow(4, float64(attempts)), float64(giveUpAfter))
|
||||
return time.Duration(d * (1 + jitter))
|
||||
}
|
||||
|
||||
func backoff(attempts int) time.Duration {
|
||||
return backoffFor(attempts, rand.Float64()*0.4-0.2) //nolint:gosec // retry jitter, not security-sensitive
|
||||
return backoffFor(attempts, rand.Float64()*0.8-0.4) //nolint:gosec // retry jitter, not security-sensitive
|
||||
}
|
||||
|
||||
// breaker opens after breakerThreshold consecutive external errors and admits a
|
||||
|
||||
@ -311,6 +311,31 @@ var _ = Describe("Worker", func() {
|
||||
Expect(it.RetryAt).To(BeTemporally("==", dequeued.Add(time.Minute)))
|
||||
})
|
||||
|
||||
It("gives up and settles absent once the retry budget is exhausted", func() {
|
||||
conf.Server.CoverArtPriority = "external"
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "al9", Name: "Album"}})
|
||||
imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")})
|
||||
w = NewWorker(ds, store, ag, ffm, broker, imgCache)
|
||||
Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ItemKind: "al", ItemID: "al9"})).To(Succeed())
|
||||
// Age the row past the budget so the next retry would land beyond enqueued_at+giveUpAfter.
|
||||
for k, v := range queueRepo.Data {
|
||||
if v.ItemID == "al9" {
|
||||
v.EnqueuedAt = time.Now().Add(-(giveUpAfter + time.Hour))
|
||||
queueRepo.Data[k] = v
|
||||
}
|
||||
}
|
||||
|
||||
n, err := w.drain(ctx, 1)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(n).To(Equal(1))
|
||||
|
||||
// Row removed (stops retrying) and the failure settles absent for the periodic sweep.
|
||||
Expect(findQueued(queueRepo, "al", "al9")).To(BeNil())
|
||||
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al9", model.ImageTypePrimary)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(ia.Hash).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("resolves a private playlist under an admin context instead of failing forever", func() {
|
||||
ds.MockedUser = adminUserRepo()
|
||||
vds := &visibilityPlaylistDS{
|
||||
@ -565,15 +590,15 @@ var _ = Describe("backoff", func() {
|
||||
attempts int
|
||||
want time.Duration
|
||||
}{
|
||||
{0, 15 * time.Second},
|
||||
{1, 60 * time.Second},
|
||||
{2, 4 * time.Minute},
|
||||
{3, 16 * time.Minute},
|
||||
{4, 64 * time.Minute},
|
||||
{5, 256 * time.Minute},
|
||||
{6, 1024 * time.Minute},
|
||||
{7, 48 * time.Hour},
|
||||
{8, 48 * time.Hour},
|
||||
{0, 5 * time.Second},
|
||||
{1, 20 * time.Second},
|
||||
{2, 80 * time.Second},
|
||||
{3, 320 * time.Second},
|
||||
{4, 1280 * time.Second},
|
||||
{5, 5120 * time.Second},
|
||||
{6, 20480 * time.Second},
|
||||
{7, 12 * time.Hour},
|
||||
{8, 12 * time.Hour},
|
||||
} {
|
||||
Expect(backoffFor(c.attempts, 0)).To(Equal(c.want), "attempt %d", c.attempts)
|
||||
}
|
||||
@ -585,9 +610,9 @@ var _ = Describe("backoff", func() {
|
||||
Expect(backoffFor(2, -0.2)).To(Equal(time.Duration(float64(base) * 0.8)))
|
||||
})
|
||||
|
||||
It("keeps random jitter within +/-20%", func() {
|
||||
lo := time.Duration(float64(16*time.Minute) * 0.8)
|
||||
hi := time.Duration(float64(16*time.Minute) * 1.2)
|
||||
It("keeps random jitter within +/-40%", func() {
|
||||
lo := time.Duration(float64(320*time.Second) * 0.6)
|
||||
hi := time.Duration(float64(320*time.Second) * 1.4)
|
||||
for range 200 {
|
||||
d := backoff(3)
|
||||
Expect(d).To(BeNumerically(">=", lo))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user