From 05b44fa48e214e24c66f699b3e60c951dfde7819 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Wed, 16 Sep 2026 06:14:46 -0300 Subject: [PATCH] fix(db): stop backoff-reset from busting the model catalog cache (#13389) (#13783) Merged in the 2026-09-16 sweep of the maintainer's own open PRs, at the owner's explicit instruction. No push was made to the PR branch: the merge took the head as the owning session left it (verified OPEN, non-draft and MERGEABLE against the release tip immediately before merging). --- .../13389-catalog-cache-backoff-reset.md | 1 + src/lib/db/providers.ts | 4 +- src/lib/db/readCache.ts | 18 +++- ...-13389-catalog-cache-backoff-reset.test.ts | 86 +++++++++++++++++++ 4 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 changelog.d/fixes/13389-catalog-cache-backoff-reset.md create mode 100644 tests/unit/issue-13389-catalog-cache-backoff-reset.test.ts diff --git a/changelog.d/fixes/13389-catalog-cache-backoff-reset.md b/changelog.d/fixes/13389-catalog-cache-backoff-reset.md new file mode 100644 index 0000000000..943befe238 --- /dev/null +++ b/changelog.d/fixes/13389-catalog-cache-backoff-reset.md @@ -0,0 +1 @@ +- **fix(db):** stop routine connection-backoff auto-recovery from busting the entire `/v1/models` response cache, which was causing intermittent 75-120s/502 responses on deployments routing many providers (#13389) — thanks @RaviTharuma diff --git a/src/lib/db/providers.ts b/src/lib/db/providers.ts index ab59a69255..1337f8e8c8 100644 --- a/src/lib/db/providers.ts +++ b/src/lib/db/providers.ts @@ -1126,6 +1126,8 @@ export async function touchConnectionSyncedModelsAt(id: string): Promise { * since the caller already verified the connection is eligible for reset. * Resets all backoff/error columns so the connection re-enters the selection pool. * Does invalidateDbCache + bumpProxyConfigGeneration since backoff affects priority. + * #13389: `skipModelCatalog` — the catalog builder never reads backoff/error + * state, so this must not bust the expensive-to-rebuild `/v1/models` cache. */ export async function resetConnectionBackoff(id: string): Promise { if (!id) return; @@ -1146,7 +1148,7 @@ export async function resetConnectionBackoff(id: string): Promise { updatedAt: now, id, }); - invalidateDbCache("connections"); + invalidateDbCache("connections", id, { skipModelCatalog: true }); bumpProxyConfigGeneration(); } diff --git a/src/lib/db/readCache.ts b/src/lib/db/readCache.ts index 088e494a6f..8c270f4f4a 100644 --- a/src/lib/db/readCache.ts +++ b/src/lib/db/readCache.ts @@ -276,10 +276,25 @@ export function invalidateModelCatalogCache(): void { * connection's by-ID cache entry is invalidated (the filter-keyed raw * cache must still be fully cleared since overlapping filter results * cannot be selectively invalidated). + * + * `skipModelCatalog` (#13389): the unified `/v1/models` builder + * (`src/app/api/v1/models/catalog.ts`) never reads routing/health-only + * connection fields — `backoffLevel`, `testStatus`, `rateLimitedUntil`, + * `lastError*`, `errorCode` — only structural fields such as + * `excludedModels` or enabled/disabled. A caller that only touched those + * routing fields (e.g. `resetConnectionBackoff`) should still bust the + * connections read cache but must NOT bump `modelCatalogCacheVersion`: + * doing so was busting the entire `/v1/models` response cache on every + * routine backoff auto-recovery during normal request routing, far more + * often than the cache's own 60s TTL / 30s stale-while-revalidate window + * intends, forcing frequent expensive cold rebuilds. Structural connection + * writes (create/update/delete) must keep the default (omit this flag) so + * the catalog still reflects them immediately. */ export function invalidateDbCache( scope?: "settings" | "pricing" | "connections" | "combos" | "nodes" | "model-capabilities", - id?: string + id?: string, + opts?: { skipModelCatalog?: boolean } ): void { if (!scope || scope === "settings") settingsCache.invalidate(); if (!scope || scope === "pricing") pricingCache.invalidate(); @@ -294,6 +309,7 @@ export function invalidateDbCache( } if (!scope || scope === "nodes") nodesCache.invalidate(); if (!scope || scope === "combos") combosCacheVersion++; + if (opts?.skipModelCatalog) return; // Settings/connections/combos all feed the unified model catalog builder // (blockedProviders + hidePaidModels, provider connections + excludedModels, // combo definitions, respectively) — pricing does too, via isFreeModel(). diff --git a/tests/unit/issue-13389-catalog-cache-backoff-reset.test.ts b/tests/unit/issue-13389-catalog-cache-backoff-reset.test.ts new file mode 100644 index 0000000000..5852d37e66 --- /dev/null +++ b/tests/unit/issue-13389-catalog-cache-backoff-reset.test.ts @@ -0,0 +1,86 @@ +/** + * #13389 — GET /v1/models intermittently takes 75-120s or returns 502. + * + * Must-fix half of the issue: `resetConnectionBackoff()` (`src/lib/db/providers.ts`) + * fires automatically whenever a previously-cooled-down connection is + * auto-recovered during normal request routing (`src/sse/services/auth.ts`), and + * previously busted the *entire* `/v1/models` response cache as a side effect — + * even though the catalog builder (`src/app/api/v1/models/catalog.ts`) never reads + * backoff/cooldown/error state at all (only structural fields like + * `excludedModels` or enabled/disabled). On a deployment routing many providers, + * this invalidated the catalog cache far more often than its 60s TTL / 30s + * stale-while-revalidate window intends, purely as a side effect of unrelated + * chat traffic, forcing frequent expensive cold rebuilds. + * + * This regression test asserts a `resetConnectionBackoff()` call does NOT bump + * `getModelCatalogCacheVersion()`, while a genuinely structural connection write + * (`updateProviderConnection`) still does. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-13389-cache-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = "test-13389-catalog-cache-backoff-reset-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const readCache = await import("../../src/lib/db/readCache.ts"); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +async function createBackedOffConnection() { + const created = await providersDb.createProviderConnection({ + provider: "glm", + authType: "apikey", + name: `GLM Backoff 13389 ${Date.now()}-${Math.random()}`, + apiKey: "glm-test-key", + }); + const connectionId = (created as { id: string }).id; + await providersDb.updateProviderConnection(connectionId, { + testStatus: "unavailable", + lastError: "rate limit exceeded", + lastErrorType: "rate_limited", + lastErrorSource: "executor", + errorCode: 429, + backoffLevel: 3, + }); + return connectionId; +} + +test("#13389 resetConnectionBackoff does NOT bust the model catalog cache", async () => { + const connectionId = await createBackedOffConnection(); + + const versionBeforeReset = readCache.getModelCatalogCacheVersion(); + + await providersDb.resetConnectionBackoff(connectionId); + + assert.equal( + readCache.getModelCatalogCacheVersion(), + versionBeforeReset, + "a pure backoff/error-state reset must not invalidate the model catalog cache — " + + "the catalog builder never reads backoffLevel/testStatus/rateLimitedUntil" + ); +}); + +test("#13389 a structural connection write still busts the model catalog cache", async () => { + const connectionId = await createBackedOffConnection(); + + const versionBeforeUpdate = readCache.getModelCatalogCacheVersion(); + + // excludedModels is catalog-relevant (structural) — must still invalidate. + await providersDb.updateProviderConnection(connectionId, { + excludedModels: ["some-model-id"], + }); + + assert.ok( + readCache.getModelCatalogCacheVersion() > versionBeforeUpdate, + "a structural connection write (e.g. excludedModels) must still invalidate the model catalog cache" + ); +});