mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 02:17:25 +02:00
fix(artwork): honor a provider's explicit retry-later delay in the circuit breaker (#6056)
An explicit RetryLaterError now opens the agent's breaker immediately for the provider's own delay, instead of counting it as one generic failure that needs five to open and then always probes after a fixed minute.
This commit is contained in:
parent
09867e5cc1
commit
3784fd0ea7
2 changed files with 31 additions and 1 deletions
|
|
@ -1,6 +1,7 @@
|
|||
package artwork
|
||||
|
||||
import (
|
||||
"cmp"
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
|
|
@ -95,6 +96,9 @@ type breaker struct {
|
|||
// generation identifies the current open episode, so an answer from a call admitted before
|
||||
// the breaker opened cannot be mistaken for evidence that it has recovered.
|
||||
generation int
|
||||
// probeAfter overrides the probe delay for the current episode when a provider named its own
|
||||
// back-off; zero falls back to breakerProbeAfter.
|
||||
probeAfter time.Duration
|
||||
}
|
||||
|
||||
func newBreaker() *breaker { return &breaker{} }
|
||||
|
|
@ -107,7 +111,7 @@ func (b *breaker) allow() (bool, int) {
|
|||
if b.failures < breakerThreshold {
|
||||
return true, 0
|
||||
}
|
||||
if time.Since(b.openedAt) >= breakerProbeAfter {
|
||||
if time.Since(b.openedAt) >= cmp.Or(b.probeAfter, breakerProbeAfter) {
|
||||
b.openedAt = time.Now() // start a fresh probe window so only one caller passes
|
||||
return true, b.generation
|
||||
}
|
||||
|
|
@ -121,11 +125,24 @@ func (b *breaker) record(name string, gen int, err error) {
|
|||
}
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
// An explicit back-off is a definitive "stop for this long", so it opens the breaker at once
|
||||
// with the provider's own delay instead of waiting for the failure threshold.
|
||||
if retry, ok := errors.AsType[*agents.RetryLaterError](err); ok && retry.RetryIn > 0 {
|
||||
b.recoveries = 0
|
||||
b.failures = breakerThreshold
|
||||
b.openedAt = time.Now()
|
||||
b.probeAfter = retry.RetryIn
|
||||
b.generation++
|
||||
log.Warn("Artwork: Circuit breaker opened for agent, provider asked to back off", "agent", name,
|
||||
"probeAfter", retry.RetryIn)
|
||||
return
|
||||
}
|
||||
if isTransientExternal(err) {
|
||||
b.recoveries = 0
|
||||
b.failures++
|
||||
if b.failures == breakerThreshold {
|
||||
b.openedAt = time.Now()
|
||||
b.probeAfter = 0
|
||||
b.generation++
|
||||
log.Warn("Artwork: Circuit breaker opened for agent", "agent", name,
|
||||
"consecutiveFailures", b.failures, "probeAfter", breakerProbeAfter, err)
|
||||
|
|
|
|||
|
|
@ -2,7 +2,9 @@ package artwork
|
|||
|
||||
import (
|
||||
"errors"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
|
@ -40,4 +42,15 @@ var _ = Describe("breaker", func() {
|
|||
Expect(allowed(b)).To(BeFalse(),
|
||||
"answers from calls admitted before the breaker opened must not close it")
|
||||
})
|
||||
|
||||
It("opens at once when a provider asks to retry later, honoring its delay", func() {
|
||||
b := newBreaker()
|
||||
Expect(allowed(b)).To(BeTrue(), "starts closed")
|
||||
|
||||
// A single explicit back-off opens the breaker without reaching the failure threshold.
|
||||
b.record("agentA", 0, &agents.RetryLaterError{RetryIn: 5 * time.Second})
|
||||
|
||||
Expect(allowed(b)).To(BeFalse(), "an explicit back-off opens the breaker immediately")
|
||||
Expect(b.probeAfter).To(Equal(5*time.Second), "the provider's delay drives the probe interval")
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue