Repository navigation
fix(sse): auto-calibrate heap-pressure threshold to the V8 heap ceiling #3052
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import v8 from "node:v8"; | ||
|
|
||
| /** | ||
| * Compute the V8 heap-pressure shed threshold (MB). | ||
| * | ||
| * The chat pipeline rejects new requests with a 503 once `heapUsed` exceeds this | ||
| * value, to avoid hard "JavaScript heap out of memory" crashes under concurrent | ||
| * large-context load. The threshold is derived from the process's *actual* V8 | ||
| * heap ceiling (`heap_size_limit`, which reflects `--max-old-space-size` when set, | ||
| * otherwise Node's RAM-derived default) so it auto-adapts across 1 GB / 2 GB / | ||
| * large VPS instead of using a fixed number. | ||
| * | ||
| * A fixed default was the bug: 200 MB sat *below* the app's ~260 MB working set, | ||
| * so the guard rejected every request once the heap warmed up (the v3.8.8 | ||
| * "resource pressure" outage). We shed at 85% of the ceiling — leaving headroom | ||
| * for in-flight requests + GC — with a floor that always clears the runtime | ||
| * baseline so a small/undersized heap never rejects all traffic. | ||
| * | ||
| * @param heapSizeLimitMb `v8.getHeapStatistics().heap_size_limit` expressed in MB | ||
| * @param override `HEAP_PRESSURE_THRESHOLD_MB` env value — wins when it | ||
| * parses to a positive number; invalid/unset → auto-calibrate | ||
| */ | ||
| export function computeHeapPressureThresholdMb( | ||
| heapSizeLimitMb: number, | ||
| override?: string | number | null | ||
| ): number { | ||
| const explicit = Number(override); | ||
| if (Number.isFinite(explicit) && explicit > 0) return Math.floor(explicit); | ||
| // Shed at 85% of the heap ceiling, but never below a floor that clears the | ||
| // runtime's own ~260 MB baseline (+ margin) so an undersized heap degrades to | ||
| // "guard never fires" rather than "guard rejects everything". | ||
| const SHED_RATIO = 0.85; | ||
| const FLOOR_MB = 400; | ||
| return Math.max(Math.round(heapSizeLimitMb * SHED_RATIO), FLOOR_MB); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For supported low-memory settings, this floor puts the shed threshold above the process's actual V8 ceiling: the launchers accept Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| /** | ||
| * Heap-pressure threshold (MB) resolved once at module load from the live V8 heap | ||
| * ceiling. Read by `open-sse/handlers/chatCore.ts`'s memory-pressure guard. | ||
| */ | ||
| export const HEAP_PRESSURE_THRESHOLD_MB = computeHeapPressureThresholdMb( | ||
| v8.getHeapStatistics().heap_size_limit / (1024 * 1024), | ||
| process.env.HEAP_PRESSURE_THRESHOLD_MB | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import { describe, it } from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
| import { computeHeapPressureThresholdMb } from "../../open-sse/utils/heapPressure.ts"; | ||
|
|
||
| // Regression guard for the v3.8.8 "Service temporarily unavailable due to resource | ||
| // pressure" outage: a fixed 200MB threshold sat below the app's ~260MB working set, | ||
| // so the chatCore heap guard rejected every request. The threshold must now track | ||
| // the real V8 heap ceiling so it stays above the baseline on every VPS tier. | ||
| describe("computeHeapPressureThresholdMb", () => { | ||
| it("sheds at 85% of a no-cap heap ceiling (2240MB → 1904)", () => { | ||
| assert.equal(computeHeapPressureThresholdMb(2240), 1904); | ||
| }); | ||
|
|
||
| it("tracks the default 512MB cap (heap_limit ~704 → 598), clearing the ~260MB baseline", () => { | ||
| const t = computeHeapPressureThresholdMb(704); | ||
| assert.equal(t, 598); | ||
| assert.ok(t > 260, "threshold must stay above the ~260MB app baseline"); | ||
| }); | ||
|
|
||
| it("tracks a 1 GB-box 640MB cap (heap_limit 832 → 707)", () => { | ||
| assert.equal(computeHeapPressureThresholdMb(832), 707); | ||
| }); | ||
|
|
||
| it("tracks a 2 GB-box 1536MB cap (heap_limit 1728 → 1469)", () => { | ||
| assert.equal(computeHeapPressureThresholdMb(1728), 1469); | ||
| }); | ||
|
|
||
| it("floors at 400MB so a tiny/undersized heap never rejects all traffic", () => { | ||
| // 85% of 300 = 255, which would sit below the baseline — the floor wins. | ||
| assert.equal(computeHeapPressureThresholdMb(300), 400); | ||
| }); | ||
|
|
||
| it("honors a positive explicit override (string or number)", () => { | ||
| assert.equal(computeHeapPressureThresholdMb(2240, "1024"), 1024); | ||
| assert.equal(computeHeapPressureThresholdMb(2240, 800), 800); | ||
| assert.equal(computeHeapPressureThresholdMb(2240, "1500.9"), 1500); | ||
| }); | ||
|
|
||
| it("ignores invalid/zero/negative overrides and auto-calibrates", () => { | ||
| for (const bad of ["", "0", "-5", "abc", null, undefined]) { | ||
| assert.equal( | ||
| computeHeapPressureThresholdMb(2240, bad as string | number | null | undefined), | ||
| 1904, | ||
| `override ${JSON.stringify(bad)} must fall back to auto-calibration` | ||
| ); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
Number(override)to parse the environment variable is less robust thanparseIntand changes the previous behavior inchatCore.ts(which usedparseInt). Specifically,Numberwill fail (returningNaNand falling back to auto-calibration) on values with common unit suffixes like"400MB"or"512mb". UsingparseIntwith a fallback totypeof override === 'number'maintains backward compatibility and gracefully handles trailing non-numeric characters.