mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
perf(db): add jitter to stagger due-on-restart connections (#6919)
* 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 <oyi77@users.noreply.github.com> Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
@@ -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<string> {
|
||||
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) {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user