From 7d38e67120c371f522164ba850669923a01cd53c Mon Sep 17 00:00:00 2001 From: shentry <111497882+shentry@users.noreply.github.com> Date: Tue, 4 Aug 2026 17:59:16 +0800 Subject: [PATCH] fix(openai): keep transient failure streak from resetting on sparse traffic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../service/openai_account_model_transient.go | 20 +++++++-- .../openai_account_model_transient_test.go | 42 ++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/backend/internal/service/openai_account_model_transient.go b/backend/internal/service/openai_account_model_transient.go index 6ef710d23c..60fcff9053 100644 --- a/backend/internal/service/openai_account_model_transient.go +++ b/backend/internal/service/openai_account_model_transient.go @@ -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 } diff --git a/backend/internal/service/openai_account_model_transient_test.go b/backend/internal/service/openai_account_model_transient_test.go index bd8d92f4d2..f395883045 100644 --- a/backend/internal/service/openai_account_model_transient_test.go +++ b/backend/internal/service/openai_account_model_transient_test.go @@ -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)