mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-21 22:52:19 +03:00
fix(docker): warn when OMNIROUTE_MEMORY_MB disagrees with NODE_OPTIONS heap (#10818)
Merged — validated together with a batch of related RaviTharuma PRs in one combined worktree (typecheck:core clean, complexity/file-size/changelog gates green, focused tests passing). Thanks for the contribution!
This commit is contained in:
1
changelog.d/fixes/10353-memory-heap-conflict-warn.md
Normal file
1
changelog.d/fixes/10353-memory-heap-conflict-warn.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(docker):** warn at boot when `OMNIROUTE_MEMORY_MB` disagrees with `NODE_OPTIONS --max-old-space-size`, and document that the standalone/Docker launcher appends `OMNIROUTE_MEMORY_MB` last ([#10353](https://github.com/diegosouzapw/OmniRoute/issues/10353))
|
||||
@@ -843,7 +843,7 @@ The logging system writes to both stdout and rotated log files. All configuratio
|
||||
|
||||
| Variable | Default | Description |
|
||||
| -------------------------- | ------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `OMNIROUTE_MEMORY_MB` | _auto_ | Runtime V8 heap limit (MB). When unset, calibrated dynamically (~35% of system RAM, clamped to `[512, 4096]`); `512` is only the floor when total memory can't be read. Set explicitly to override. Docker standalone and `omniroute serve` use it to set `--max-old-space-size`. |
|
||||
| `OMNIROUTE_MEMORY_MB` | _auto_ | **Recommended** Docker/standalone V8 heap limit (MB). When unset, calibrated dynamically (~35% of system RAM, clamped to `[512, 4096]`); `512` is only the floor when total memory can't be read. On `run-standalone.mjs` (Docker CMD), an **explicit** value is appended as `--max-old-space-size` and **wins** over a conflicting NODE_OPTIONS heap flag (V8 last-flag). `omniroute serve` still prefers an existing NODE_OPTIONS heap (#5238). Do not set both to different numbers — the process logs a warn naming both values and the winner. |
|
||||
| `PROMPT_CACHE_MAX_SIZE` | `50` | Max cached system prompt entries. |
|
||||
| `PROMPT_CACHE_MAX_BYTES` | `2097152` (2 MB) | Max total prompt cache size. |
|
||||
| `PROMPT_CACHE_TTL_MS` | `300000` (5 min) | Prompt cache entry TTL. |
|
||||
|
||||
@@ -49,6 +49,59 @@ export function envHasExplicitHeapFlag(env) {
|
||||
return String(sourceEnv?.NODE_OPTIONS || "").includes(MAX_OLD_SPACE_FLAG);
|
||||
}
|
||||
|
||||
/** Last `--max-old-space-size=` value in NODE_OPTIONS, or null if absent. */
|
||||
export function parseNodeOptionsHeapMb(nodeOptions) {
|
||||
const matches = [...String(nodeOptions || "").matchAll(/--max-old-space-size=(\d+)/g)];
|
||||
if (matches.length === 0) return null;
|
||||
const parsed = Number.parseInt(matches[matches.length - 1][1], 10);
|
||||
return Number.isFinite(parsed) ? parsed : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when OMNIROUTE_MEMORY_MB is an explicit in-range integer (not the
|
||||
* unset/invalid fallback). Docker images set this; Compose may also set
|
||||
* NODE_OPTIONS — #10353 needs to know both knobs were intentionally present.
|
||||
*/
|
||||
export function envHasExplicitOmnirouteMemoryMb(env) {
|
||||
const sourceEnv = arguments.length === 0 ? process.env : env;
|
||||
const parsed = Number.parseInt(String(sourceEnv?.OMNIROUTE_MEMORY_MB ?? ""), 10);
|
||||
return Number.isFinite(parsed) && parsed >= 64 && parsed <= 16384;
|
||||
}
|
||||
|
||||
/**
|
||||
* Docker `run-standalone.mjs` appends `--max-old-space-size` from
|
||||
* OMNIROUTE_MEMORY_MB. V8 last-flag semantics mean that appended value wins
|
||||
* over an earlier NODE_OPTIONS heap. Warn once when both are set and disagree
|
||||
* so env dumps stop looking like NODE_OPTIONS is in effect (#10353).
|
||||
*
|
||||
* @returns {boolean} true when a warn was emitted
|
||||
*/
|
||||
export function warnConflictingHeapLimits(env, omnirouteMb, log = console.warn) {
|
||||
const nodeMb = parseNodeOptionsHeapMb(env?.NODE_OPTIONS);
|
||||
if (nodeMb == null || !envHasExplicitOmnirouteMemoryMb(env)) return false;
|
||||
if (nodeMb === omnirouteMb) return false;
|
||||
log(
|
||||
`[omniroute] heap limit conflict: OMNIROUTE_MEMORY_MB=${omnirouteMb} disagrees with NODE_OPTIONS --max-old-space-size=${nodeMb}. ` +
|
||||
`run-standalone.mjs / Docker appends OMNIROUTE_MEMORY_MB last, so the effective V8 heap is ${omnirouteMb} MB. ` +
|
||||
`Set only OMNIROUTE_MEMORY_MB (recommended) or make both values match.`
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* NODE_OPTIONS string for Docker / run-standalone.mjs.
|
||||
* Explicit OMNIROUTE_MEMORY_MB always appends (wins). Otherwise keep an
|
||||
* existing NODE_OPTIONS heap flag (#5238). Otherwise append the fallback.
|
||||
*/
|
||||
export function buildStandaloneNodeOptions(env = process.env, omnirouteMb) {
|
||||
const existing = String(env?.NODE_OPTIONS || "").trim();
|
||||
if (envHasExplicitOmnirouteMemoryMb(env)) {
|
||||
return `${existing} ${MAX_OLD_SPACE_FLAG}=${omnirouteMb}`.trim();
|
||||
}
|
||||
if (existing.includes(MAX_OLD_SPACE_FLAG)) return existing;
|
||||
return `${existing} ${MAX_OLD_SPACE_FLAG}=${omnirouteMb}`.trim();
|
||||
}
|
||||
|
||||
/**
|
||||
* Assemble the NODE_OPTIONS string for the spawned server, preserving any flags
|
||||
* the user already exported. #5238: `omniroute serve` used to UNCONDITIONALLY
|
||||
|
||||
@@ -5,6 +5,8 @@ import {
|
||||
resolveRuntimePorts,
|
||||
withRuntimePortEnv,
|
||||
resolveMaxOldSpaceMb,
|
||||
warnConflictingHeapLimits,
|
||||
buildStandaloneNodeOptions,
|
||||
spawnWithForwardedSignals,
|
||||
} from "../build/runtime-env.mjs";
|
||||
import { bootstrapEnv } from "../build/bootstrap-env.mjs";
|
||||
@@ -13,13 +15,13 @@ const env = bootstrapEnv();
|
||||
const runtimePorts = resolveRuntimePorts(env);
|
||||
const childEnv = withRuntimePortEnv(env, runtimePorts);
|
||||
|
||||
// #2939: honor OMNIROUTE_MEMORY_MB (default 512), the same knob
|
||||
// `omniroute serve` uses, so Docker users can control the server heap under
|
||||
// load / large SQLite DBs. A trailing --max-old-space-size wins, so this
|
||||
// overrides the image fallback without clobbering any other NODE_OPTIONS flags.
|
||||
// #2939 / #10353: OMNIROUTE_MEMORY_MB is the Docker/standalone heap knob.
|
||||
// When it is set, we append --max-old-space-size last (V8 last-flag wins).
|
||||
// When it is unset and NODE_OPTIONS already pins the heap, keep NODE_OPTIONS
|
||||
// (#5238). Warn when both are set and the numbers disagree.
|
||||
const maxOldSpaceMb = resolveMaxOldSpaceMb(childEnv.OMNIROUTE_MEMORY_MB);
|
||||
childEnv.NODE_OPTIONS =
|
||||
`${childEnv.NODE_OPTIONS || ""} --max-old-space-size=${maxOldSpaceMb}`.trim();
|
||||
warnConflictingHeapLimits(childEnv, maxOldSpaceMb);
|
||||
childEnv.NODE_OPTIONS = buildStandaloneNodeOptions(childEnv, maxOldSpaceMb);
|
||||
|
||||
// Prefer the WS-aware wrapper (server-ws.mjs) over the bare Next standalone
|
||||
// server.js: it installs the trusted peer-IP stamp (scripts/dev/peer-stamp.mjs)
|
||||
|
||||
73
tests/unit/10353-heap-limit-conflict.test.ts
Normal file
73
tests/unit/10353-heap-limit-conflict.test.ts
Normal file
@@ -0,0 +1,73 @@
|
||||
/**
|
||||
* #10353 — warn when OMNIROUTE_MEMORY_MB disagrees with NODE_OPTIONS heap.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const {
|
||||
parseNodeOptionsHeapMb,
|
||||
envHasExplicitOmnirouteMemoryMb,
|
||||
warnConflictingHeapLimits,
|
||||
buildStandaloneNodeOptions,
|
||||
} = await import("../../scripts/build/runtime-env.mjs");
|
||||
|
||||
test("parseNodeOptionsHeapMb reads the last heap flag", () => {
|
||||
assert.equal(parseNodeOptionsHeapMb(""), null);
|
||||
assert.equal(parseNodeOptionsHeapMb("--enable-source-maps"), null);
|
||||
assert.equal(parseNodeOptionsHeapMb("--max-old-space-size=512"), 512);
|
||||
assert.equal(
|
||||
parseNodeOptionsHeapMb("--max-old-space-size=512 --max-old-space-size=2048"),
|
||||
2048
|
||||
);
|
||||
});
|
||||
|
||||
test("envHasExplicitOmnirouteMemoryMb requires an in-range integer", () => {
|
||||
assert.equal(envHasExplicitOmnirouteMemoryMb({}), false);
|
||||
assert.equal(envHasExplicitOmnirouteMemoryMb({ OMNIROUTE_MEMORY_MB: "" }), false);
|
||||
assert.equal(envHasExplicitOmnirouteMemoryMb({ OMNIROUTE_MEMORY_MB: "abc" }), false);
|
||||
assert.equal(envHasExplicitOmnirouteMemoryMb({ OMNIROUTE_MEMORY_MB: "32" }), false);
|
||||
assert.equal(envHasExplicitOmnirouteMemoryMb({ OMNIROUTE_MEMORY_MB: "2048" }), true);
|
||||
});
|
||||
|
||||
test("#10353 dual-set disagree → warn + OMNIROUTE_MEMORY_MB wins", () => {
|
||||
const messages: string[] = [];
|
||||
const env = {
|
||||
NODE_OPTIONS: "--max-old-space-size=512",
|
||||
OMNIROUTE_MEMORY_MB: "2048",
|
||||
};
|
||||
assert.equal(warnConflictingHeapLimits(env, 2048, (m: string) => messages.push(m)), true);
|
||||
assert.match(messages[0], /OMNIROUTE_MEMORY_MB=2048/);
|
||||
assert.match(messages[0], /--max-old-space-size=512/);
|
||||
assert.match(messages[0], /effective V8 heap is 2048 MB/);
|
||||
assert.equal(
|
||||
buildStandaloneNodeOptions(env, 2048),
|
||||
"--max-old-space-size=512 --max-old-space-size=2048"
|
||||
);
|
||||
});
|
||||
|
||||
test("#10353 only one knob set → no conflict warn", () => {
|
||||
const messages: string[] = [];
|
||||
const log = (m: string) => messages.push(m);
|
||||
assert.equal(
|
||||
warnConflictingHeapLimits({ NODE_OPTIONS: "--max-old-space-size=512" }, 512, log),
|
||||
false
|
||||
);
|
||||
assert.equal(
|
||||
warnConflictingHeapLimits({ OMNIROUTE_MEMORY_MB: "2048" }, 2048, log),
|
||||
false
|
||||
);
|
||||
assert.equal(
|
||||
warnConflictingHeapLimits(
|
||||
{ NODE_OPTIONS: "--max-old-space-size=1024", OMNIROUTE_MEMORY_MB: "1024" },
|
||||
1024,
|
||||
log
|
||||
),
|
||||
false
|
||||
);
|
||||
assert.equal(messages.length, 0);
|
||||
});
|
||||
|
||||
test("#10353 unset OMNIROUTE_MEMORY_MB keeps NODE_OPTIONS heap", () => {
|
||||
const env = { NODE_OPTIONS: "--max-old-space-size=8192" };
|
||||
assert.equal(buildStandaloneNodeOptions(env, 512), "--max-old-space-size=8192");
|
||||
});
|
||||
Reference in New Issue
Block a user