diff --git a/cli/README.md b/cli/README.md index 050d996a87..dd7884f629 100644 --- a/cli/README.md +++ b/cli/README.md @@ -88,6 +88,20 @@ That's it! Start coding with FREE AI models. **Dashboard**: `http://localhost:20128/dashboard` +### Memory limit + +The server process starts with a 6 GB V8 heap cap. On a memory-limited host +(systemd `MemoryMax`, `docker --memory`, k8s limits) lower it so the garbage +collector feels the limit before the kernel does: + +```bash +NINEROUTER_MAX_OLD_SPACE_SIZE=384 9router # cap the heap at 384 MB +NINEROUTER_MAX_OLD_SPACE_SIZE=0 9router # no cap — let node size it +``` + +`NODE_OPTIONS=--max-old-space-size=…` is honored too, and takes effect only +because 9Router stops passing its own default when you set one. + --- ## 🛠️ Supported CLI Tools diff --git a/cli/cli.js b/cli/cli.js index 703039dd91..65501ab75b 100755 --- a/cli/cli.js +++ b/cli/cli.js @@ -64,6 +64,7 @@ function createSpinner(text) { const pkg = require("./package.json"); const { ensureSqliteRuntime, buildEnvWithRuntime } = require("./hooks/sqliteRuntime"); +const { resolveHeapFlags } = require("./hooks/nodeFlags"); const { ensureTrayRuntime } = require("./hooks/trayRuntime"); const args = process.argv.slice(2); @@ -612,7 +613,7 @@ function startServer(updatePromise) { function spawnServer() { serverStartTime = Date.now(); crashLog = []; - const child = spawn(RUNTIME, ["--dns-result-order=ipv4first", "--max-old-space-size=6144", serverPath], { + const child = spawn(RUNTIME, ["--dns-result-order=ipv4first", ...resolveHeapFlags(process.env), serverPath], { cwd: standaloneDir, stdio: showLog ? "inherit" : ["ignore", "ignore", "pipe"], detached: true, diff --git a/cli/hooks/nodeFlags.js b/cli/hooks/nodeFlags.js new file mode 100644 index 0000000000..ec646bc682 --- /dev/null +++ b/cli/hooks/nodeFlags.js @@ -0,0 +1,47 @@ +/** + * Node flags for the spawned next-server child. + * + * Node reads NODE_OPTIONS first and lets command-line flags win, so a + * hard-coded --max-old-space-size on the spawn line silently overrides + * whatever the operator configured. Where a cgroup limit applies (systemd + * MemoryMax, docker --memory, k8s), the child then runs believing it has + * several GB of heap: GC never feels pressure, RSS climbs to the ceiling, and + * the kernel OOM-kills next-server mid-stream — taking in-flight streaming + * responses with it (#3365). + * + * The default stays as it was, for the desktop case it was raised for. It just + * steps aside once the operator has said what they want. + */ + +const DEFAULT_MAX_OLD_SPACE_MB = 6144; + +// Node accepts the underscore spelling of V8 flags too (--max_old_space_size). +const HEAP_FLAG_PATTERN = /(^|\s)--max[-_]old[-_]space[-_]size(=|\s|$)/; + +/** + * @param {NodeJS.ProcessEnv} env + * @returns {string[]} heap flags to pass to the child, possibly empty + */ +function resolveHeapFlags(env = process.env) { + const explicit = String(env.NINEROUTER_MAX_OLD_SPACE_SIZE ?? "").trim(); + if (explicit) { + // 0 hands the decision back to node, which sizes the heap from the memory + // it can actually see — the right answer inside a container. + if (explicit === "0") return []; + const megabytes = Number(explicit); + if (Number.isInteger(megabytes) && megabytes > 0) { + return [`--max-old-space-size=${megabytes}`]; + } + console.warn( + `[9router] ignoring NINEROUTER_MAX_OLD_SPACE_SIZE="${explicit}": expected a positive integer (MB) or 0`, + ); + } + + // Already set in NODE_OPTIONS: leave it alone, otherwise the spawn line + // would win and the setting would look ignored. + if (HEAP_FLAG_PATTERN.test(String(env.NODE_OPTIONS ?? ""))) return []; + + return [`--max-old-space-size=${DEFAULT_MAX_OLD_SPACE_MB}`]; +} + +module.exports = { resolveHeapFlags, DEFAULT_MAX_OLD_SPACE_MB }; diff --git a/tests/unit/cli-heap-flags-3365.test.js b/tests/unit/cli-heap-flags-3365.test.js new file mode 100644 index 0000000000..491a784bff --- /dev/null +++ b/tests/unit/cli-heap-flags-3365.test.js @@ -0,0 +1,76 @@ +// #3365 — the CLI hard-coded --max-old-space-size=6144 on the next-server +// spawn line. Node lets command-line flags beat NODE_OPTIONS, so an operator +// running under a cgroup limit could not lower it: the child kept a 6 GB heap +// budget, GC never felt pressure, and the kernel OOM-killed next-server. +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { createRequire } from "node:module"; + +const require = createRequire(import.meta.url); +const { resolveHeapFlags, DEFAULT_MAX_OLD_SPACE_MB } = require("../../cli/hooks/nodeFlags.js"); + +const DEFAULT_FLAG = `--max-old-space-size=${DEFAULT_MAX_OLD_SPACE_MB}`; + +describe("resolveHeapFlags (#3365)", () => { + let warn; + + beforeEach(() => { + warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + warn.mockRestore(); + }); + + it("keeps the 6144 default when nothing is configured", () => { + expect(resolveHeapFlags({})).toEqual([DEFAULT_FLAG]); + expect(DEFAULT_MAX_OLD_SPACE_MB).toBe(6144); + }); + + it("honours an explicit cap", () => { + expect(resolveHeapFlags({ NINEROUTER_MAX_OLD_SPACE_SIZE: "384" })) + .toEqual(["--max-old-space-size=384"]); + expect(resolveHeapFlags({ NINEROUTER_MAX_OLD_SPACE_SIZE: " 1024 " })) + .toEqual(["--max-old-space-size=1024"]); + }); + + it("emits no flag at all for 0, leaving the sizing to node", () => { + expect(resolveHeapFlags({ NINEROUTER_MAX_OLD_SPACE_SIZE: "0" })).toEqual([]); + }); + + // The spawn-line flag would beat NODE_OPTIONS, so an operator who set it + // there would see their setting silently ignored — the bug in the report. + it("stands aside when NODE_OPTIONS already caps the heap", () => { + expect(resolveHeapFlags({ NODE_OPTIONS: "--max-old-space-size=384" })).toEqual([]); + expect(resolveHeapFlags({ NODE_OPTIONS: "--enable-source-maps --max-old-space-size=384" })).toEqual([]); + // Node accepts the underscore spelling of V8 flags too. + expect(resolveHeapFlags({ NODE_OPTIONS: "--max_old_space_size=384" })).toEqual([]); + }); + + it("ignores unrelated NODE_OPTIONS", () => { + expect(resolveHeapFlags({ NODE_OPTIONS: "--enable-source-maps" })).toEqual([DEFAULT_FLAG]); + // Not a match: a different flag that merely contains the name. + expect(resolveHeapFlags({ NODE_OPTIONS: "--max-old-space-size-hint=8" })).toEqual([DEFAULT_FLAG]); + }); + + it("prefers the dedicated var over NODE_OPTIONS", () => { + const flags = resolveHeapFlags({ + NINEROUTER_MAX_OLD_SPACE_SIZE: "512", + NODE_OPTIONS: "--max-old-space-size=4096", + }); + expect(flags).toEqual(["--max-old-space-size=512"]); + }); + + // A safety cap must not disappear because someone typed the value wrong. + it("falls back to the default on a junk value, and says so", () => { + for (const value of ["abc", "-1", "1.5", "512MB"]) { + expect(resolveHeapFlags({ NINEROUTER_MAX_OLD_SPACE_SIZE: value }), value).toEqual([DEFAULT_FLAG]); + } + expect(warn).toHaveBeenCalledTimes(4); + expect(warn.mock.calls[0][0]).toContain("NINEROUTER_MAX_OLD_SPACE_SIZE"); + }); + + it("treats an empty or whitespace value as unset, without warning", () => { + expect(resolveHeapFlags({ NINEROUTER_MAX_OLD_SPACE_SIZE: " " })).toEqual([DEFAULT_FLAG]); + expect(warn).not.toHaveBeenCalled(); + }); +});