mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-10-19 05:20:05 +00:00
fix(chat-admission): derive the queue wait from the turn it must bridge (#13676)
The chat-admission queue wait is derived from the turn it must bridge (RATE_LIMIT_MAX_WAIT_MS), with the end-to-end aux-call test from #13675 carried with credit. Boarded on the release/v3.8.51 tip (3a2678f) with the b3 release-drain batch (26 PRs): typecheck:core and the open-sse typecheck are clean, ESLint is clean on every changed file, migration numbering OK, and the focused tests of the whole board pass 611/611 (51 files). File-size ceiling growth is reconciled in the wave follow-up. Thank you @abhisheksharma2411!
This commit is contained in:
parent
b5720e6420
commit
5ad9ec0442
6 changed files with 84 additions and 10 deletions
12
.env.example
12
.env.example
|
|
@ -502,8 +502,10 @@ ALLOW_API_KEY_REVEAL=false
|
|||
# DISABLE_CONTEXT_WINDOW_CHECKS=false
|
||||
# How long a heavy request waits for heavyweight capacity before a retryable 503.
|
||||
# A short bounded wait serializes agent bursts instead of an instant 503; 0 = instant.
|
||||
# Default 2000 (2s).
|
||||
# OMNIROUTE_CHAT_ADMISSION_QUEUE_MS=2000
|
||||
# Defaults to RATE_LIMIT_MAX_WAIT_MS (see that var elsewhere in this file for its
|
||||
# current value), the same budget the resilience layer gives the upstream turn that
|
||||
# holds the slot — a shorter wait cannot bridge it.
|
||||
# OMNIROUTE_CHAT_ADMISSION_QUEUE_MS=20000 # example override; default tracks RATE_LIMIT_MAX_WAIT_MS
|
||||
# Queued-bytes budget for the admission wait: bounds total buffered body bytes parked
|
||||
# per lane so the wait cannot amplify the heap (#4380). Over-budget waits 503 immediately.
|
||||
# Default 4194304 (4 MB).
|
||||
|
|
@ -3390,8 +3392,10 @@ QUOTA_STORE_DRIVER=sqlite
|
|||
|
||||
# Heavy chat admission queue wait before returning retryable 503. Set 0 for the
|
||||
# legacy immediate rejection. Used by: src/shared/middleware/chatBodyAdmission.ts.
|
||||
# Default: 5000 (5 seconds)
|
||||
# OMNIROUTE_CHAT_ADMISSION_QUEUE_MS=5000
|
||||
# Documented once above, under the chat admission block — this entry said 5000
|
||||
# while the code said 2000, so it is a pointer now rather than a second answer.
|
||||
# Default: RATE_LIMIT_MAX_WAIT_MS (see that var elsewhere in this file for its
|
||||
# current value)
|
||||
|
||||
# Timeout for /api/jobs/:id/run-now while it waits for an in-flight run.
|
||||
# Used by: src/app/api/jobs/[id]/run-now/route.ts. Default: 30000 (30 seconds)
|
||||
|
|
|
|||
1
changelog.d/fixes/13676-chat-admission-queue-wait.md
Normal file
1
changelog.d/fixes/13676-chat-admission-queue-wait.md
Normal file
|
|
@ -0,0 +1 @@
|
|||
- **fix(chat-admission):** derive the heavyweight queue wait from `requestQueue.maxWaitMs` (`RATE_LIMIT_MAX_WAIT_MS`, 15 s) instead of a fixed 2 s — the single default heavyweight slot is held for the whole upstream turn, so a 2 s wait expired inside every occupancy it exists to bridge and a lone large-context session shed its own next request (`reason:"queue_timeout", activeHeavy:1, waiting:0`) with nothing to protect against; an explicit `OMNIROUTE_CHAT_ADMISSION_QUEUE_MS` still wins, `0` included, and `.env.example` no longer documents the variable twice with different defaults ([#13648](https://github.com/diegosouzapw/OmniRoute/issues/13648)) (#13676 — thanks @pacocartones)
|
||||
|
|
@ -46,7 +46,7 @@ complementary; operators should know which one they are looking at.
|
|||
- **Tuning:**
|
||||
- `OMNIROUTE_CHAT_MAX_INFLIGHT_BYTES` — override for the auto-derived byte budget
|
||||
- `OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT` — legacy request-count cap, opt-in only
|
||||
- `OMNIROUTE_CHAT_ADMISSION_QUEUE_MS` — queue-wait before 503 (default 2000)
|
||||
- `OMNIROUTE_CHAT_ADMISSION_QUEUE_MS` — queue-wait before 503 (defaults to `RATE_LIMIT_MAX_WAIT_MS`)
|
||||
- `OMNIROUTE_CHAT_ADMISSION_MAX_QUEUED_BYTES` — queued-bytes heap valve (default 4 MB)
|
||||
- `OMNIROUTE_CHAT_VIRTUAL_TTL_MS` / `OMNIROUTE_CHAT_VIRTUAL_MAX_SESSIONS` — deprecated
|
||||
no-ops since #10110 (accepted for config compatibility, ignored)
|
||||
|
|
|
|||
|
|
@ -54,7 +54,7 @@ Common problems and solutions for OmniRoute.
|
|||
```bash
|
||||
export OMNIROUTE_ROTATE_ON_400=true # hop to another model/provider on 400/401 (skips broken passthrough models)
|
||||
export OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT=4 # explicit heavyweight admission ceiling (unset by default: no request-count cap, see note below)
|
||||
export OMNIROUTE_CHAT_ADMISSION_QUEUE_MS=5000 # longer bounded wait for heavyweight capacity instead of an immediate retryable 503
|
||||
export OMNIROUTE_CHAT_ADMISSION_QUEUE_MS=20000 # raise the bounded wait past the RATE_LIMIT_MAX_WAIT_MS default for slow upstreams
|
||||
```
|
||||
|
||||
Set these in the OmniRoute process environment (the daemon, e.g. via the LaunchAgent plist or `systemctl edit`), then restart OmniRoute. The rotation flag is the single highest-leverage lever: it converts a hard failure into a transparent retry against a healthy provider in the pool.
|
||||
|
|
@ -615,7 +615,7 @@ heavy request arrived at once. The old count cap (`OMNIROUTE_CHAT_MAX_HEAVY_IN_F
|
|||
still honored, but only if you explicitly set it.
|
||||
|
||||
When capacity is busy, a heavyweight request first waits up to
|
||||
`OMNIROUTE_CHAT_ADMISSION_QUEUE_MS` (default `2000`, `0` disables the wait) for a slot to free up
|
||||
`OMNIROUTE_CHAT_ADMISSION_QUEUE_MS` (defaults to `RATE_LIMIT_MAX_WAIT_MS`; `0` disables the wait) for a slot to free up
|
||||
before answering the retryable `503`. The bounded wait exists so agent-style clients
|
||||
(OpenCode, Claude Code, Cursor) that fan out heavy sub-requests concurrently serialize the burst
|
||||
instead of burning their whole retry budget on immediate rejections and dying mid-task.
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@
|
|||
*/
|
||||
|
||||
import { createLogger } from "../utils/logger";
|
||||
import { DEFAULT_REQUEST_QUEUE_MAX_WAIT_MS } from "../../lib/resilience/settings";
|
||||
import v8 from "node:v8";
|
||||
import { trackRequest } from "../../lib/gracefulShutdown";
|
||||
import { resolveIngestByteBudget, type IngestBudgetSource } from "./admissionBudget";
|
||||
|
|
@ -74,10 +75,20 @@ export const CHAT_MAX_HEAVY_IN_FLIGHT = parsePositiveInt(
|
|||
* that routinely land on the admission gate together; an immediate 503 makes the
|
||||
* client burn its retry budget in seconds and the agent dies mid-task. A short
|
||||
* bounded wait serializes the burst instead. `0` (legacy) rejects immediately.
|
||||
*
|
||||
* The default tracks `requestQueue.maxWaitMs` rather than carrying a number of its
|
||||
* own, because a wait shorter than the occupancy it must bridge cannot serialize
|
||||
* anything. The single default heavyweight slot is held for the whole upstream
|
||||
* turn, which the resilience layer already budgets at that value (15s), so the old
|
||||
* fixed 2s expired inside every one of them: a single large-context session shed
|
||||
* itself with `reason:"queue_timeout", activeHeavy:1, waiting:0` — one holder, no
|
||||
* contention, nothing for the 503 to protect against (#13648). Tying the two
|
||||
* together also means an operator who raises `RATE_LIMIT_MAX_WAIT_MS` for slow
|
||||
* upstreams does not have to discover this second knob to keep the gate coherent.
|
||||
*/
|
||||
export const CHAT_ADMISSION_QUEUE_MAX_MS = parseNonNegativeInt(
|
||||
process.env.OMNIROUTE_CHAT_ADMISSION_QUEUE_MS,
|
||||
2000
|
||||
DEFAULT_REQUEST_QUEUE_MAX_WAIT_MS
|
||||
);
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ const {
|
|||
CHAT_ADMISSION_MAX_QUEUED_BYTES,
|
||||
CHAT_LARGE_BODY_BYTES,
|
||||
} = admissionModule;
|
||||
const { DEFAULT_REQUEST_QUEUE_MAX_WAIT_MS } = await import("../../src/lib/resilience/settings.ts");
|
||||
|
||||
function chatRequest(body: string, contentLength: string | null = String(body.length)): Request {
|
||||
const headers: Record<string, string> = { "content-type": "application/json" };
|
||||
|
|
@ -418,9 +419,17 @@ test("structural admission enforces the queued-bytes cap end-to-end", async () =
|
|||
assert.equal(controller.activeHeavy, 0);
|
||||
});
|
||||
|
||||
test("queue-wait defaults are bounded (2s wait, 4MB queued-bytes budget)", () => {
|
||||
test("queue-wait defaults are bounded, and the wait tracks the turn it must bridge", () => {
|
||||
if (process.env.OMNIROUTE_CHAT_ADMISSION_QUEUE_MS === undefined) {
|
||||
assert.equal(CHAT_ADMISSION_QUEUE_MAX_MS, 2_000);
|
||||
// Was a fixed 2_000. A wait shorter than the occupancy it bridges cannot
|
||||
// serialize a burst — it just 503s the holder's own next request (#13648).
|
||||
// Asserted against the constant rather than 15_000 so the two cannot drift:
|
||||
// an operator who raises RATE_LIMIT_MAX_WAIT_MS moves both together.
|
||||
assert.equal(CHAT_ADMISSION_QUEUE_MAX_MS, DEFAULT_REQUEST_QUEUE_MAX_WAIT_MS);
|
||||
assert.ok(
|
||||
CHAT_ADMISSION_QUEUE_MAX_MS >= 15_000,
|
||||
"the shipped default must be able to bridge a default heavyweight turn"
|
||||
);
|
||||
}
|
||||
if (process.env.OMNIROUTE_CHAT_ADMISSION_MAX_QUEUED_BYTES === undefined) {
|
||||
assert.equal(CHAT_ADMISSION_MAX_QUEUED_BYTES, 4 * 1024 * 1024);
|
||||
|
|
@ -546,3 +555,52 @@ test("aborting the signal cancels a structural queue-wait", async () => {
|
|||
held.release();
|
||||
assert.equal(controller.activeHeavy, 0);
|
||||
});
|
||||
|
||||
// Ported from #13675 (Co-authored-by: oyi77), with queueMs derived from
|
||||
// DEFAULT_REQUEST_QUEUE_MAX_WAIT_MS instead of a hardcoded 15_000 so the
|
||||
// end-to-end proof cannot drift out of sync with the resilience layer's
|
||||
// default the way the doc references in #13676 briefly did.
|
||||
test("#13648: a resilience-scale occupancy bridges the default queue instead of self-shedding", async () => {
|
||||
// The reported failure: a single large-context agent holds the one heavyweight
|
||||
// slot for the resilience layer's default turn budget while its own aux call
|
||||
// arrives — with the old fixed 2s default the aux call always shed
|
||||
// `queue_timeout` and the session died. The default queue must bridge a full
|
||||
// resilience-scale occupancy, whatever that default currently is.
|
||||
const controller = new ChatAdmissionController(1);
|
||||
const held = controller.tryAcquireHeavy();
|
||||
assert.ok(held);
|
||||
|
||||
const queueMs = DEFAULT_REQUEST_QUEUE_MAX_WAIT_MS;
|
||||
const pending = admitChatStructure(
|
||||
{
|
||||
messages: [
|
||||
{ role: "user", content: "one" },
|
||||
{ role: "user", content: "two" },
|
||||
],
|
||||
},
|
||||
null,
|
||||
{
|
||||
controller,
|
||||
maxMessages: 10,
|
||||
heavyMessages: 2,
|
||||
heavyTools: 10,
|
||||
heavyTokens: 10_000,
|
||||
queueMs,
|
||||
heapPressureCheck: () => true,
|
||||
}
|
||||
);
|
||||
|
||||
let settled = false;
|
||||
void pending.then(() => {
|
||||
settled = true;
|
||||
});
|
||||
// Old default (2s) would have shed by now; the bridged wait must still park.
|
||||
await new Promise((resolve) => setTimeout(resolve, 2_500));
|
||||
assert.equal(settled, false, "aux call must still wait past the old 2s shed point");
|
||||
|
||||
held.release();
|
||||
const result = await pending;
|
||||
assert.equal(result.admit, true, "aux call acquires the freed slot instead of self-shedding");
|
||||
if (result.admit) result.lease?.release();
|
||||
assert.equal(controller.activeHeavy, 0);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue