From 16e481ba3e73f1195f2bbbc8b072c0e8e78bb697 Mon Sep 17 00:00:00 2001 From: Paijo <14921983+oyi77@users.noreply.github.com> Date: Sun, 19 Jul 2026 01:14:48 +0700 Subject: [PATCH] perf(db): add jitter to stagger due-on-restart connections (#6919) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(db): add jitter to stagger due-on-restart connections - Add MIN_RESTART_REFRESH_JITTER_MS=500 and MAX_RESTART_REFRESH_JITTER_MS=5000 - Replace fixed stagger delay with stagger + random jitter in sweep() - Export sweep() for testing (marked @internal) - Test: 3 connections with 100ms base stagger, verify all processed - Uses Promise.withResolvers() pattern * fix(test): make the sweep jitter test actually assert the jitter floor The "sweep processes all connections with stagger + jitter delay" test asserted elapsed >= 50ms, which was already trivially satisfied by the pre-existing fixed stagger alone (3 connections -> 2 gaps * 100ms = 200ms), so the test passed identically whether or not the jitter change was present and never actually exercised the new behavior. Tighten the bound to >= 1000ms: with MIN_RESTART_REFRESH_JITTER_MS=500 and MAX=5000, the true floor with jitter is 2 * (100 + 500) = 1200ms — a hard guarantee (setTimeout never fires early), not a probabilistic one. Verified this fails (205ms) with the jitter term zeroed out and passes (7-9s, within the [1200ms, 10200ms] range) with it restored. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * fix(health): make jitter configurable via env vars and tighten test assertion - Replace hardcoded jitter [500, 5000)ms with HEALTHCHECK_JITTER_MIN_MS / HEALTHCHECK_JITTER_MAX_MS env vars (defaults 500/5000). - In test: set HEALTHCHECK_STAGGER_MS=1, HEALTHCHECK_JITTER_MIN_MS=100, HEALTHCHECK_JITTER_MAX_MS=100 (fixed jitter), assert elapsed >= 190ms. - Without jitter: 2 gaps * 1ms = ~2ms. With jitter: 2 gaps * 101ms = ~202ms. The assert proves jitter is applied. Fixes #6919 * refactor(health): compact jitter await + tighten comments (file-size cap) Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: oyi77 Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- src/lib/tokenHealthCheck.ts | 19 +++-- .../apikey-connection-health-check.test.ts | 74 +++++++++++++++++++ 2 files changed, 83 insertions(+), 10 deletions(-) diff --git a/src/lib/tokenHealthCheck.ts b/src/lib/tokenHealthCheck.ts index e1d091c5ef..5593edfdba 100644 --- a/src/lib/tokenHealthCheck.ts +++ b/src/lib/tokenHealthCheck.ts @@ -39,7 +39,6 @@ function isBuildProcess(): boolean { return typeof process !== "undefined" && process.env.NEXT_PHASE === "phase-production-build"; } - function getConnectionLogLabel(conn: { name?: string; email?: string; id?: string }): string { return pickMaskedDisplayValue([conn.name, conn.email], conn.id || "-"); } @@ -178,13 +177,10 @@ function isHealthCheckDisabled(): boolean { } /** - * Providers excluded from the PROACTIVE refresh sweep, comma-separated and - * case-insensitive (e.g. "codex,openai"). A targeted alternative to the blunt - * OMNIROUTE_DISABLE_TOKEN_HEALTHCHECK switch: it lets an operator keep the - * rotating-token cascade providers (Codex/OpenAI share one Auth0 family) off the - * proactive sweep — leaving their refresh to the reactive, serialized 401 path — - * WITHOUT also starving short-TTL providers like Kimi-coding, whose tokens expire - * while idle when the whole sweep is disabled. + * Providers excluded from the PROACTIVE sweep, comma-separated, case-insensitive + * (e.g. "codex,openai"). Targeted alternative to OMNIROUTE_DISABLE_TOKEN_HEALTHCHECK: + * keeps rotating-token cascade providers (Codex/OpenAI share one Auth0 family) on the + * reactive 401 path WITHOUT starving short-TTL providers (Kimi-coding) sweep-wide. */ function getHealthCheckSkipProviders(): Set { const raw = process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS || ""; @@ -337,9 +333,12 @@ export async function sweep() { logError(`${LOG_PREFIX} Error checking ${conn.name || conn.id}:`, err.message); } - // Stagger delay between checks to prevent bursting (Issue #1220) + // Stagger + randomized jitter between checks to prevent bursting (Issue #1220) if (staggerMs > 0 && i < connections.length - 1) { - await new Promise((resolve) => setTimeout(resolve, staggerMs)); + const jitterMin = parseInt(process.env.HEALTHCHECK_JITTER_MIN_MS || "500", 10); + const jitterMax = parseInt(process.env.HEALTHCHECK_JITTER_MAX_MS || "5000", 10); + const jitter = jitterMin + Math.random() * Math.max(0, jitterMax - jitterMin); + await new Promise((resolve) => setTimeout(resolve, staggerMs + jitter)); } } } catch (err) { diff --git a/tests/unit/apikey-connection-health-check.test.ts b/tests/unit/apikey-connection-health-check.test.ts index 7f5b0b0d8e..582f0ad50f 100644 --- a/tests/unit/apikey-connection-health-check.test.ts +++ b/tests/unit/apikey-connection-health-check.test.ts @@ -171,3 +171,77 @@ test("connection with both apiKey and refreshToken: refresh path is tried", asyn "dual-auth connection with stale refresh token should be expired (refresh path takes precedence)" ); }); + +test("sweep processes all connections with stagger + jitter delay", async () => { + await resetStorage(); + + // Create multiple connections; set isActive=false so checkConnection + // returns immediately at the !conn.isActive guard without OAuth calls. + const c1 = await providersDb.createProviderConnection({ + provider: "openai", + authType: "oauth", + name: "Stagger Test 1", + email: "t1@example.com", + refreshToken: "test-rt", + isActive: false, + }); + const c2 = await providersDb.createProviderConnection({ + provider: "openai", + authType: "oauth", + name: "Stagger Test 2", + email: "t2@example.com", + refreshToken: "test-rt", + isActive: false, + }); + const c3 = await providersDb.createProviderConnection({ + provider: "openai", + authType: "oauth", + name: "Stagger Test 3", + email: "t3@example.com", + refreshToken: "test-rt", + isActive: false, + }); + + // Clear any health-check skip config + const origSetting = process.env.HEALTHCHECK_SKIP_PROVIDERS; + const origJitterMin = process.env.HEALTHCHECK_JITTER_MIN_MS; + const origJitterMax = process.env.HEALTHCHECK_JITTER_MAX_MS; + delete process.env.HEALTHCHECK_SKIP_PROVIDERS; + process.env.HEALTHCHECK_STAGGER_MS = "1"; + process.env.HEALTHCHECK_JITTER_MIN_MS = "100"; + process.env.HEALTHCHECK_JITTER_MAX_MS = "100"; // fixed jitter = deterministic + + try { + // Import sweep — exported for testing from tokenHealthCheck + const { sweep } = await import("../../src/lib/tokenHealthCheck.ts"); + + const start = Date.now(); + await sweep(); + const elapsed = Date.now() - start; + + // 3 connections -> 2 gaps. Each gap waits + // HEALTHCHECK_STAGGER_MS (1ms) + HEALTHCHECK_JITTER_MIN_MS (100ms) = 101ms. + // Without jitter: 2 * 1ms = ~2ms. With jitter: 2 * 101ms = ~202ms. + // The assert proves the jitter is actually applied. + assert.ok( + elapsed >= 190, + `sweep took ${elapsed}ms — expected >= 190ms with jitter applied (jitter-free baseline would be ~2ms)` + ); + + // Verify all connections still exist (sweep didn't error out mid-loop) + const reloaded1 = await providersDb.getProviderConnectionById(c1.id); + const reloaded2 = await providersDb.getProviderConnectionById(c2.id); + const reloaded3 = await providersDb.getProviderConnectionById(c3.id); + assert.ok(reloaded1, "connection 1 should still exist"); + assert.ok(reloaded2, "connection 2 should still exist"); + assert.ok(reloaded3, "connection 3 should still exist"); + } finally { + if (origSetting !== undefined) process.env.HEALTHCHECK_SKIP_PROVIDERS = origSetting; + else delete process.env.HEALTHCHECK_SKIP_PROVIDERS; + if (origJitterMin !== undefined) process.env.HEALTHCHECK_JITTER_MIN_MS = origJitterMin; + else delete process.env.HEALTHCHECK_JITTER_MIN_MS; + if (origJitterMax !== undefined) process.env.HEALTHCHECK_JITTER_MAX_MS = origJitterMax; + else delete process.env.HEALTHCHECK_JITTER_MAX_MS; + delete process.env.HEALTHCHECK_STAGGER_MS; + } +});