From e9be562bf19e396b8235a5027e87ec2c3e5ef6a3 Mon Sep 17 00:00:00 2001 From: KooshaPari <42529354+KooshaPari@users.noreply.github.com> Date: Mon, 22 Jun 2026 13:13:40 -0700 Subject: [PATCH] fix: retain provider cooldowns for configured max window (#4588) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integrated into release/v3.8.34 (rebuilt — bundled commits stripped) --- open-sse/services/providerCooldownTracker.ts | 38 ++++++++++++++----- .../services/providerCooldownTracker.test.ts | 28 ++++++++++++++ 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/open-sse/services/providerCooldownTracker.ts b/open-sse/services/providerCooldownTracker.ts index afb5a9303f..f5a14d2006 100644 --- a/open-sse/services/providerCooldownTracker.ts +++ b/open-sse/services/providerCooldownTracker.ts @@ -17,13 +17,16 @@ interface CooldownEntry { lastFailureAt: number; /** Number of consecutive failures (resets on success) */ failureCount: number; + /** How long this entry must be retained for cleanup purposes */ + retentionMs: number; } // Global cooldown state: keyed by "provider:connectionId" or "provider" const cooldownMap = new Map(); -// Evict entries older than this to prevent unbounded memory growth -const MAX_ENTRY_AGE_MS = 30 * 60 * 1000; // 30 minutes +// Evict entries older than their configured retention horizon to prevent +// unbounded memory growth without shortening operator-configured cooldowns. +const DEFAULT_ENTRY_RETENTION_MS = 30 * 60 * 1000; // 30 minutes const CLEANUP_INTERVAL_MS = 60 * 1000; // Cleanup every 60s let cleanupTimer: ReturnType | null = null; @@ -31,17 +34,32 @@ let cleanupTimer: ReturnType | null = null; function startCleanupIfNeeded(): void { if (cleanupTimer) return; cleanupTimer = setInterval(() => { - const now = Date.now(); - for (const [key, entry] of cooldownMap) { - if (now - entry.lastFailureAt > MAX_ENTRY_AGE_MS) { - cooldownMap.delete(key); - } - } + cleanupExpiredCooldownEntries(); }, CLEANUP_INTERVAL_MS); // Allow Node.js to exit even if the timer is running if (cleanupTimer.unref) cleanupTimer.unref(); } +function getEntryRetentionMs(settings?: ResilienceSettings): number { + const maxRetryCooldownMs = + settings?.providerCooldown?.maxRetryCooldownMs ?? + DEFAULT_RESILIENCE_SETTINGS.providerCooldown.maxRetryCooldownMs; + return Math.max(DEFAULT_ENTRY_RETENTION_MS, maxRetryCooldownMs); +} + +/** + * Remove expired cooldown entries using each entry's configured retention + * horizon. Exported for diagnostics and focused tests; normal runtime cleanup is + * still performed by the unref'd interval started on first cooldown record. + */ +export function cleanupExpiredCooldownEntries(now = Date.now()): void { + for (const [key, entry] of cooldownMap) { + if (now - entry.lastFailureAt > entry.retentionMs) { + cooldownMap.delete(key); + } + } +} + /** * Build a cooldown key from provider and optional connectionId. */ @@ -66,12 +84,14 @@ export function recordProviderCooldown( const key = cooldownKey(provider, connectionId); const existing = cooldownMap.get(key); const now = Date.now(); + const retentionMs = getEntryRetentionMs(settings); if (existing) { existing.lastFailureAt = now; existing.failureCount++; + existing.retentionMs = Math.max(existing.retentionMs, retentionMs); } else { - cooldownMap.set(key, { lastFailureAt: now, failureCount: 1 }); + cooldownMap.set(key, { lastFailureAt: now, failureCount: 1, retentionMs }); } startCleanupIfNeeded(); diff --git a/tests/unit/services/providerCooldownTracker.test.ts b/tests/unit/services/providerCooldownTracker.test.ts index 335575ee86..8d3f88f35f 100644 --- a/tests/unit/services/providerCooldownTracker.test.ts +++ b/tests/unit/services/providerCooldownTracker.test.ts @@ -7,6 +7,7 @@ import { recordProviderSuccess, clearCooldownState, getCooldownEntryCount, + cleanupExpiredCooldownEntries, } from "../../../open-sse/services/providerCooldownTracker.ts"; import { resolveResilienceSettings, @@ -140,6 +141,33 @@ test("cooldown respects maxRetryCooldownMs cap", () => { assert.ok(remaining <= 20000, "cooldown capped at maxRetryCooldownMs"); }); +test("cleanup keeps entries for configured long maxRetryCooldownMs", () => { + const settings = makeSettings(5000, 60 * 60 * 1000); + const originalNow = Date.now; + + try { + let fakeNow = Date.now(); + Date.now = () => fakeNow; + + recordProviderCooldown("openai", "conn-1", settings); + assert.equal(getCooldownEntryCount(), 1); + + fakeNow += 31 * 60 * 1000; + cleanupExpiredCooldownEntries(); + assert.equal( + getCooldownEntryCount(), + 1, + "cleanup must not evict entries before configured maxRetryCooldownMs" + ); + + fakeNow += 30 * 60 * 1000; + cleanupExpiredCooldownEntries(); + assert.equal(getCooldownEntryCount(), 0); + } finally { + Date.now = originalNow; + } +}); + test("different connections have independent cooldowns", () => { const settings = makeSettings(); recordProviderCooldown("openai", "conn-1", settings);