Merge pull request #5266 from shentry/fix/transient-streak-rate-dependence
fix(openai): keep transient failure streak from resetting on sparse traffic
This commit is contained in:
2 files changed
+58
-4
No files matched your search
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user