mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-21 06:12:17 +03:00
fix(redis): namespace warmup circuit-breaker keys with REDIS_KEY_PREFIX (#13328)
The warmup scheduler's circuit-breaker keys were written to Redis without `REDIS_KEY_PREFIX`, so they escaped OmniRoute's namespace and could collide with another app sharing the instance — the one Redis surface the prefix wasn't reaching. Probe: 2/2 pass in `tests/unit/lib/warmupScheduler/redisCircuitBreakerStorePrefix.test.ts`, covering both the prefixed case and the unset/blank case where keys must stay unchanged.
**Batch validation** — boarded with the other 10 PRs of your batch into one worktree cut from `release/v3.8.51`; every PR verified as an ancestor of the combined HEAD before validating.
- Focused tests across all 11 PRs: **104/104 pass** on the combined tree.
- Gates on the combined tree: `check-changelog-integrity` PASS, `check-complexity` PASS, `check-cognitive-complexity` PASS, `typecheck:core` PASS, `check:open-sse-typecheck` PASS.
- `check-file-size` is red, but reproduces with byte-identical line counts on the pure `release/v3.8.51` tip (`open-sse/handlers/imageGeneration.ts` 3304, `open-sse/services/combo/roundRobinCombo.ts` 1221, `open-sse/utils/stream.ts` 3115). Inherited base-red, nothing added by this batch — it is also why this PR's "Fast Quality Gates" check was red.
**Reconciled** — this PR was `CONFLICTING`. The conflict was in `docs/reference/ENVIRONMENT.md` and purely additive: the release tip had inserted `APP_BIND_HOST` / `QDRANT_BIND_HOST` / `BIFROST_BIND_HOST` rows directly above the `REDIS_KEY_PREFIX` row you edited. Kept both sides — the tip's three new rows and your updated description naming the warmup circuit breaker — then merged the current release branch in (120a92f6) and re-ran your focused test on the reconciled tree: 2/2 pass. No line of your diff was dropped.
Thanks, @datrixlab — you also updated `.env.example`, `docs/ops/REDIS_PRODUCTION_CONFIG.md` and `ENVIRONMENT.md` alongside the code, which is why the only thing left to do here was a mechanical conflict resolution.
This commit is contained in:
@@ -112,8 +112,9 @@ DISABLE_SQLITE_AUTO_BACKUP=false
|
||||
# Example: redis://localhost:6379 (or redis://redis:6379 in Docker)
|
||||
# REDIS_URL=redis://localhost:6379
|
||||
# Namespace prefix for ALL OmniRoute Redis keys (rate limiter + auth cache +
|
||||
# quota store). Prevents key collisions when OmniRoute shares a Redis instance
|
||||
# with other apps (e.g. on 127.0.0.1:6379). Default when unset: omniroute:
|
||||
# quota store + warmup circuit breaker). Prevents key collisions when OmniRoute
|
||||
# shares a Redis instance with other apps (e.g. on 127.0.0.1:6379). Default when
|
||||
# unset: omniroute:
|
||||
# REDIS_KEY_PREFIX=omniroute:
|
||||
# Host interface docker-compose publishes the Redis sidecar on.
|
||||
# Default: 127.0.0.1 (loopback only). The compose Redis runs WITHOUT
|
||||
|
||||
1
changelog.d/fixes/13328-warmup-cb-redis-key-prefix.md
Normal file
1
changelog.d/fixes/13328-warmup-cb-redis-key-prefix.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(redis):** Warmup circuit-breaker keys now honor `REDIS_KEY_PREFIX` like every other OmniRoute Redis key, instead of always using `omniroute:warmup:cb:` ([#13328](https://github.com/diegosouzapw/OmniRoute/pull/13328))
|
||||
@@ -9,7 +9,7 @@ lastUpdated: 2026-08-06
|
||||
## Overview
|
||||
|
||||
Redis is an **optional, soft dependency** in OmniRoute — the application degrades gracefully (in-memory
|
||||
fallbacks) when Redis is unavailable. In production, tuning Redis reduces latency for three distinct
|
||||
fallbacks) when Redis is unavailable. In production, tuning Redis reduces latency for four distinct
|
||||
workloads:
|
||||
|
||||
| Workload | Driver | Client Factory | Key Pattern |
|
||||
@@ -17,8 +17,9 @@ workloads:
|
||||
| Rate limiting | `rateLimiter.ts` | `getRedisClient()` — lazy `ioredis` singleton | `<prefix>rl:*` Lua‑atomic rate limit windows |
|
||||
| Auth cache | `apiKeys.ts` | Reuses `rateLimiter`'s client | `<prefix>auth:api_key:<sha256>` with TTL |
|
||||
| Quota store | `redisQuotaStore.ts` | Separate `getRedisClient(url)` singleton | `<prefix>quota:*` configurable per-instance |
|
||||
| Warmup circuit breaker | `redisCircuitBreakerStore.ts` | Separate client in `circuitBreakerFactory.ts` | `<prefix>warmup:cb:<connectionId>` |
|
||||
|
||||
All three workloads share one namespace prefix so OmniRoute can co-exist with other apps on a
|
||||
All four workloads share one namespace prefix so OmniRoute can co-exist with other apps on a
|
||||
single Redis instance (e.g. `127.0.0.1:6379`). See [Key Namespacing](#key-namespacing).
|
||||
|
||||
---
|
||||
@@ -28,7 +29,7 @@ single Redis instance (e.g. `127.0.0.1:6379`). See [Key Namespacing](#key-namesp
|
||||
| Setting | Value | Where |
|
||||
|---|---|---|
|
||||
| `REDIS_URL` env var | `redis://redis:6379` (compose), optional | `rateLimiter.ts:5`, `.env.example` |
|
||||
| `REDIS_KEY_PREFIX` env var | `omniroute:` (default) | `rateLimiter.ts`, `redisQuotaStore.ts`, `.env.example` |
|
||||
| `REDIS_KEY_PREFIX` env var | `omniroute:` (default) | `rateLimiter.ts`, `redisQuotaStore.ts`, `redisCircuitBreakerStore.ts`, `.env.example` |
|
||||
| `QUOTA_STORE_REDIS_URL` env var | separate, can differ from `REDIS_URL` | `quota/storeFactory.ts` |
|
||||
| `QUOTA_STORE_DRIVER` | `"sqlite"` (default), `"redis"` optional | `quota/storeFactory.ts` |
|
||||
| ioredis `maxRetriesPerRequest` | `3` | `rateLimiter.ts` client creation |
|
||||
@@ -49,15 +50,18 @@ using the same Redis (this instance runs Redis on `127.0.0.1:6379` alongside oth
|
||||
Set `REDIS_KEY_PREFIX` to a non-empty string to prefix **every** OmniRoute key:
|
||||
|
||||
```bash
|
||||
# .env — all OmniRoute keys become omniroute:rl:*, omniroute:auth:*, omniroute:quota:*
|
||||
# .env — all OmniRoute keys become omniroute:rl:*, omniroute:auth:*, omniroute:quota:*, omniroute:warmup:cb:*
|
||||
REDIS_KEY_PREFIX=omniroute:
|
||||
```
|
||||
|
||||
- **Default:** `omniroute:` (applied when `REDIS_KEY_PREFIX` is unset or blank).
|
||||
- **Applied to:** rate limiter + auth cache (shared `ioredis` client via `keyPrefix`) and the
|
||||
quota store (`KEY_PREFIX = "${REDIS_KEY_PREFIX}quota"`).
|
||||
quota store (`KEY_PREFIX = "${REDIS_KEY_PREFIX}quota"`) and the warmup circuit breaker
|
||||
(`KEY_PREFIX = "${REDIS_KEY_PREFIX}warmup:cb:"`).
|
||||
- **Changing the prefix** when keys already exist in Redis orphans the old keys (they expire
|
||||
via TTL / LRU). Safe to change; no migration needed.
|
||||
via TTL / LRU). Safe to change; no migration needed. The one exception is a warmup
|
||||
circuit-breaker key for a connection marked forbidden: it is persisted without a TTL, so
|
||||
list leftovers with `redis-cli --scan --pattern '<old-prefix>warmup:cb:*'` and delete them.
|
||||
- **ioredis `keyPrefix`** automatically prepends the prefix on writes **and** strips it on reads,
|
||||
so application code never sees the prefix.
|
||||
|
||||
|
||||
@@ -1407,7 +1407,7 @@ Provider quota endpoints, network tunnels (Tailscale, Ngrok, MITM debug proxy),
|
||||
| `APP_BIND_HOST` | `127.0.0.1` | `docker-compose.yml`, `docker-compose.prod.yml` | Host interface docker-compose publishes the app's own dashboard/API/live-WS ports on (#12568). With `REQUIRE_API_KEY=false` shipping as the `.env.example` default, `0.0.0.0` exposes the anonymous `/v1` LLM proxy to the whole LAN/WAN — only widen once `REQUIRE_API_KEY=true` or a reverse proxy in front enforces its own auth. |
|
||||
| `QDRANT_BIND_HOST` | `127.0.0.1` | `docker-compose.yml` | Host interface docker-compose publishes the Qdrant memory sidecar on (#12578). Same LAN-exposure reasoning as `REDIS_BIND_HOST`. |
|
||||
| `BIFROST_BIND_HOST` | `127.0.0.1` | `docker-compose.yml` | Host interface docker-compose publishes the Bifrost router sidecar on (#12578). Same LAN-exposure reasoning as `REDIS_BIND_HOST`. |
|
||||
| `REDIS_KEY_PREFIX` | `omniroute:` | `src/shared/utils/rateLimiter.ts` | Namespace prefix applied to every OmniRoute Redis key (rate limiter, auth cache, quota store). Prevents key collisions when the Redis instance is shared with other apps (#11042). |
|
||||
| `REDIS_KEY_PREFIX` | `omniroute:` | `src/shared/utils/rateLimiter.ts` | Namespace prefix applied to every OmniRoute Redis key (rate limiter, auth cache, quota store, warmup circuit breaker). Prevents key collisions when the Redis instance is shared with other apps (#11042). |
|
||||
| `OMNIROUTE_INTERNAL_SERVICE_TOKEN` | _(unset — mechanism disabled)_ | `src/lib/api/internalServiceAuth.ts` | Shared secret for identity-preserving internal REST hops (#9260): OmniRoute components calling other local OmniRoute routes send it as `x-omniroute-internal-service-token` so the original caller identity is preserved. Compared with `timingSafeEqual`. |
|
||||
| `OMNIROUTE_INTERNAL_SERVICE_TOKEN_FILE` | _(unset)_ | `src/lib/api/internalServiceAuth.ts` | Secret-file variant of the internal service token: path to a file whose trimmed content is the token. Only consulted when the inline var is unset. |
|
||||
| `OPENROUTER_PROVIDER_STATS_ENABLED` | `true` | `src/lib/catalog/openrouterProviderStats.ts` | Enrich the dashboard providers list with OpenRouter weekly ranking stats (#9324). On by default; set `false` to skip the background fetch entirely (non-blocking, never fatal). |
|
||||
|
||||
@@ -18,7 +18,8 @@ type RedisLike = {
|
||||
persist: (key: string) => Promise<unknown>;
|
||||
};
|
||||
|
||||
const KEY_PREFIX = "omniroute:warmup:cb:";
|
||||
// Same namespace as the rate limiter and quota store (REDIS_KEY_PREFIX, #11042).
|
||||
const KEY_PREFIX = `${process.env.REDIS_KEY_PREFIX?.trim() || "omniroute:"}warmup:cb:`;
|
||||
|
||||
export class RedisCircuitBreakerStore implements CircuitBreakerStore {
|
||||
constructor(private redis: RedisLike) {}
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
/**
|
||||
* REDIS_KEY_PREFIX is documented as the namespace for ALL OmniRoute Redis keys, and the rate
|
||||
* limiter, auth cache and quota store honor it. The warmup circuit breaker hardcoded
|
||||
* `omniroute:warmup:cb:`, so on a shared Redis with a custom prefix its keys still landed in
|
||||
* the default namespace (outside a `~tenantA:*` key ACL, and colliding between instances).
|
||||
* Raised as MANDATORY in the #11042 review.
|
||||
*
|
||||
* KEY_PREFIX is read at module load, so each case imports a fresh copy of the store.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
function makeKeyRecordingRedis() {
|
||||
const hashes = new Map<string, Record<string, string>>();
|
||||
return {
|
||||
keys: () => [...hashes.keys()],
|
||||
redis: {
|
||||
async hgetall(key: string) {
|
||||
return hashes.get(key) ?? {};
|
||||
},
|
||||
async hset(key: string, fields: Record<string, string>) {
|
||||
hashes.set(key, { ...(hashes.get(key) ?? {}), ...fields });
|
||||
return 1;
|
||||
},
|
||||
async hget(key: string, field: string) {
|
||||
return hashes.get(key)?.[field] ?? null;
|
||||
},
|
||||
async expire() {
|
||||
return 1;
|
||||
},
|
||||
async persist() {
|
||||
return 1;
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
let importCount = 0;
|
||||
|
||||
async function keysWrittenWith(prefix: string | undefined) {
|
||||
const prev = process.env.REDIS_KEY_PREFIX;
|
||||
if (prefix === undefined) delete process.env.REDIS_KEY_PREFIX;
|
||||
else process.env.REDIS_KEY_PREFIX = prefix;
|
||||
try {
|
||||
const { RedisCircuitBreakerStore } = await import(
|
||||
`../../../../src/lib/warmupScheduler/redisCircuitBreakerStore.ts?case=${importCount++}`
|
||||
);
|
||||
const { keys, redis } = makeKeyRecordingRedis();
|
||||
const store = new RedisCircuitBreakerStore(redis);
|
||||
await store.recordResult("conn-1", { success: false, failureKind: "network" });
|
||||
assert.equal(await store.isInBackoff("conn-1"), true);
|
||||
assert.equal((await store.get("conn-1"))?.streak, 1);
|
||||
return keys();
|
||||
} finally {
|
||||
if (prev === undefined) delete process.env.REDIS_KEY_PREFIX;
|
||||
else process.env.REDIS_KEY_PREFIX = prev;
|
||||
}
|
||||
}
|
||||
|
||||
test("warmup circuit-breaker keys use REDIS_KEY_PREFIX", async () => {
|
||||
assert.deepEqual(await keysWrittenWith("tenantA:"), ["tenantA:warmup:cb:conn-1"]);
|
||||
});
|
||||
|
||||
test("warmup circuit-breaker keys are unchanged when REDIS_KEY_PREFIX is unset or blank", async () => {
|
||||
assert.deepEqual(await keysWrittenWith(undefined), ["omniroute:warmup:cb:conn-1"]);
|
||||
assert.deepEqual(await keysWrittenWith(" "), ["omniroute:warmup:cb:conn-1"]);
|
||||
});
|
||||
Reference in New Issue
Block a user