feat(scrobbler): exponential backoff for scrobble retries during outages (#5818)

* feat(scrobbler): add exponential backoff delay helper

* feat(scrobbler): back off retries up to 4m during outages

* test(scrobbler): verify backoff schedule with synctest; clarify backoffDelay doc

Adds a testing/synctest-based test that drives the real run loop against a
failing service and asserts the exact 5s/10s/20s/40s retry schedule and the
drain-on-recovery reset, addressing the review note that the run loop's
behavior was untested. Also clarifies the backoffDelay doc comment: the
argument is a zero-based retry index.
This commit is contained in:
Deluan Quintão 2026-07-19 13:36:05 -04:00 committed by GitHub
parent 5927e693d1
commit e6597398c2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 128 additions and 5 deletions

View File

@ -10,6 +10,30 @@ import (
"github.com/navidrome/navidrome/model/request"
)
const (
minRetryDelay = 5 * time.Second
maxRetryDelay = 4 * time.Minute
// maxRetryShift caps the exponent so the shift never overflows int64.
// minRetryDelay<<6 = 320s already exceeds maxRetryDelay, so 6 reaches the ceiling.
maxRetryShift = 6
)
// backoffDelay returns the delay for a zero-based retry index (0 = first retry):
// minRetryDelay doubled per prior failure, clamped to maxRetryDelay.
func backoffDelay(failures int) time.Duration {
if failures < 0 {
failures = 0
}
if failures >= maxRetryShift {
return maxRetryDelay
}
d := minRetryDelay << failures
if d > maxRetryDelay {
return maxRetryDelay
}
return d
}
// Loader is a function that loads a scrobbler by name.
// It returns the scrobbler and true if found, or nil and false if not available.
// This allows the buffered scrobbler to always get the current plugin instance.
@ -98,15 +122,23 @@ func (b *bufferedScrobbler) sendWakeSignal() {
}
func (b *bufferedScrobbler) run(ctx context.Context) {
timer := time.NewTimer(time.Hour)
timer.Stop()
defer timer.Stop()
failures := 0
for {
if !b.processQueue(ctx) {
time.AfterFunc(5*time.Second, func() {
b.sendWakeSignal()
})
if b.processQueue(ctx) {
failures = 0
timer.Stop()
} else {
timer.Reset(backoffDelay(failures))
if failures < maxRetryShift {
failures++
}
}
select {
case <-b.wakeSignal:
continue
case <-timer.C:
case <-ctx.Done():
return
}

View File

@ -2,6 +2,9 @@ package scrobbler
import (
"context"
"sync/atomic"
"testing"
"testing/synctest"
"time"
"github.com/navidrome/navidrome/model"
@ -100,3 +103,91 @@ var _ = Describe("BufferedScrobbler", func() {
}
})
})
var _ = Describe("backoffDelay", func() {
DescribeTable("computes the exponential backoff curve clamped to the ceiling",
func(failures int, expected time.Duration) {
Expect(backoffDelay(failures)).To(Equal(expected))
},
Entry("first failure", 0, 5*time.Second),
Entry("second failure", 1, 10*time.Second),
Entry("third failure", 2, 20*time.Second),
Entry("fourth failure", 3, 40*time.Second),
Entry("fifth failure", 4, 80*time.Second),
Entry("sixth failure", 5, 160*time.Second),
Entry("reaches the ceiling", 6, 4*time.Minute),
Entry("stays clamped past the ceiling", 7, 4*time.Minute),
Entry("stays clamped for large values", 1000, 4*time.Minute),
Entry("negative is treated as zero", -1, 5*time.Second),
)
})
// Drives the real run loop and asserts the exact retry schedule + recovery. Plain
// test: testing/synctest's fake clock needs a *testing.T, which Ginkgo doesn't give.
func TestBufferedScrobblerBackoffSchedule(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
g := NewWithT(t)
buffer := tests.CreateMockedScrobbleBufferRepo()
userRepo := tests.CreateMockUserRepo()
g.Expect(userRepo.Put(&model.User{ID: "user1", UserName: "alice"})).To(Succeed())
ds := &tests.MockDataStore{MockedScrobbleBuffer: buffer, MockedUser: userRepo}
flaky := &recoveringScrobbler{}
flaky.fail(ErrRetryLater)
bs := newBufferedScrobbler(ds, flaky, "flaky")
defer func() { bs.Stop(); synctest.Wait() }()
// Let the loop settle on the empty buffer, then enqueue a scrobble.
synctest.Wait()
track := model.MediaFile{ID: "123", Title: "Test Track", Artist: "Test Artist"}
g.Expect(bs.Scrobble(context.Background(), "user1", Scrobble{MediaFile: track, TimeStamp: time.Now()})).To(Succeed())
// First attempt fires immediately on the enqueue wake and is left buffered.
synctest.Wait()
g.Expect(flaky.count.Load()).To(Equal(int32(1)))
g.Expect(buffer.Length()).To(Equal(int64(1)))
// Each subsequent retry waits exactly double the previous: 5s, 10s, 20s, 40s.
for i, gap := range []time.Duration{5 * time.Second, 10 * time.Second, 20 * time.Second, 40 * time.Second} {
want := int32(i + 2)
time.Sleep(gap - time.Nanosecond)
synctest.Wait()
g.Expect(flaky.count.Load()).To(Equal(want-1), "retry fired before the %s backoff", gap)
time.Sleep(time.Nanosecond)
synctest.Wait()
g.Expect(flaky.count.Load()).To(Equal(want), "retry did not fire after the %s backoff", gap)
}
// Once the service recovers, waking the loop drains the buffered entry.
flaky.succeed()
bs.sendWakeSignal()
synctest.Wait()
g.Expect(buffer.Length()).To(Equal(int64(0)))
})
}
// recoveringScrobbler is a race-safe Scrobbler whose error can be toggled while
// the buffered scrobbler's goroutine is draining, to exercise retry then recovery.
type recoveringScrobbler struct {
err atomic.Pointer[error]
count atomic.Int32
}
func (f *recoveringScrobbler) fail(err error) { f.err.Store(&err) }
func (f *recoveringScrobbler) succeed() { f.err.Store(nil) }
func (f *recoveringScrobbler) IsAuthorized(context.Context, string) bool { return true }
func (f *recoveringScrobbler) NowPlaying(context.Context, string, *model.MediaFile, int) error {
return nil
}
func (f *recoveringScrobbler) Scrobble(_ context.Context, _ string, _ Scrobble) error {
f.count.Add(1)
if e := f.err.Load(); e != nil {
return *e
}
return nil
}
func (f *recoveringScrobbler) PlaybackReport(context.Context, PlaybackSession) error { return nil }