fix(openai): keep transient failure streak from resetting on sparse traffic

The account+model transient breaker reset its failure streak whenever the
gap since the previous failure exceeded a one-minute window. That made the
breaker's sensitivity a function of request rate rather than upstream
health: a gateway called less often than once a minute never advanced past
streak 1, where the cooldown is zero, so a consistently broken account was
never blocked. Every request re-selected it, paid a full upstream attempt,
and only then failed over to a healthy account.

Observed on a low-traffic deployment: two accounts returning 500 and 503
stayed in rotation indefinitely, logging `failure_streak: 1, cooldown_ms: 0`
on every request and adding ~750ms to each one before a working account was
reached.

The streak is already cleared on success — recordSuccess deletes the entry,
and every OpenAI handler reports the schedule result — so the time-based
reset is not needed to recover a healthy account. Keep a TTL purely to bound
the map for account+model pairs that stopped being used, and raise it well
above the cooldowns so it no longer doubles as a streak reset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
shentry
2026-08-04 17:59:16 +08:00
co-authored by Claude Opus 5
parent a4d263f62f
commit 7d38e67120
2 changed files with 58 additions and 4 deletions
@@ -7,7 +7,19 @@ import (
)
const (
openAIModelTransientFailureWindow = time.Minute
// openAIModelTransientStreakTTL bounds how long a failure streak survives
// without a new failure. It exists only so the map does not keep state for
// account+model pairs that stopped being used; a streak is otherwise reset
// by recordSuccess alone.
//
// It must stay well above the cooldowns. Resetting the streak on a short
// wall-clock window makes the breaker's sensitivity depend on request rate:
// a gateway called less often than the window never reaches streak 2, so a
// broken upstream is never cooled down and every request pays a failed
// attempt plus a failover before reaching a healthy account. Low-traffic
// deployments were hit hardest, which is the opposite of what a breaker
// should do.
openAIModelTransientStreakTTL = 30 * time.Minute
openAIModelTransientShortCooldown = 10 * time.Second
openAIModelTransientLongCooldown = 45 * time.Second
openAIModelTransientDefaultMax = 4096
@@ -86,7 +98,9 @@ func (s *openAIAccountModelTransientState) recordFailure(accountID int64, model
if !exists {
s.evictOldestLocked()
}
if !exists || entry.lastFailure.IsZero() || now.Sub(entry.lastFailure) > openAIModelTransientFailureWindow || now.Before(entry.lastFailure) {
// The streak is cleared by recordSuccess. Only drop it here when the entry
// is stale beyond the TTL, or when the clock moved backwards.
if !exists || entry.lastFailure.IsZero() || now.Sub(entry.lastFailure) > openAIModelTransientStreakTTL || now.Before(entry.lastFailure) {
entry.failureStreak = 0
entry.blockUntil = time.Time{}
}
@@ -139,7 +153,7 @@ func (s *openAIAccountModelTransientState) isBlocked(accountID int64, model stri
if !exists {
return false
}
if !entry.lastFailure.IsZero() && now.Sub(entry.lastFailure) > openAIModelTransientFailureWindow {
if !entry.lastFailure.IsZero() && now.Sub(entry.lastFailure) > openAIModelTransientStreakTTL {
delete(s.entries, key)
return false
}
@@ -80,12 +80,52 @@ func TestOpenAIModelTransient_StaleStreakExpires(t *testing.T) {
now := time.Date(2026, 7, 10, 10, 0, 0, 0, time.UTC)
state.recordFailure(35, "gpt-5.5", now)
decision := state.recordFailure(35, "gpt-5.5", now.Add(openAIModelTransientFailureWindow+time.Second))
decision := state.recordFailure(35, "gpt-5.5", now.Add(openAIModelTransientStreakTTL+time.Second))
assert.Equal(t, 1, decision.FailureStreak)
assert.Zero(t, decision.Cooldown)
}
// A streak must not depend on how often the gateway is called. Sparse traffic
// used to reset the streak between every request, so a broken account+model was
// never cooled down and each request paid a failed attempt plus a failover.
func TestOpenAIModelTransient_StreakSurvivesSparseTraffic(t *testing.T) {
state := newOpenAIAccountModelTransientState(128)
now := time.Date(2026, 7, 10, 10, 0, 0, 0, time.UTC)
gap := 5 * time.Minute
require.Greater(t, gap, openAIModelTransientLongCooldown,
"the gap must exceed every cooldown, otherwise this passes for the wrong reason")
first := state.recordFailure(35, "gpt-5.5", now)
second := state.recordFailure(35, "gpt-5.5", now.Add(gap))
third := state.recordFailure(35, "gpt-5.5", now.Add(2*gap))
assert.Equal(t, 1, first.FailureStreak)
assert.Zero(t, first.Cooldown)
assert.Equal(t, 2, second.FailureStreak)
assert.Equal(t, openAIModelTransientShortCooldown, second.Cooldown)
assert.Equal(t, 3, third.FailureStreak)
assert.Equal(t, openAIModelTransientLongCooldown, third.Cooldown)
assert.True(t, state.isBlocked(35, "gpt-5.5", now.Add(2*gap+time.Second)))
}
// A success between two sparse failures still clears the streak, so an account
// that intermittently works is not pushed into the long cooldown.
func TestOpenAIModelTransient_SuccessResetsStreakAcrossSparseTraffic(t *testing.T) {
state := newOpenAIAccountModelTransientState(128)
now := time.Date(2026, 7, 10, 10, 0, 0, 0, time.UTC)
gap := 5 * time.Minute
state.recordFailure(35, "gpt-5.5", now)
state.recordSuccess(35, "gpt-5.5")
decision := state.recordFailure(35, "gpt-5.5", now.Add(gap))
assert.Equal(t, 1, decision.FailureStreak)
assert.Zero(t, decision.Cooldown)
assert.False(t, state.isBlocked(35, "gpt-5.5", now.Add(gap+time.Second)))
}
func TestOpenAIModelTransient_IgnoresInvalidKeys(t *testing.T) {
state := newOpenAIAccountModelTransientState(128)
now := time.Date(2026, 7, 10, 10, 0, 0, 0, time.UTC)