mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-15 11:43:10 +03:00
fix(sse): gate structural chat admission shedding on real heap pressure
Closes #10183, Closes #10268 3.8.49 (#9654/#9940) replaced the 3.8.48 heap-ratio shed (heapUsed/heapLimit >= 0.75) in chatBodyAdmission.ts with an unconditional CHAT_MAX_HEAVY_IN_FLIGHT=1 structural lease. A second concurrent "structurally heavy" chat request (>=200 messages, >=64 tools, or >=32k estimated tokens — routine for coding-agent fan-out like Hermes/Cursor/Claude Code) was hard-rejected with a retryable HTTP 503 chat_admission_busy/structure_limit regardless of actual heap pressure, even on a host with ample free RAM. Restore the heap-conditional gate as an ADDITIONAL check layered on top of (not a replacement for) the #9654 bounded-concurrency / per-connection-lane protection: when heavyweight capacity is busy, only enter the bounded-wait/shed path when a live heap-pressure probe (heapUsed / v8 heap_size_limit >= OMNIROUTE_CHAT_ADMISSION_HEAP_SHED_RATIO, default 0.75) confirms real pressure. A healthy heap now admits the second heavy request immediately via a no-op lease instead of parking or shedding it. The probe is injectable via admitChatStructure({ heapPressureCheck }) for deterministic tests. Regression tests: - tests/unit/bug-10183-admission-heavy-healthy-heap.test.ts (new, permanent): healthy-heap 2nd heavy request now admitted (was RED); genuinely pressured heap still sheds it. - tests/unit/probe-10268-structural-503.test.ts (promoted to permanent): the exact reported 503 chat_admission_busy shape is still produced under real heap pressure, and the same fan-out is admitted on a healthy heap. - tests/unit/chat-body-admission.test.ts, tests/unit/chat-body-admission-queue.test.ts, tests/unit/per-connection-admission-9654.test.ts updated to inject heapPressureCheck: () => true where they exercise the busy/shed path, preserving #9654/#4380 coverage. Gates run: npm run typecheck:core (clean), eslint --suppressions-location config/quality/eslint-suppressions.json on changed files (clean), scripts/check/check-file-size.mjs (OK), scripts/check/check-test-discovery.mjs (OK), focused admission suite (68/68 passing) and npm run test:unit (in progress at commit time under heavy shared-devbox contention from a 13-way parallel session fan-out; no admission-related failures observed through 1873 lines of output, the sole failure seen was a pre-existing unrelated proxy/search timeout consistent with known load-induced flakiness, not a regression from this change). ⚠️ base-red inherited: #9985 — ESLint errors (2) from #10250
This commit is contained in:
@@ -0,0 +1 @@
|
||||
- fix(sse): gate structural chat admission shedding on real heap pressure instead of unconditional capacity (#10183, #10268)
|
||||
@@ -14,6 +14,7 @@
|
||||
|
||||
import { CORS_HEADERS } from "../utils/cors";
|
||||
import { createHash } from "crypto";
|
||||
import v8 from "node:v8";
|
||||
|
||||
|
||||
const OMNIROUTE_CHAT_VIRTUAL_TTL_MS = parsePositiveInt(
|
||||
@@ -83,6 +84,41 @@ export const CHAT_HEAVY_ESTIMATED_TOKENS = parsePositiveInt(
|
||||
process.env.OMNIROUTE_CHAT_HEAVY_ESTIMATED_TOKENS,
|
||||
32_000
|
||||
);
|
||||
|
||||
/**
|
||||
* Heap-pressure shed ratio for the structural admission gate (#10183, #10268).
|
||||
*
|
||||
* 3.8.48 only shed a heavy request once `heapUsed / heapLimit >= shedRatio` (0.75).
|
||||
* 3.8.49 (#9654/#9940) replaced that heap-conditional shed with an unconditional
|
||||
* `CHAT_MAX_HEAVY_IN_FLIGHT=1` structural lease, so a second concurrent "heavy"
|
||||
* request (coding-agent fan-out is the common trigger) was hard-rejected with a
|
||||
* retryable 503 even on a host with ample free RAM. This restores the heap
|
||||
* condition as an ADDITIONAL gate layered on top of the bounded-concurrency /
|
||||
* per-connection-lane protection from #9654 (that protection stays in force —
|
||||
* this constant only decides whether a *busy* lease is still shed with a 503 or
|
||||
* admitted anyway because the heap has real headroom).
|
||||
*/
|
||||
export const CHAT_ADMISSION_HEAP_SHED_RATIO = (() => {
|
||||
const parsed = Number(process.env.OMNIROUTE_CHAT_ADMISSION_HEAP_SHED_RATIO);
|
||||
return Number.isFinite(parsed) && parsed > 0 && parsed <= 1 ? parsed : 0.75;
|
||||
})();
|
||||
|
||||
/**
|
||||
* Live `heapUsed / heap_size_limit` pressure probe, injectable for deterministic
|
||||
* tests (`admitChatStructure({ heapPressureCheck })`). Defaults to the real V8
|
||||
* heap statistics. Any read failure is treated as "not under pressure" so a
|
||||
* transient stats error never turns into a false structural shed.
|
||||
*/
|
||||
export function defaultHeapPressureCheck(): boolean {
|
||||
try {
|
||||
const heapUsed = process.memoryUsage().heapUsed;
|
||||
const heapLimit = v8.getHeapStatistics().heap_size_limit;
|
||||
if (!Number.isFinite(heapLimit) || heapLimit <= 0) return false;
|
||||
return heapUsed / heapLimit >= CHAT_ADMISSION_HEAP_SHED_RATIO;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
/**
|
||||
* Optional per-deployment history cap. `0` (the default) disables it.
|
||||
*
|
||||
@@ -487,6 +523,12 @@ export async function admitChatStructure(
|
||||
heavyTokens?: number;
|
||||
queueMs?: number;
|
||||
signal?: AbortSignal;
|
||||
/**
|
||||
* Heap-pressure probe consulted only when heavyweight capacity is busy
|
||||
* (#10183, #10268). Defaults to `defaultHeapPressureCheck` (live V8 heap
|
||||
* stats). Tests inject a deterministic override.
|
||||
*/
|
||||
heapPressureCheck?: () => boolean;
|
||||
} = {}
|
||||
): Promise<ChatStructureAdmission> {
|
||||
if (!body || typeof body !== "object" || Array.isArray(body)) return { admit: true, lease };
|
||||
@@ -524,6 +566,26 @@ export async function admitChatStructure(
|
||||
(options.sessionId
|
||||
? perConnectionAdmissionController.getController(options.sessionId)
|
||||
: defaultAdmissionController);
|
||||
|
||||
// Uncontended fast path: capacity is free, no need to consult heap pressure at all.
|
||||
const immediate = controller.tryAcquireHeavy();
|
||||
if (immediate) return { admit: true, lease: immediate };
|
||||
|
||||
// Heavyweight capacity is momentarily busy (a concurrent heavy request holds the
|
||||
// lease). #10183 / #10268: only enter the bounded-wait / shed path — with its
|
||||
// queued-bytes heap valve and abort handling (#9654) — when the heap is
|
||||
// GENUINELY under pressure. This restores the 3.8.48 `heapUsed/heapLimit >=
|
||||
// shedRatio` condition as an additional gate on top of (never a replacement
|
||||
// for) the bounded-concurrency / per-connection-lane protection above. A
|
||||
// healthy heap has real headroom for a second heavy request even while the
|
||||
// single lease is momentarily busy, so admit it immediately instead of
|
||||
// parking/shedding a request that has nothing to do with actual resource
|
||||
// pressure.
|
||||
const heapPressureCheck = options.heapPressureCheck ?? defaultHeapPressureCheck;
|
||||
if (!heapPressureCheck()) {
|
||||
return { admit: true, lease: createNoopLease() };
|
||||
}
|
||||
|
||||
// Structural-only waits happen on byte-light bodies (a byte-heavy body already
|
||||
// holds the byte-stage lease), so the conservative 256KB weight bounds the
|
||||
// parsed JSON the waiter keeps resident while parked.
|
||||
@@ -596,14 +658,18 @@ export function resolveSelfLoopBearer(): string {
|
||||
* gap that kept the Zoo Code / api-key describe call failing even after the byte
|
||||
* stage was bypassed. Release is a no-op; capacity was never reserved.
|
||||
*/
|
||||
const NULL_LEASE: ChatAdmissionLease = {
|
||||
get released() {
|
||||
return true;
|
||||
},
|
||||
release() {
|
||||
// No-op: the sentinel never reserved heavyweight capacity.
|
||||
},
|
||||
};
|
||||
function createNoopLease(): ChatAdmissionLease {
|
||||
return {
|
||||
get released() {
|
||||
return true;
|
||||
},
|
||||
release() {
|
||||
// No-op: this sentinel never reserved heavyweight capacity.
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const NULL_LEASE: ChatAdmissionLease = createNoopLease();
|
||||
|
||||
/**
|
||||
* True when the request is a trusted in-process self-loop sub-request that must
|
||||
|
||||
66
tests/unit/bug-10183-admission-heavy-healthy-heap.test.ts
Normal file
66
tests/unit/bug-10183-admission-heavy-healthy-heap.test.ts
Normal file
@@ -0,0 +1,66 @@
|
||||
// #10183: regression 3.8.48 → 3.8.49 — chat admission rejected a second concurrent
|
||||
// "heavy" request even on a healthy heap. `admitChatStructure`'s CHAT_MAX_HEAVY_IN_FLIGHT=1
|
||||
// cap (#9654/#9940) sheds unconditionally once busy; this test proves shedding must be
|
||||
// gated on real heap pressure (restoring 3.8.48's `heapUsed/heapLimit >= shedRatio`
|
||||
// semantics) instead of firing regardless of free memory.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
ChatAdmissionController,
|
||||
admitChatStructure,
|
||||
} from "../../src/shared/middleware/chatBodyAdmission.ts";
|
||||
|
||||
function heavyBody() {
|
||||
return {
|
||||
messages: Array.from({ length: 200 }, () => ({
|
||||
role: "user",
|
||||
content: "x".repeat(400),
|
||||
})),
|
||||
tools: [] as unknown[],
|
||||
};
|
||||
}
|
||||
|
||||
test("bug-10183: second concurrent heavy request admitted on a healthy heap", async () => {
|
||||
const controller = new ChatAdmissionController(1); // default CHAT_MAX_HEAVY_IN_FLIGHT=1
|
||||
const first = await admitChatStructure(heavyBody(), null, { controller });
|
||||
assert.equal(first.admit, true);
|
||||
assert.ok(first.admit && first.lease, "first heavy request should hold the lease");
|
||||
|
||||
try {
|
||||
const second = await admitChatStructure(heavyBody(), null, {
|
||||
controller,
|
||||
queueMs: 50,
|
||||
// No override: default heap probe reads live process stats, which are
|
||||
// healthy in the test process — proves the fix without mocking away the
|
||||
// real check.
|
||||
});
|
||||
assert.equal(second.admit, true, "healthy heap must not shed a 2nd heavy request");
|
||||
if (second.admit) second.lease?.release();
|
||||
} finally {
|
||||
if (first.admit) first.lease?.release();
|
||||
}
|
||||
});
|
||||
|
||||
test("bug-10183: a genuinely pressured heap still sheds the 2nd heavy request", async () => {
|
||||
const controller = new ChatAdmissionController(1);
|
||||
const first = await admitChatStructure(heavyBody(), null, { controller });
|
||||
assert.equal(first.admit, true);
|
||||
assert.ok(first.admit && first.lease);
|
||||
|
||||
try {
|
||||
const second = await admitChatStructure(heavyBody(), null, {
|
||||
controller,
|
||||
queueMs: 0,
|
||||
heapPressureCheck: () => true, // simulate real heap pressure
|
||||
});
|
||||
assert.equal(second.admit, false, "real heap pressure must still shed the 2nd request");
|
||||
if (!second.admit) {
|
||||
assert.equal(second.response.status, 503);
|
||||
const payload = await second.response.json();
|
||||
assert.equal(payload.error.code, "chat_admission_busy");
|
||||
assert.equal(payload.error.reason, "structure_limit");
|
||||
}
|
||||
} finally {
|
||||
if (first.admit) first.lease?.release();
|
||||
}
|
||||
});
|
||||
@@ -43,6 +43,9 @@ test("a heavy structural request waits for capacity instead of failing immediate
|
||||
heavyTools: 10,
|
||||
heavyTokens: 10_000,
|
||||
queueMs: 500,
|
||||
// #10183/#10268: entry into the bounded-wait path requires real heap
|
||||
// pressure now; force it so this test still exercises the wait.
|
||||
heapPressureCheck: () => true,
|
||||
}
|
||||
);
|
||||
|
||||
@@ -85,6 +88,9 @@ test("waiting for admission times out into a retryable 503", async () => {
|
||||
heavyTools: 10,
|
||||
heavyTokens: 10_000,
|
||||
queueMs: 50,
|
||||
// #10183/#10268: entry into the bounded-wait/shed path requires real
|
||||
// heap pressure now; force it to still exercise the timeout.
|
||||
heapPressureCheck: () => true,
|
||||
}
|
||||
);
|
||||
|
||||
@@ -148,6 +154,9 @@ test("expired admission queue keeps the legacy immediate 503 behaviour", async (
|
||||
heavyTools: 10,
|
||||
heavyTokens: 10_000,
|
||||
queueMs: 0,
|
||||
// #10183/#10268: shedding now requires real heap pressure; force it to
|
||||
// still exercise the legacy immediate-reject path.
|
||||
heapPressureCheck: () => true,
|
||||
}
|
||||
);
|
||||
|
||||
@@ -174,6 +183,9 @@ test("admission waiters are served FIFO as capacity frees", async () => {
|
||||
heavyTools: 10,
|
||||
heavyTokens: 10_000,
|
||||
queueMs: 500,
|
||||
// #10183/#10268: entry into the bounded-wait path requires real heap
|
||||
// pressure now; force it so both waiters still queue.
|
||||
heapPressureCheck: () => true,
|
||||
};
|
||||
const first = admitChatStructure(body, null, options);
|
||||
const second = admitChatStructure(body, null, options);
|
||||
@@ -367,6 +379,9 @@ test("structural admission enforces the queued-bytes cap end-to-end", async () =
|
||||
heavyTools: 10,
|
||||
heavyTokens: 10_000,
|
||||
queueMs: 2_000,
|
||||
// #10183/#10268: entry into the bounded-wait path requires real heap
|
||||
// pressure now; force it so the queued-bytes cap is still exercised.
|
||||
heapPressureCheck: () => true,
|
||||
};
|
||||
|
||||
// First structural wait parks, charging the conservative 256KB weight.
|
||||
@@ -485,6 +500,9 @@ test("aborting the signal cancels a structural queue-wait", async () => {
|
||||
heavyTokens: 10_000,
|
||||
queueMs: 2_000,
|
||||
signal: abortController.signal,
|
||||
// #10183/#10268: entry into the bounded-wait path requires real heap
|
||||
// pressure now; force it so the abort is still exercised mid-wait.
|
||||
heapPressureCheck: () => true,
|
||||
}
|
||||
);
|
||||
|
||||
|
||||
@@ -80,7 +80,7 @@ test("a byte-light request above the message threshold acquires heavyweight capa
|
||||
assert.equal(controller.activeHeavy, 0);
|
||||
});
|
||||
|
||||
test("a byte-light request above the tool threshold is rejected when heavy capacity is busy", async () => {
|
||||
test("a byte-light request above the tool threshold is rejected when heavy capacity is busy AND the heap is genuinely under pressure (#10183/#10268)", async () => {
|
||||
const controller = new ChatAdmissionController(1);
|
||||
const occupied = controller.tryAcquireHeavy();
|
||||
assert.ok(occupied);
|
||||
@@ -88,7 +88,16 @@ test("a byte-light request above the tool threshold is rejected when heavy capac
|
||||
const result = await admitChatStructure(
|
||||
{ messages: [], tools: [{ type: "function" }, { type: "function" }] },
|
||||
null,
|
||||
{ controller, maxMessages: 10, heavyMessages: 10, heavyTools: 2, heavyTokens: 10_000 }
|
||||
{
|
||||
controller,
|
||||
maxMessages: 10,
|
||||
heavyMessages: 10,
|
||||
heavyTools: 2,
|
||||
heavyTokens: 10_000,
|
||||
// #10183/#10268: shedding is now conditional on real heap pressure, not
|
||||
// capacity alone — simulate the pressured case this test targets.
|
||||
heapPressureCheck: () => true,
|
||||
}
|
||||
);
|
||||
|
||||
assert.equal(result.admit, false);
|
||||
@@ -135,7 +144,7 @@ test("no history cap is enforced by default; long conversations are admitted", a
|
||||
result.lease?.release();
|
||||
});
|
||||
|
||||
test("an uncapped oversized conversation still yields to occupied heavyweight capacity", async () => {
|
||||
test("an uncapped oversized conversation still yields to occupied heavyweight capacity when the heap is genuinely under pressure (#10183/#10268)", async () => {
|
||||
const controller = new ChatAdmissionController(1);
|
||||
const occupied = controller.tryAcquireHeavy();
|
||||
assert.ok(occupied);
|
||||
@@ -143,7 +152,15 @@ test("an uncapped oversized conversation still yields to occupied heavyweight ca
|
||||
const result = await admitChatStructure(
|
||||
{ messages: Array.from({ length: 5_000 }, () => ({ role: "user", content: "x" })) },
|
||||
null,
|
||||
{ controller, maxMessages: 0, heavyMessages: 200, heavyTools: 64, heavyTokens: 32_000 }
|
||||
{
|
||||
controller,
|
||||
maxMessages: 0,
|
||||
heavyMessages: 200,
|
||||
heavyTools: 64,
|
||||
heavyTokens: 32_000,
|
||||
// #10183/#10268: shedding is now conditional on real heap pressure.
|
||||
heapPressureCheck: () => true,
|
||||
}
|
||||
);
|
||||
|
||||
assert.equal(result.admit, false);
|
||||
|
||||
@@ -149,7 +149,7 @@ test("admitChatRequest with explicit controller overrides per-connection lookup"
|
||||
if (result.admit) result.lease?.release();
|
||||
});
|
||||
|
||||
test("admitChatStructure routes structural rejection to per-connection controller", async () => {
|
||||
test("admitChatStructure routes structural rejection to per-connection controller when heap pressure is genuinely high (#10183/#10268)", async () => {
|
||||
// occupy sess-a's per-connection controller via the module-level instance
|
||||
const controller = perConnectionAdmissionController.getController("sess-a");
|
||||
const occupied = controller.tryAcquireHeavy();
|
||||
@@ -166,6 +166,8 @@ test("admitChatStructure routes structural rejection to per-connection controlle
|
||||
heavyMessages: 1,
|
||||
heavyTools: 10,
|
||||
heavyTokens: 10_000,
|
||||
// #10183/#10268: shedding is now conditional on real heap pressure.
|
||||
heapPressureCheck: () => true,
|
||||
}
|
||||
);
|
||||
// Session A is busy → 503
|
||||
|
||||
70
tests/unit/probe-10268-structural-503.test.ts
Normal file
70
tests/unit/probe-10268-structural-503.test.ts
Normal file
@@ -0,0 +1,70 @@
|
||||
// #10268: "[BUG] API call failed (attempt 1/3): InternalServerError [HTTP 503]" — Hermes
|
||||
// Agent / Cursor coding-agent fan-out landed on the same structural admission gate as
|
||||
// #10183 and burned its 3 retries on OmniRoute's own `chat_admission_busy` 503, which it
|
||||
// misread as an upstream capacity error. Same root cause, same fix (heap-conditional
|
||||
// shedding in `admitChatStructure`): this test is the permanent regression guard proving
|
||||
// the exact reported 503 shape is still produced when heap pressure is GENUINELY high,
|
||||
// so the #4380 heap-amplification shed path is preserved rather than removed outright.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
admitChatStructure,
|
||||
ChatAdmissionController,
|
||||
type ChatAdmissionLease,
|
||||
} from "../../src/shared/middleware/chatBodyAdmission.ts";
|
||||
|
||||
function heavyBody() {
|
||||
const messages = Array.from({ length: 201 }, (_, i) => ({ role: "user", content: `prompt ${i}` }));
|
||||
const tools = Array.from({ length: 32 }, (_, i) => ({
|
||||
type: "function",
|
||||
function: { name: `tool_${i}`, description: "a".repeat(64), parameters: { type: "object" } },
|
||||
}));
|
||||
return { model: "grok-4.5-fast-high", messages, tools, stream: true };
|
||||
}
|
||||
|
||||
test("#10268: 2nd structurally-heavy agent request is rejected 503 (chat_admission_busy) under real heap pressure", async () => {
|
||||
const controller = new ChatAdmissionController(1);
|
||||
const first = await admitChatStructure(heavyBody(), null, { controller, queueMs: 0 });
|
||||
assert.equal(first.admit, true);
|
||||
const lease = (first as { admit: true; lease: ChatAdmissionLease | null }).lease;
|
||||
assert.ok(lease);
|
||||
try {
|
||||
const second = await admitChatStructure(heavyBody(), null, {
|
||||
controller,
|
||||
queueMs: 0,
|
||||
// Simulate genuine heap pressure (#10183/#10268 fix: shedding is now
|
||||
// conditional on this, not unconditional on capacity alone).
|
||||
heapPressureCheck: () => true,
|
||||
});
|
||||
assert.equal(second.admit, false); // reported failure path, still reachable under real pressure
|
||||
const res = (second as { admit: false; response: Response }).response;
|
||||
assert.equal(res.status, 503); // client is shown HTTP 503
|
||||
const body = await res.json();
|
||||
assert.equal(body.error?.message, "Structurally heavy chat request capacity is busy; retry shortly.");
|
||||
assert.equal(body.error?.code, "chat_admission_busy");
|
||||
assert.equal(body.error?.reason, "structure_limit");
|
||||
} finally {
|
||||
lease.release();
|
||||
}
|
||||
});
|
||||
|
||||
test("#10268: 2nd structurally-heavy agent request is admitted on a healthy heap (the fix)", async () => {
|
||||
const controller = new ChatAdmissionController(1);
|
||||
const first = await admitChatStructure(heavyBody(), null, { controller, queueMs: 0 });
|
||||
assert.equal(first.admit, true);
|
||||
const lease = (first as { admit: true; lease: ChatAdmissionLease | null }).lease;
|
||||
assert.ok(lease);
|
||||
try {
|
||||
const second = await admitChatStructure(heavyBody(), null, {
|
||||
controller,
|
||||
queueMs: 0,
|
||||
// No override: default heap probe reads live process stats (healthy here),
|
||||
// reproducing legitimate Hermes/Cursor fan-out traffic that must no longer
|
||||
// be shed on ample free RAM.
|
||||
});
|
||||
assert.equal(second.admit, true, "healthy heap must admit legitimate agent fan-out");
|
||||
if (second.admit) second.lease?.release();
|
||||
} finally {
|
||||
lease.release();
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user