From ad1b9227c4964cae26416b4c0743f1dd489a5038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 25 Jul 2026 09:16:17 +0100 Subject: [PATCH] fix: large-repo analyze OOM and false worker-timeout cascade (#2649) (#2679) --- .devcontainer/Dockerfile | 1 + gitnexus/README.md | 24 +++ gitnexus/src/cli/analyze.ts | 85 ++++++--- .../ingestion/pipeline-phases/parse-impl.ts | 105 +++++++++++ .../src/core/ingestion/utils/effective-ram.ts | 67 +++++++ .../src/core/ingestion/workers/worker-pool.ts | 116 +++++++++++- gitnexus/src/server/analyze-launch.ts | 11 +- .../integration/analyze-heap-oom-e2e.test.ts | 3 + .../test/unit/analyze-heap-respawn.test.ts | 165 +++++++++++++++-- .../parse-impl-heap-guard-pipeline.test.ts | 136 ++++++++++++++ .../test/unit/parse-impl-heap-guard.test.ts | 87 +++++++++ .../unit/worker-pool-resource-limits.test.ts | 166 ++++++++++++++++++ .../unit/worker-pool-stall-credit.test.ts | 150 ++++++++++++++++ gitnexus/vitest.config.ts | 6 + 14 files changed, 1085 insertions(+), 37 deletions(-) create mode 100644 gitnexus/src/core/ingestion/utils/effective-ram.ts create mode 100644 gitnexus/test/unit/parse-impl-heap-guard-pipeline.test.ts create mode 100644 gitnexus/test/unit/parse-impl-heap-guard.test.ts create mode 100644 gitnexus/test/unit/worker-pool-resource-limits.test.ts create mode 100644 gitnexus/test/unit/worker-pool-stall-credit.test.ts diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index c57c0d6aa..96003dcef 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -39,6 +39,7 @@ ENV BUN_VERSION=${BUN_VERSION} \ TZ=${TZ} \ DEVCONTAINER=true \ NODE_OPTIONS=--max-old-space-size=4096 \ + GITNEXUS_AUTO_HEAP=0 \ POWERLEVEL9K_DISABLE_GITSTATUS=true # Native build toolchain that gitnexus/postinstall needs. It compiles diff --git a/gitnexus/README.md b/gitnexus/README.md index c1e9e6050..756a40d88 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -506,6 +506,27 @@ GITNEXUS_FTS_CJK_SEGMENTATION=bigram npx gitnexus analyze --force ### Analysis runs out of memory +Memory management is automatic: `analyze` sizes its heap to the machine +(always below physical RAM), caps each parse worker, and — rather than +grinding into a GC death spiral or crash — stops early with a message telling +you the one thing to do. Repeated +`Replacement worker did not report ready within 5000ms` warnings on a large +repository are part of the same picture: memory pressure starving healthy +workers, not a worker bug (#2649). + +If analyze says the repository doesn't fit, do what the message says: + +- **The machine has more memory to give** (a `NODE_OPTIONS` + `--max-old-space-size` pin from your environment is holding analyze back): + re-run without the pin — no flags needed. +- **The machine is the ceiling**: shrink the scope (exclude generated or + vendored directories, below) or use a machine with more RAM. + +Escape hatches (`GITNEXUS_MEMORY=off` to decline the autopilot, +`GITNEXUS_WORKER_HEAP_MB` to size workers yourself) are listed in the +environment-variable table below — +most users never need them. + For very large repositories: ```bash @@ -568,6 +589,9 @@ Four env vars expose the pool's resilience layers (respawn budget, cumulative-ti | `GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD` | `max(3, poolSize)` | Per-slot consecutive deaths before the pool's circuit breaker trips. After tripping, dispatches require a fresh pool. | | `GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS` | `30000` | Max wait at pool shutdown for a retired worker still inside native code — terminated at its next JS-safe point instead of mid-native-call, which would abort the process (`Napi::Error`, #2432). | | `GITNEXUS_WORKER_READY_TIMEOUT_MS` | `5000` | Startup budget for a parse worker to load its grammar bindings and report `{type:'ready'}`. Slots that miss it are treated as startup crashes. Raise it on a slow or heavily loaded host where a full pool cold-starting concurrently needs more than 5s. | +| `GITNEXUS_MEMORY` | `off` | unset (autopilot on) | `off` declines GitNexus's memory autopilot: analyze will neither re-run itself with a RAM-aware heap cap nor abort the parse before V8 enters its ineffective-mark-compact death spiral. Use it when you want to drive memory manually; to simply pin a heap size, pass Node's own `--max-old-space-size`, which is already honoured as your decision. | +| `GITNEXUS_WORKER_HEAP_MB` | `clamp(512, RAM/2/poolSize, 4096)` | Per-worker V8 old-generation heap cap (#2649). Bounds pool RSS on large repos; a worker exceeding it dies with a real heap error handled by quarantine/respawn. | +| `GITNEXUS_SERVER_ANALYZE_HEAP_MB` | `min(8192, auto cap)` | Heap for the web/MCP server's forked analyze worker (#2649). Defaults to the historical 8192 MB bounded by the machine/container's RAM-aware auto cap; set an absolute MB value to override. | | `GITNEXUS_CPP_CAPTURE_BUDGET_MS` | `20000` | Per-file wall-clock budget for C++ capture extraction; on breach the file keeps partial captures with a warning (#2432). `0` expires immediately. | ### Graph cleanup tuning diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index b9a50a720..9475ee25f 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -54,7 +54,8 @@ import { getMaxFileSizeBannerMessage } from '../core/ingestion/utils/max-file-si import { warnMissingOptionalGrammars, getOptionalGrammarExtensions } from './optional-grammars.js'; import { glob } from 'glob'; import fs from 'fs/promises'; -import { cliError } from './cli-message.js'; +import { cliError, cliWarn } from './cli-message.js'; +import { heapCapMbFor, memoryAutopilotDisabled } from '../core/ingestion/utils/effective-ram.js'; import { EMBEDDING_DIMS_ERROR, normalizeEmbeddingDims } from './embedding-dims.js'; import { formatElapsed } from './format-elapsed.js'; import { isHfDownloadFailure } from '../core/embeddings/hf-env.js'; @@ -135,25 +136,21 @@ const installFatalHandlers = (): void => { }); }; -/** Historical floor for the re-exec heap cap — the auto-sizer never goes below - * this, so small boxes / CI never regress. */ -const DEFAULT_HEAP_MB = 16384; - /** - * RAM-aware re-exec heap cap (MB): `0.75 × effective RAM`, clamped to - * `>= DEFAULT_HEAP_MB`. Kept BELOW physical RAM on purpose — a cap `>=` RAM makes - * V8 collect lazily and inflate the heap into swap-thrash (observed analyzing the - * Linux kernel at a 30GB cap on a 31GB box). `constrainedBytes` is the cgroup - * limit or `null`; it is honored only as a real, smaller-than-physical cap, because + * RAM-aware re-exec heap cap (MB) — the formula itself is single-sourced in + * `core/ingestion/utils/effective-ram.ts` (`heapCapMbFor`), shared with the + * server's analyze fork. `constrainedBytes` is the cgroup limit or `null`; + * it is honored only as a real, smaller-than-physical cap, because * `process.constrainedMemory()` returns a huge sentinel when UNCONSTRAINED. + * (Observed rationale: a cap ≥ RAM made V8 collect lazily and swap-thrash — + * the #2649 worker-timeout cascade on 16 GB boxes.) */ export function computeHeapCapMb(totalBytes: number, constrainedBytes: number | null): number { const effectiveBytes = constrainedBytes !== null && constrainedBytes > 0 && constrainedBytes < totalBytes ? constrainedBytes : totalBytes; - const effectiveMb = Math.floor(effectiveBytes / (1024 * 1024)); - return Math.max(DEFAULT_HEAP_MB, Math.floor(0.75 * effectiveMb)); + return heapCapMbFor(effectiveBytes); } function readConstrainedBytes(): number | null { @@ -523,21 +520,69 @@ const forceHeapOOMForTestIfEnabled = (): void => { // `gitnexus/src/core/lbug/lbug-config.ts` in sync with this value. const RECOMMENDED_WAL_CHECKPOINT_THRESHOLD = 64 * 1024 * 1024; -/** Re-exec the process with the RAM-aware auto heap cap + larger semi-space/stack - * if we're currently below that. A user-supplied NODE_OPTIONS heap wins (no re-exec). */ -async function ensureHeap(): Promise { - const nodeOpts = process.env.NODE_OPTIONS || ''; - if (nodeOpts.includes('--max-old-space-size')) return false; +/** + * Last `--max-old-space-size` value (MB) in a NODE_OPTIONS string, or `null` + * when absent/unparseable. Last occurrence wins, matching V8's own + * later-flag-wins semantics when NODE_OPTIONS repeats a flag. + */ +export function parseMaxOldSpaceMb(nodeOptions: string): number | null { + // V8 accepts `-` and `_` interchangeably in flag names, and Node accepts a + // space-separated value in NODE_OPTIONS — honor every spelling of the pin + // instead of silently overriding it (#2649 review). + const matches = [...nodeOptions.matchAll(/--max[-_]old[-_]space[-_]size(?:=|\s+)(\d+)/g)]; + if (matches.length === 0) return null; + const mb = Number(matches[matches.length - 1][1]); + return Number.isFinite(mb) && mb > 0 ? mb : null; +} - const v8Heap = v8.getHeapStatistics().heap_size_limit; - if (v8Heap >= HEAP_MB * 1024 * 1024 * 0.9) return false; +/** Re-exec the process with the RAM-aware auto heap cap + larger semi-space/stack + * if we're currently below that. + * + * Heap-source precedence (#2649): + * - an explicit per-invocation `--max-old-space-size` (execArgv) always wins; + * - `GITNEXUS_MEMORY=off` declines the memory autopilot entirely; + * - an ambient NODE_OPTIONS heap >= the auto cap is honored as-is; + * - an ambient NODE_OPTIONS heap BELOW the auto cap is treated as an + * inherited environment default (devcontainers/CI export one for other + * tooling), not a deliberate per-run choice: warn and respawn with the + * auto cap. Pre-#2649 this returned early and large repos then OOM'd on + * whatever heap the environment happened to specify. */ +async function ensureHeap(): Promise { + // Explicit opt-out disables auto-sizing ENTIRELY — both the ambient-pin + // override and the default v8-limit respawn — and is honored SILENTLY: + // the operator already made the call, and stderr-sensitive consumers + // (test harnesses, scripts, supervisors that track a single PID) rely on + // a quiet, single-process run. + if (memoryAutopilotDisabled()) return false; + const nodeOpts = process.env.NODE_OPTIONS || ''; + if (process.execArgv.some((a) => a.startsWith('--max-old-space-size'))) return false; + + const ambientHeapMb = parseMaxOldSpaceMb(nodeOpts); + if (ambientHeapMb !== null) { + if (ambientHeapMb >= RESPAWN_HEAP_MB) return false; + cliWarn( + ` NODE_OPTIONS pins the heap to ${ambientHeapMb}MB — below the ${RESPAWN_HEAP_MB}MB this machine's RAM supports.\n` + + ` Re-running analyze with the larger auto-sized cap (set GITNEXUS_MEMORY=off to keep the NODE_OPTIONS value).\n`, + ); + } else { + const v8Heap = v8.getHeapStatistics().heap_size_limit; + if (v8Heap >= HEAP_MB * 1024 * 1024 * 0.9) return false; + } // --stack-size is a V8 flag not allowed in NODE_OPTIONS on Node 24+, so pass it // only as a direct CLI argument. --max-semi-space-size IS allowed in NODE_OPTIONS. const cliFlags = [HEAP_FLAG, SEMI_FLAG]; if (!nodeOpts.includes('--stack-size')) cliFlags.push(STACK_FLAG); - const childArgs = [...cliFlags, ...process.argv.slice(1)]; + // Preserve the parent's node flags (execArgv) — dropping them breaks any + // loader-launched CLI: `node --import tsx src/cli/index.ts` respawned + // without `--import tsx` cannot execute TypeScript and dies with a + // swallowed exit 1 (#2649 review). Our heap/semi/stack flags come AFTER + // execArgv so V8's later-flag-wins semantics resolve duplicates our way. + // Inspector flags are the one exception: replaying `--inspect[-brk]` makes + // the child fight the parent for the debug port and die with EADDRINUSE. + const preservedExecArgv = process.execArgv.filter((a) => !a.startsWith('--inspect')); + const childArgs = [...preservedExecArgv, ...cliFlags, ...process.argv.slice(1)]; const childEnv = { ...process.env, NODE_OPTIONS: `${nodeOpts} ${HEAP_FLAG} ${SEMI_FLAG}`.trim(), diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index a9d601f11..55611848a 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -95,7 +95,9 @@ import { import type { KnowledgeGraph } from '../../graph/types.js'; import type { PipelineOptions } from '../pipeline.js'; import fs from 'node:fs'; +import { effectiveRamBytes, memoryAutopilotDisabled } from '../utils/effective-ram.js'; import path from 'node:path'; +import v8 from 'node:v8'; import { fileURLToPath, pathToFileURL } from 'node:url'; import { isDev } from '../utils/env.js'; @@ -111,6 +113,81 @@ import { isDebugHeapEnabled, logHeapProbe } from '../utils/heap-probe.js'; import { logger } from '../../logger.js'; // ── Constants ────────────────────────────────────────────────────────────── +/** + * Heap-scale guardrail constants (#2649). Measured on a Linux-kernel analyze: + * ~75 graph nodes per PARSEABLE file (~5M nodes / ~65k parseable files; + * validated against heap probes at chunks 25/50/75 of 113 — the first + * calibration divided by total scanned files and under-projected by ~30%), + * main-thread heap per node. C-heavy corpus; other language mixes vary — these + * feed a WARNING and an emergency abort, never a hard admission gate, so + * estimate error only shifts when the operator hears about the problem, not + * whether analyze runs. + * + * RECALIBRATED for streamed structural emit (#2680), which is on by default for + * full rebuilds and holds relationships out of the JS heap. The original 2250 + * was measured against the object-based graph; an A/B at 400k nodes / 1.08M + * edges put streaming at 1.40x smaller (819 MB -> 584 MB), so the corpus- + * calibrated figure is divided by that ratio: 2250 / 1.40 ~= 1600. Scaling the + * measured constant rather than substituting a synthetic one keeps #2649's + * kernel calibration intact and changes only the one thing that actually moved. + * + * If streaming is disabled (GITNEXUS_STREAM_GRAPH_EMIT=0, or any non-force run) + * this UNDER-projects by ~40%, so the preflight warning may stay quiet on a repo + * that then struggles. That is the safe direction to be wrong in: the abort + * below reads LIVE heap use, not this projection, so it still catches the real + * condition — only the early warning is affected. + */ +const PROJECTED_NODES_PER_FILE = 75; +const PROJECTED_HEAP_BYTES_PER_NODE = 1600; +/** Warn at scan end when the projection crosses this share of the heap limit. */ +const PREFLIGHT_WARN_FRACTION = 0.85; +/** + * Abort the chunk loop when live heap use crosses this share of the limit. + * Above ~0.95 V8 enters the ineffective-mark-compact death spiral (2s+ GC + * pauses that also falsely idle-timeout healthy workers, #2649); 0.92 leaves + * one chunk's worth of headroom to fail with an actionable message instead. + * `GITNEXUS_MEMORY=off` declines the abort (proceed-at-own-risk). + */ +const HEAP_ABORT_FRACTION = 0.92; + +/** Projected main-thread heap need for the parse phase (#2649). */ +export function projectParseHeapNeedBytes(parseableFileCount: number): number { + return parseableFileCount * PROJECTED_NODES_PER_FILE * PROJECTED_HEAP_BYTES_PER_NODE; +} + +/** True when the mid-loop heap guard should abort the parse (#2649). */ +export function shouldAbortForHeapPressure(heapUsedBytes: number, heapLimitBytes: number): boolean { + if (memoryAutopilotDisabled()) return false; + return heapUsedBytes > heapLimitBytes * HEAP_ABORT_FRACTION; +} + +/** + * The ONE action a user should take when this repository doesn't fit the + * current heap (#2649). Users hitting memory limits are already frustrated — + * a menu of env knobs at that moment is noise. Branch on whether the machine + * itself has more memory to give: if this process's limit sits well below + * what the RAM-aware auto-sizer would grant (an inherited NODE_OPTIONS pin or + * explicit flag), the fix is to drop the pin — gitnexus sizes itself. + * Otherwise the machine is the ceiling and only scope or hardware helps. + * Escape hatches (GITNEXUS_MEMORY etc.) stay in the README env table. + */ +export function heapPressureRemedy(heapLimitBytes: number): string { + // Effective RAM honors a real cgroup limit — raw os.totalmem() told users + // inside an 8GB-limited container on a 64GB host that "this machine has + // more memory available", an advice loop with no exit (#2649 review). + const autoCapBytes = effectiveRamBytes() * 0.75; + if (heapLimitBytes < autoCapBytes * 0.9) { + return ( + `This machine has more memory available: re-run without the --max-old-space-size ` + + `pin (NODE_OPTIONS or node flag) — gitnexus sizes its heap to the machine automatically.` + ); + } + return ( + `This machine is at its memory ceiling: exclude generated or vendored directories ` + + `via .gitnexusignore, or analyze on a machine with more memory.` + ); +} + /** Max bytes of source content to load per parse chunk. * * Memory bound for the worker pool dispatch + a granularity knob for @@ -516,6 +593,22 @@ export async function runChunkedParseAndResolve( MIN_SUB_BATCH_BYTES, Math.ceil(chunkByteBudget / (effectivePoolSize * TARGET_JOBS_PER_WORKER)), ); + // Heap-scale guardrails (#2649), measured on a Linux-kernel analyze + // (94,773 files): ~55 graph nodes per parseable file and ~2.2KB of + // main-thread heap per node, linear across 113 chunks (see + // docs/plans/2026-07-23-gitnexus-plan-large-repo-analyze-oom.md §2). + // Estimates, not contracts — used only to warn early (preflight) and to + // convert a certain multi-minute GC death spiral into an immediate + // actionable error (mid-loop guard). + const projectedHeapNeedBytes = projectParseHeapNeedBytes(parseableScanned.length); + const heapLimitBytes = v8.getHeapStatistics().heap_size_limit; + if (projectedHeapNeedBytes > heapLimitBytes * PREFLIGHT_WARN_FRACTION) { + logger.warn( + `Large repository: analyzing ${parseableScanned.length} files needs roughly ${Math.round(projectedHeapNeedBytes / 1024 / 1024 / 1024)}GB of memory, ` + + `but Node is limited to ${Math.round(heapLimitBytes / 1024 / 1024 / 1024)}GB — analyze may stop early. ${heapPressureRemedy(heapLimitBytes)}`, + ); + } + const chunks: string[][] = []; let currentChunk: string[] = []; let currentBytes = 0; @@ -869,6 +962,18 @@ export async function runChunkedParseAndResolve( `nodes=${graph.nodeCount} parsedFiles=${allParsedFiles.length}`, ); } + // #2649 mid-loop heap guard: fail actionably BEFORE V8 enters the + // ineffective-mark-compact death spiral (which also falsely times out + // healthy workers). The pool is torn down by this function's finally. + const heapUsedNow = process.memoryUsage().heapUsed; + const heapLimitNow = v8.getHeapStatistics().heap_size_limit; + if (shouldAbortForHeapPressure(heapUsedNow, heapLimitNow)) { + throw new Error( + `Analyze stopped before running out of memory: ${Math.round(heapUsedNow / 1024 / 1024)}MB of the ` + + `${Math.round(heapLimitNow / 1024 / 1024)}MB Node heap in use at parse chunk ${chunkIdx + 1}/${numChunks} (#2649). ` + + heapPressureRemedy(heapLimitNow), + ); + } const chunkPaths = chunks[chunkIdx]; // Start wall-clock for the per-chunk throughput log emitted at end // of this iteration. The gate is computed once above; here we just diff --git a/gitnexus/src/core/ingestion/utils/effective-ram.ts b/gitnexus/src/core/ingestion/utils/effective-ram.ts new file mode 100644 index 000000000..c2503ba97 --- /dev/null +++ b/gitnexus/src/core/ingestion/utils/effective-ram.ts @@ -0,0 +1,67 @@ +import os from 'node:os'; + +/** + * Effective RAM in bytes: physical total, or a REAL smaller cgroup limit + * (#2649). `process.constrainedMemory()` returns a huge sentinel when + * unconstrained, and only the leaf cgroup's limit is visible (parent-slice + * caps are not) — so a smaller-than-physical value is trusted and anything + * else falls back to `os.totalmem()`. Mirrors `computeHeapCapMb`'s + * constrained handling in `cli/analyze.ts`; container-blind sizing told + * users "this machine has more memory" inside an 8GB-limited container on + * a 64GB host, and sized worker heap caps past the whole container. + */ +export function effectiveRamBytes(): number { + const total = os.totalmem(); + const constrained = + typeof process.constrainedMemory === 'function' ? process.constrainedMemory() : undefined; + return typeof constrained === 'number' && constrained > 0 && constrained < total + ? constrained + : total; +} + +/** Historical floor for the auto heap cap — applied only up to 0.80 × RAM + * (a floor at or above physical memory swap-thrashes instead of OOMing, + * #2649). */ +const HEAP_FLOOR_MB = 16384; + +/** + * The RAM-aware heap cap formula (#2649), single-sourced here so the CLI + * respawn (`computeHeapCapMb` in `cli/analyze.ts`), and the server's + * analyze fork size from the same rule: `0.75 × effective RAM`, raised to + * the floor when RAM allows, never above `0.80 × effective RAM`. + */ +export function heapCapMbFor(effectiveBytes: number): number { + const effectiveMb = Math.floor(effectiveBytes / (1024 * 1024)); + return Math.min( + Math.max(HEAP_FLOOR_MB, Math.floor(0.75 * effectiveMb)), + Math.floor(0.8 * effectiveMb), + ); +} + +/** + * True when the operator has turned GitNexus's memory autopilot off + * (`GITNEXUS_MEMORY=off`). + * + * One switch for one concern. Memory management has two automatic behaviours — + * re-running analyze with a RAM-aware heap cap, and aborting the parse before + * V8's ineffective-mark-compact death spiral — and an operator who wants to + * drive manually wants both off, not one. They were previously two separate + * variables (`GITNEXUS_AUTO_HEAP`, `GITNEXUS_HEAP_GUARD`), which is three knobs + * for one intent once the worker-heap override is counted; neither had shipped, + * so this consolidates them rather than deprecating anything. + * + * Note the ordinary way to pin the heap is Node's own `--max-old-space-size`, + * which `ensureHeap` already honours as the operator's decision. This switch is + * for declining the autopilot WITHOUT naming a size. + * + * Lives here beside the cap formula so policy and its escape hatch are + * single-sourced. Read every call (not memoized) so tests can stub the env. + */ +export function memoryAutopilotDisabled(): boolean { + return process.env.GITNEXUS_MEMORY === 'off'; +} + +/** The cap for THIS machine/container: `heapCapMbFor(effectiveRamBytes())`. */ +export function autoHeapCapMb(): number { + return heapCapMbFor(effectiveRamBytes()); +} diff --git a/gitnexus/src/core/ingestion/workers/worker-pool.ts b/gitnexus/src/core/ingestion/workers/worker-pool.ts index 9cf40a1f9..de102ebef 100644 --- a/gitnexus/src/core/ingestion/workers/worker-pool.ts +++ b/gitnexus/src/core/ingestion/workers/worker-pool.ts @@ -1,5 +1,6 @@ import { Worker } from 'node:worker_threads'; import os from 'node:os'; +import { effectiveRamBytes } from '../utils/effective-ram.js'; import fs from 'node:fs'; import { fileURLToPath } from 'node:url'; @@ -224,6 +225,13 @@ export interface WorkerPoolOptions { * code should leave this unset. */ workerFactory?: (workerUrl: URL) => Worker; + /** + * Test-only injection point for the main-thread stall probe (#2649): + * returns cumulative event-loop stall in ms. When provided, the pool + * skips its heartbeat tracker and reads this instead. Production code + * should leave this unset. + */ + stallMsProbe?: () => number; /** * Storage path for the disk-backed ParsedFile store (#1983 parallel * serialization). When set, it is baked into every spawned worker's @@ -811,7 +819,7 @@ function waitForWorkerReady(worker: Worker, readyTimeoutMs: number): Promise( * single non-cloneable value can't masquerade as a worker death and exhaust a * slot's respawn budget here. */ + +/** + * Main-thread stall tracking (#2649). Near the V8 heap limit, multi-second + * mark-compact pauses freeze the main thread's message processing, so a + * healthy worker's `progress` messages sit unread and the worker LOOKS idle — + * the idle-timeout path then splits/retires it, and the respawn storm ends in + * "Replacement worker did not report ready". A 250ms unref'd heartbeat + * accumulates observed event-loop drift; the idle-timeout handler credits + * that stall once per job instead of retiring a worker the main thread + * starved. The floor filters scheduler jitter from real stalls. + */ +const HEARTBEAT_INTERVAL_MS = 250; +const HEARTBEAT_STALL_FLOOR_MS = 100; +/** Fraction of the idle-timeout budget that must be main-thread stall before + * the timeout is credited and re-armed instead of acted on. */ +const STALL_CREDIT_FRACTION = 0.5; + +export function startHeartbeatStallTracker(): { read: () => number; stop: () => void } { + let totalStallMs = 0; + let last = Date.now(); + const handle = setInterval(() => { + const now = Date.now(); + const drift = now - last - HEARTBEAT_INTERVAL_MS; + if (drift > HEARTBEAT_STALL_FLOOR_MS) totalStallMs += drift; + last = now; + }, HEARTBEAT_INTERVAL_MS); + handle.unref?.(); + return { read: () => totalStallMs, stop: () => clearInterval(handle) }; +} + +/** + * Per-worker V8 old-generation heap cap in MB (#2649). Without one, worker + * isolates inherit an unbounded default and a full pool can inflate process + * RSS past physical RAM on large repos. Half of RAM split across the pool, + * clamped to [512, 4096] MB — generous for the per-sub-batch working set + * (jobs are byte-budgeted), and a worker that does exceed it dies with a + * real heap error surfaced by the stderr-tail machinery + the + * quarantine/respawn path, instead of silently dragging the host into swap. + * `GITNEXUS_WORKER_HEAP_MB` overrides the formula. Exported for unit tests. + */ +export function resolveWorkerHeapCapMb(poolSize: number): number { + return ( + positiveInteger(process.env.GITNEXUS_WORKER_HEAP_MB) ?? + Math.min(4096, Math.max(512, Math.floor(effectiveRamBytes() / (1024 * 1024) / 2 / poolSize))) + ); +} + export const createWorkerPool = ( workerUrl: URL, poolSize?: number, @@ -957,6 +1012,24 @@ export const createWorkerPool = ( parsedFileStoreStoragePath || durableParsedFileStoragePath || pdg ? { parsedFileStoreStoragePath, durableParsedFileStoragePath, pdg, pdgMaxFunctionLines } : undefined; + const workerHeapCapMb = resolveWorkerHeapCapMb(size); + // The 512MB per-worker floor exists so a worker can parse anything real, + // but on a very small container a large pool of floored workers can still + // overcommit total memory (#2649 review). Behavior is unchanged — deaths + // are attributed and quarantine converges — but say so up front, with the + // two levers, instead of letting the operator discover it from worker OOMs. + const poolCommitMb = workerHeapCapMb * size; + const effectiveMb = Math.floor(effectiveRamBytes() / (1024 * 1024)); + if (poolCommitMb > 0.6 * effectiveMb) { + logger.warn( + { poolSize: size, workerHeapCapMb, effectiveMb }, + `Worker pool may overcommit memory: ${size} workers × ${workerHeapCapMb}MB heap cap exceeds 60% of the ${effectiveMb}MB available to this process. Reduce GITNEXUS_WORKER_POOL_SIZE or set GITNEXUS_WORKER_HEAP_MB.`, + ); + } + // #2649 stall probe: test seam wins; production uses the heartbeat tracker. + const stallTracker = options?.stallMsProbe + ? { read: options.stallMsProbe, stop: (): void => undefined } + : startHeartbeatStallTracker(); const spawnWorker = options?.workerFactory ?? ((url: URL) => @@ -976,7 +1049,7 @@ export const createWorkerPool = ( // nesting levels (far beyond any hand-written code); a deeper machine- // generated nest is still caught per-function (buildFunctionCfg's R4 // try/catch) and only that function's PDG is skipped, never a crash. - resourceLimits: { stackSizeMb: 16 }, + resourceLimits: { stackSizeMb: 16, maxOldGenerationSizeMb: workerHeapCapMb }, })); /** Spawn + wire stdio capture/forwarding in one step (used by all spawn sites). */ const spawnAndCapture = (url: URL): Worker => { @@ -1843,10 +1916,28 @@ export const createWorkerPool = ( maybeDone(); }; + let stallCreditUsed = false; + let stallAtArm = 0; const resetIdleTimer = () => { if (idleTimer) clearTimeout(idleTimer); + stallAtArm = stallTracker.read(); idleTimer = setTimeout(() => { if (!settled) { + // #2649: when at least STALL_CREDIT_FRACTION of the timeout + // window was main-thread stall (GC pressure near the heap + // limit), the worker's progress messages were starved, not + // absent — credit the stall once per job and re-arm instead + // of splitting/retiring a healthy worker. + const stallMs = stallTracker.read() - stallAtArm; + if (!stallCreditUsed && stallMs >= job.timeoutMs * STALL_CREDIT_FRACTION) { + stallCreditUsed = true; + logger.warn( + { workerIndex, stallMs: Math.round(stallMs), timeoutMs: job.timeoutMs }, + `Worker ${workerIndex} idle timeout overlapped a main-thread stall (GC pressure); re-arming once instead of retiring.`, + ); + resetIdleTimer(); + return; + } settled = true; cleanup(); inFlightProgress[workerIndex] = 0; @@ -2084,10 +2175,22 @@ export const createWorkerPool = ( // the `{type:'error'}` message, the event delivers a real Error whose // `.stack` is the worker-side frame — carry it so the surfaced reason // points at the actual failure site, not just `err.message` (#2068). - void recoverAndResume( - workerErrorReason(workerIndex, err.message, err.stack), - resolveExcludePaths(), - ); + // A worker dying on ITS OWN heap cap (#2649) must be attributable to + // that cap, not read as generic quarantine noise — name the cap and + // its override so an oversized-but-legitimate file (e.g. under a + // raised GITNEXUS_MAX_FILE_SIZE) is a one-env-var fix. + // The 'error' event does not guarantee a well-formed Error: the + // structured-clone failure path can deliver a value with no + // `message` — guard every property access or the handler itself + // throws and the pool hangs instead of recovering. + const isWorkerHeapOom = + (err as NodeJS.ErrnoException | undefined)?.code === 'ERR_WORKER_OUT_OF_MEMORY' || + (typeof err?.message === 'string' && + err.message.includes('ERR_WORKER_OUT_OF_MEMORY')); + const reason = isWorkerHeapOom + ? `${workerErrorReason(workerIndex, err.message, err.stack)} (worker hit its ${workerHeapCapMb}MB heap cap — raise with GITNEXUS_WORKER_HEAP_MB)` + : workerErrorReason(workerIndex, err.message, err.stack); + void recoverAndResume(reason, resolveExcludePaths()); } }; @@ -2154,6 +2257,7 @@ export const createWorkerPool = ( const terminate = async (): Promise => { terminated = true; + stallTracker.stop(); // Cancel any in-flight startup backoff so its ref'd timer doesn't keep the // event loop alive after terminate; each cancel resolves the awaiting sleep // and the slot loop then sees `terminated` and gives up (#1741). diff --git a/gitnexus/src/server/analyze-launch.ts b/gitnexus/src/server/analyze-launch.ts index b505cdfcf..720ebe7f8 100644 --- a/gitnexus/src/server/analyze-launch.ts +++ b/gitnexus/src/server/analyze-launch.ts @@ -23,6 +23,7 @@ import { registryPathEquals, } from '../storage/repo-manager.js'; import { logger } from '../core/logger.js'; +import { autoHeapCapMb } from '../core/ingestion/utils/effective-ram.js'; import type { JobManager } from './analyze-job.js'; import type { WorkerMessage } from './analyze-worker.js'; @@ -151,12 +152,20 @@ export function createLaunchAnalysisWorker(deps: LaunchDeps) { ? ['--import', pathToFileURL(_require.resolve('tsx/esm')).href] : []; + // Worker heap: 8192MB historical default, but never above what this + // machine/container actually has (#2649 review — a fixed 8192 inside a + // smaller cgroup limit died to the kernel with a misleading remedy). + // GITNEXUS_SERVER_ANALYZE_HEAP_MB overrides as an absolute value. + const envHeapMb = Number(process.env.GITNEXUS_SERVER_ANALYZE_HEAP_MB); + const workerHeapMb = + Number.isInteger(envHeapMb) && envHeapMb > 0 ? envHeapMb : Math.min(8192, autoHeapCapMb()); + const forkWorker = () => { const currentJob = jobManager.getJob(job.id); if (!currentJob || currentJob.status === 'complete' || currentJob.status === 'failed') return; const child = fork(workerPath, [], { - execArgv: [...tsxHookArgs, '--max-old-space-size=8192'], + execArgv: [...tsxHookArgs, `--max-old-space-size=${workerHeapMb}`], stdio: ['ignore', 'pipe', 'pipe', 'ipc'], }); diff --git a/gitnexus/test/integration/analyze-heap-oom-e2e.test.ts b/gitnexus/test/integration/analyze-heap-oom-e2e.test.ts index ea4d995f4..2176c7a76 100644 --- a/gitnexus/test/integration/analyze-heap-oom-e2e.test.ts +++ b/gitnexus/test/integration/analyze-heap-oom-e2e.test.ts @@ -18,6 +18,9 @@ const runAnalyzeWithForcedOom = (cwd: string, gitnexusHome: string) => stdio: ['pipe', 'pipe', 'pipe'], env: { ...process.env, + // This suite EXERCISES the heap respawn; the suite-wide + // GITNEXUS_MEMORY=off opt-out (vitest.config.ts) must not apply here. + GITNEXUS_MEMORY: '1', GITNEXUS_HOME: gitnexusHome, NODE_OPTIONS: '', GITNEXUS_TEST_RESPAWN_HEAP_MB: '32', diff --git a/gitnexus/test/unit/analyze-heap-respawn.test.ts b/gitnexus/test/unit/analyze-heap-respawn.test.ts index e384e460c..53b684f9b 100644 --- a/gitnexus/test/unit/analyze-heap-respawn.test.ts +++ b/gitnexus/test/unit/analyze-heap-respawn.test.ts @@ -15,8 +15,9 @@ vi.mock('v8', () => ({ }, })); -// Pin physical RAM to 16GB so the RAM-aware auto-cap (0.75 x RAM, clamped -// >= 16384) resolves deterministically to 16384 regardless of the host machine. +// Pin physical RAM to 16GB so the RAM-aware auto-cap (floor raised to 16384 +// but capped at 0.80 x RAM, #2649) resolves deterministically to 13107 +// regardless of the host machine. vi.mock('os', async () => { const actual = await vi.importActual('os'); const mocked = { ...actual, totalmem: () => 16 * 1024 * 1024 * 1024 }; @@ -75,6 +76,7 @@ describe('analyzeCommand heap respawn', () => { beforeEach(() => { initialNodeOptions = process.env.NODE_OPTIONS; + delete process.env.GITNEXUS_MEMORY; vi.resetModules(); spawnMock.mockReset(); getHeapStatisticsMock.mockReset(); @@ -114,9 +116,9 @@ describe('analyzeCommand heap respawn', () => { expect(spawnMock).toHaveBeenCalledTimes(1); const [, args, opts] = spawnMock.mock.calls[0]; - expect(args).toContain('--max-old-space-size=16384'); + expect(args).toContain('--max-old-space-size=13107'); expect(args).toContain('--max-semi-space-size=128'); - expect(opts.env.NODE_OPTIONS).toContain('--max-old-space-size=16384'); + expect(opts.env.NODE_OPTIONS).toContain('--max-old-space-size=13107'); expect(opts.env.NODE_OPTIONS).toContain('--max-semi-space-size=128'); expect(opts.env.GITNEXUS_RESPAWN_PROGRESS_TTY).toBe('1'); }); @@ -146,6 +148,129 @@ describe('analyzeCommand heap respawn', () => { expect(spawnMock).not.toHaveBeenCalled(); }); + it('re-execs with the auto cap when ambient NODE_OPTIONS pins a smaller heap (#2649)', async () => { + process.env.NODE_OPTIONS = '--max-old-space-size=4096'; + getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 4096 * 1024 * 1024 }); + mockSpawnExit(); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, {}); + cap.restore(); + + expect(spawnMock).toHaveBeenCalledTimes(1); + const [, args, opts] = spawnMock.mock.calls[0]; + expect(args).toContain('--max-old-space-size=13107'); + // The auto flag is appended after the ambient value, so V8's + // later-flag-wins semantics resolve to the larger cap. + expect(opts.env.NODE_OPTIONS.indexOf('--max-old-space-size=13107')).toBeGreaterThan( + opts.env.NODE_OPTIONS.indexOf('--max-old-space-size=4096'), + ); + const warn = cap.records().find((r) => r.msg.includes('pins the heap to 4096MB')); + expect(warn?.msg).toContain('Re-running analyze with the larger auto-sized cap'); + }); + + it('honors GITNEXUS_MEMORY=off: keeps the small ambient heap, silently (#2649)', async () => { + process.env.NODE_OPTIONS = '--max-old-space-size=4096'; + process.env.GITNEXUS_MEMORY = 'off'; + getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 4096 * 1024 * 1024 }); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand('/__gitnexus_nonexistent__', {}); + cap.restore(); + + expect(spawnMock).not.toHaveBeenCalled(); + // Explicit opt-out stays quiet: stderr-sensitive consumers (e2e + // harnesses, scripts) rely on no extra warning here. + const warns = cap.records().filter((r) => r.msg.includes('pins the heap')); + expect(warns).toEqual([]); + }); + + it('preserves parent execArgv (e.g. a tsx loader) in the respawned child argv (#2649)', async () => { + delete process.env.NODE_OPTIONS; + restoreStderrIsTTY = setStreamIsTTY(process.stderr, true); + getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 512 * 1024 * 1024 }); + mockSpawnExit(); + + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, {}); + + expect(spawnMock).toHaveBeenCalledTimes(1); + const [, args] = spawnMock.mock.calls[0]; + // The child argv must start with the parent's node flags so + // loader-launched CLIs (node --import tsx src/cli/index.ts) survive the + // respawn; our heap flags follow and win via later-flag-wins. + expect(args.slice(0, process.execArgv.length)).toEqual(process.execArgv); + }); + + it('parseMaxOldSpaceMb: last occurrence wins, absent and malformed values are null', async () => { + const { parseMaxOldSpaceMb } = await import('../../src/cli/analyze.js'); + expect(parseMaxOldSpaceMb('--max-old-space-size=4096 --max-old-space-size=8192')).toBe(8192); + expect(parseMaxOldSpaceMb('--max-semi-space-size=128')).toBeNull(); + expect(parseMaxOldSpaceMb('')).toBeNull(); + expect(parseMaxOldSpaceMb('--max-old-space-size=0')).toBeNull(); + // V8 treats - and _ interchangeably in flag names, and Node accepts a + // space-separated value in NODE_OPTIONS; every spelling of the pin must + // be honored instead of silently overridden. + expect(parseMaxOldSpaceMb('--max_old_space_size=4096')).toBe(4096); + expect(parseMaxOldSpaceMb('--max-old-space-size 4096')).toBe(4096); + expect(parseMaxOldSpaceMb('--max-old-space-size --other-flag')).toBeNull(); + }); + + it('GITNEXUS_MEMORY=off also disables the default (unpinned) respawn (#2649 review)', async () => { + delete process.env.NODE_OPTIONS; + process.env.GITNEXUS_MEMORY = 'off'; + getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 512 * 1024 * 1024 }); + + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand('/__gitnexus_nonexistent__', {}); + + expect(spawnMock).not.toHaveBeenCalled(); + }); + + it('an explicit per-invocation execArgv heap flag always wins (no respawn)', async () => { + delete process.env.NODE_OPTIONS; + getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 512 * 1024 * 1024 }); + const execArgvDesc = Object.getOwnPropertyDescriptor(process, 'execArgv'); + Object.defineProperty(process, 'execArgv', { + configurable: true, + value: ['--max-old-space-size=2048'], + }); + try { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand('/__gitnexus_nonexistent__', {}); + expect(spawnMock).not.toHaveBeenCalled(); + } finally { + if (execArgvDesc) Object.defineProperty(process, 'execArgv', execArgvDesc); + } + }); + + it('does not replay --inspect flags into the respawned child (debug-port clash)', async () => { + delete process.env.NODE_OPTIONS; + getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 512 * 1024 * 1024 }); + mockSpawnExit(); + const execArgvDesc = Object.getOwnPropertyDescriptor(process, 'execArgv'); + Object.defineProperty(process, 'execArgv', { + configurable: true, + value: ['--inspect', '--inspect-brk=9230', '--enable-source-maps'], + }); + try { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, {}); + expect(spawnMock).toHaveBeenCalledTimes(1); + const [, args] = spawnMock.mock.calls[0]; + expect({ + inspectFlags: args.filter((a: string) => a.startsWith('--inspect')), + keepsOtherFlags: args.includes('--enable-source-maps'), + }).toEqual({ inspectFlags: [], keepsOtherFlags: true }); + } finally { + if (execArgvDesc) Object.defineProperty(process, 'execArgv', execArgvDesc); + } + }); + it('prints heap guidance when respawned analyze exits with likely OOM', async () => { delete process.env.NODE_OPTIONS; getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 512 * 1024 * 1024 }); @@ -164,7 +289,7 @@ describe('analyzeCommand heap respawn', () => { .find((r) => r.msg.includes('Analysis likely ran out of memory')); expect(oomGuidance).toBeDefined(); const msg = oomGuidance?.msg ?? ''; - expect(msg).toContain('auto-sized to 16384MB'); + expect(msg).toContain('auto-sized to 13107MB'); expect(msg).toContain('NODE_OPTIONS="--max-old-space-size="'); expect(msg).toContain('[your-args]'); expect(msg).toContain('native crash unrelated to heap size'); @@ -289,10 +414,22 @@ describe('computeHeapCapMb (RAM-aware auto heap cap)', () => { expect(computeHeapCapMb(31 * GB, null)).toBe(23808); }); - it('clamps to the 16384 floor on small boxes', async () => { + it('keeps the cap below RAM on small boxes instead of the old >=RAM floor (#2649)', async () => { const { computeHeapCapMb } = await import('../../src/cli/analyze.js'); - // 8GB -> 0.75 * 8192 = 6144 -> clamped to 16384 - expect(computeHeapCapMb(8 * GB, null)).toBe(16384); + // 8GB -> floor wins the max (16384) but is capped to 0.80 * 8192 = 6553 + expect(computeHeapCapMb(8 * GB, null)).toBe(6553); + }); + + it('caps a 16GB box at 0.80x RAM, below physical memory (#2649)', async () => { + const { computeHeapCapMb } = await import('../../src/cli/analyze.js'); + // 16GB -> max(16384, 12288) = 16384 -> min(16384, floor(0.80 * 16384)) = 13107 + expect(computeHeapCapMb(16 * GB, null)).toBe(13107); + }); + + it('lets the 0.75x rule win once RAM clears the floor region', async () => { + const { computeHeapCapMb } = await import('../../src/cli/analyze.js'); + // 24GB -> max(16384, 18432) = 18432 -> min(18432, 19660) = 18432 + expect(computeHeapCapMb(24 * GB, null)).toBe(18432); }); it('ignores the unconstrained sentinel from constrainedMemory()', async () => { @@ -303,8 +440,16 @@ describe('computeHeapCapMb (RAM-aware auto heap cap)', () => { it('honors a real cgroup cap smaller than physical RAM', async () => { const { computeHeapCapMb } = await import('../../src/cli/analyze.js'); - // min(31, 12) = 12GB -> 0.75 * 12288 = 9216 -> clamped to 16384 - expect(computeHeapCapMb(31 * GB, 12 * GB)).toBe(16384); + // min(31, 12) = 12GB effective -> capped to 0.80 * 12288 = 9830, not the 16384 floor + expect(computeHeapCapMb(31 * GB, 12 * GB)).toBe(9830); + }); + + it('never returns a cap at or above effective RAM', async () => { + const { computeHeapCapMb } = await import('../../src/cli/analyze.js'); + const ramsGb = [4, 8, 12, 16, 20, 24, 32, 48, 64]; + const caps = ramsGb.map((gb) => computeHeapCapMb(gb * GB, null)); + const belowRam = caps.map((cap, i) => cap < ramsGb[i] * 1024); + expect(belowRam).toEqual(ramsGb.map(() => true)); }); it('uses a large cgroup cap when it exceeds the floor', async () => { diff --git a/gitnexus/test/unit/parse-impl-heap-guard-pipeline.test.ts b/gitnexus/test/unit/parse-impl-heap-guard-pipeline.test.ts new file mode 100644 index 000000000..d2ecf9c72 --- /dev/null +++ b/gitnexus/test/unit/parse-impl-heap-guard-pipeline.test.ts @@ -0,0 +1,136 @@ +/** + * #2649 review — pipeline-level coverage for the parse-phase heap guardrails. + * + * The pure predicates are covered in parse-impl-heap-guard.test.ts; these + * tests pin the WIRING inside runChunkedParseAndResolve: the mid-loop abort + * actually rejects the parse with the remedy message (nothing en route may + * swallow or remap it — the #2441 exit-0 bug class), and the preflight + * projection is computed from PARSEABLE files, not total scanned files (the + * miscalibration fixed on this branch). + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; + +const getHeapStatisticsMock = vi.hoisted(() => vi.fn()); +vi.mock('node:v8', async () => { + const actual = await vi.importActual('node:v8'); + const mocked = { ...actual, getHeapStatistics: getHeapStatisticsMock }; + return { ...mocked, default: mocked }; +}); + +import { createKnowledgeGraph } from '../../src/core/graph/graph.js'; +import { + projectParseHeapNeedBytes, + runChunkedParseAndResolve, +} from '../../src/core/ingestion/pipeline-phases/parse-impl.js'; +import { _captureLogger } from '../../src/core/logger.js'; + +const MB = 1024 * 1024; + +let repoDir: string; +let workerStubPath: string; +let memoryUsageSpy: ReturnType | undefined; + +beforeEach(() => { + delete process.env.GITNEXUS_MEMORY; + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-heap-guard-pipeline-')); + fs.mkdirSync(path.join(repoDir, 'src'), { recursive: true }); + // The pool validates the worker script's existence up front; the abort test + // never dispatches, and the preflight test parses one real file. + workerStubPath = path.join(repoDir, 'fake-worker.js'); + fs.writeFileSync(workerStubPath, '// worker stub for createWorkerPool'); +}); + +afterEach(() => { + memoryUsageSpy?.mockRestore(); + memoryUsageSpy = undefined; + delete process.env.GITNEXUS_MEMORY; + fs.rmSync(repoDir, { recursive: true, force: true }); +}); + +const writeFixture = (rel: string, content: string): { path: string; size: number } => { + const full = path.join(repoDir, rel); + fs.writeFileSync(full, content); + return { path: rel, size: fs.statSync(full).size }; +}; + +describe('#2649 heap guardrails wired into runChunkedParseAndResolve', () => { + it('mid-loop guard rejects the parse with the actionable remedy message', async () => { + const file = writeFixture('src/a.ts', 'export function a() { return 1; }\n'); + // 1GB limit with 95% "in use": above the 92% abort threshold. + getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 1024 * MB }); + memoryUsageSpy = vi.spyOn(process, 'memoryUsage').mockReturnValue({ + rss: 0, + heapTotal: 1024 * MB, + heapUsed: 973 * MB, + external: 0, + arrayBuffers: 0, + }); + + const graph = createKnowledgeGraph(); + await expect( + runChunkedParseAndResolve(graph, [file], [file.path], 1, repoDir, Date.now(), () => {}, { + workerUrlForTest: pathToFileURL(workerStubPath), + workerPoolSize: 1, + }), + ).rejects.toThrow(/Analyze stopped before running out of memory/); + }); + + it('preflight warn projects from PARSEABLE files only and names the parseable count', async () => { + // Mock the heap limit relative to what ONE parseable file actually projects, + // so this stays a test of the WARN BEHAVIOUR rather than of the projection + // constant. Hard-coding 150_000 tied it to PROJECTED_HEAP_BYTES_PER_NODE = + // 2250; recalibrating that constant for streamed emit (#2680) dropped the + // projection to 0.80 of the limit and the warn silently stopped firing. + // Deriving the limit keeps the ratio at 0.90 — above the 0.85 threshold — + // whatever the constant becomes. + process.env.GITNEXUS_MEMORY = 'off'; + getHeapStatisticsMock.mockReturnValue({ + heap_size_limit: Math.floor(projectParseHeapNeedBytes(1) / 0.9), + }); + + const parseable = writeFixture('src/b.ts', 'export function b() { return 2; }\n'); + const unparseable = writeFixture('src/data.zzz9', 'not source code\n'); + + const cap = _captureLogger(); + const graph = createKnowledgeGraph(); + try { + await runChunkedParseAndResolve( + graph, + [parseable, unparseable], + [parseable.path, unparseable.path], + 2, + repoDir, + Date.now(), + () => {}, + { + workerUrlForTest: pathToFileURL( + path.resolve( + __dirname, + '..', + '..', + 'dist', + 'core', + 'ingestion', + 'workers', + 'parse-worker.js', + ), + ), + workerPoolSize: 1, + }, + ); + } finally { + cap.restore(); + } + + const warn = cap.records().find((r) => r.msg.includes('Large repository')); + // "analyzing 1 files" — the parseable count, not the 2 scanned files. + expect({ + fired: warn !== undefined, + parseableBasis: warn?.msg.includes('analyzing 1 files') ?? false, + }).toEqual({ fired: true, parseableBasis: true }); + }); +}); diff --git a/gitnexus/test/unit/parse-impl-heap-guard.test.ts b/gitnexus/test/unit/parse-impl-heap-guard.test.ts new file mode 100644 index 000000000..89a535d41 --- /dev/null +++ b/gitnexus/test/unit/parse-impl-heap-guard.test.ts @@ -0,0 +1,87 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Pin physical RAM to 32GB so the remedy branch (auto cap = 24GB) resolves +// deterministically regardless of the host machine. +vi.mock('os', async () => { + const actual = await vi.importActual('os'); + const mocked = { ...actual, totalmem: () => 32 * 1024 * 1024 * 1024 }; + return { ...mocked, default: mocked }; +}); + +import { + heapPressureRemedy, + projectParseHeapNeedBytes, + shouldAbortForHeapPressure, +} from '../../src/core/ingestion/pipeline-phases/parse-impl.js'; + +const GB = 1024 * 1024 * 1024; + +const setConstrainedMemory = (value: number): (() => void) => { + const desc = Object.getOwnPropertyDescriptor(process, 'constrainedMemory'); + Object.defineProperty(process, 'constrainedMemory', { configurable: true, value: () => value }); + return () => { + if (desc) Object.defineProperty(process, 'constrainedMemory', desc); + else delete (process as { constrainedMemory?: unknown }).constrainedMemory; + }; +}; + +describe('#2649 parse-phase heap guardrails', () => { + let initialGuard: string | undefined; + let restoreConstrained: (() => void) | undefined; + + beforeEach(() => { + initialGuard = process.env.GITNEXUS_MEMORY; + delete process.env.GITNEXUS_MEMORY; + // Unconstrained by default so the mocked 32GB totalmem governs. + restoreConstrained = setConstrainedMemory(0); + }); + + afterEach(() => { + if (initialGuard === undefined) delete process.env.GITNEXUS_MEMORY; + else process.env.GITNEXUS_MEMORY = initialGuard; + restoreConstrained?.(); + restoreConstrained = undefined; + }); + + it('projects kernel-scale repos far past the 4GB default heap and small repos well under it', () => { + // 94,773 files x 55 nodes x 2250 bytes ≈ 11.7GB (the measured #2649 case); + // 2,000 files ≈ 236MB. + expect({ + kernelExceeds4Gb: projectParseHeapNeedBytes(94773) > 4 * GB, + smallRepoUnder1Gb: projectParseHeapNeedBytes(2000) < 1 * GB, + }).toEqual({ kernelExceeds4Gb: true, smallRepoUnder1Gb: true }); + }); + + it('aborts above 92% of the heap limit and not below it', () => { + const limit = 4 * GB; + expect([0.91, 0.93].map((f) => shouldAbortForHeapPressure(limit * f, limit))).toEqual([ + false, + true, + ]); + }); + + it('GITNEXUS_MEMORY=0 disables the abort entirely', () => { + process.env.GITNEXUS_MEMORY = 'off'; + const limit = 4 * GB; + expect(shouldAbortForHeapPressure(limit * 0.99, limit)).toBe(false); + }); + + it('points at the NODE_OPTIONS pin when the machine has more memory to give', () => { + // 4GB limit on a 32GB machine (auto cap 24GB): the pin is the problem. + expect(heapPressureRemedy(4 * GB)).toContain('re-run without the --max-old-space-size'); + }); + + it('points at scope or hardware when the machine is the ceiling', () => { + // 23GB limit on a 32GB machine (~auto cap): nothing more to unlock locally. + expect(heapPressureRemedy(23 * GB)).toContain('.gitnexusignore'); + }); + + it('remedy respects a real cgroup limit: a memory-limited container is never told to "drop the pin" (#2649 review)', () => { + // 8GB cgroup limit on the mocked 32GB host, heap already sized to the + // container (~6.5GB): raw totalmem would claim "more memory available"; + // the container is actually at its ceiling. + restoreConstrained?.(); + restoreConstrained = setConstrainedMemory(8 * GB); + expect(heapPressureRemedy(6.5 * GB)).toContain('.gitnexusignore'); + }); +}); diff --git a/gitnexus/test/unit/worker-pool-resource-limits.test.ts b/gitnexus/test/unit/worker-pool-resource-limits.test.ts new file mode 100644 index 000000000..94ea73346 --- /dev/null +++ b/gitnexus/test/unit/worker-pool-resource-limits.test.ts @@ -0,0 +1,166 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; + +// Pin physical RAM to 32GB so the half-of-RAM-per-worker formula resolves +// deterministically regardless of the host machine. +vi.mock('os', async () => { + const actual = await vi.importActual('os'); + const mocked = { ...actual, totalmem: () => 32 * 1024 * 1024 * 1024 }; + return { ...mocked, default: mocked }; +}); + +// Capture the exact options the pool's PRODUCTION factory passes to the +// Worker constructor — the formula alone doesn't prove the wiring, and a +// typo'd resourceLimits key would silently uncap workers again (#2649). +// vi.mock factories are hoisted above imports, so the capture array must be +// hoisted too and EventEmitter imported inside the factory. +const workerCtorOptions = vi.hoisted(() => [] as unknown[]); +vi.mock('node:worker_threads', async () => { + const actual = await vi.importActual('node:worker_threads'); + const { EventEmitter } = await import('node:events'); + class CapturingWorker extends EventEmitter { + private currentPaths: string[] = []; + constructor(_url: unknown, options: unknown) { + super(); + workerCtorOptions.push(options); + queueMicrotask(() => this.emit('message', { type: 'ready' })); + } + postMessage(msg: unknown): void { + if (msg === null || typeof msg !== 'object') return; + const type = (msg as { type?: unknown }).type; + if (type === 'sub-batch') { + const files = (msg as { files?: Array<{ path: string }> }).files ?? []; + this.currentPaths = files.map((file) => file.path); + queueMicrotask(() => { + this.emit('message', { type: 'progress', filesProcessed: this.currentPaths.length }); + this.emit('message', { type: 'sub-batch-done' }); + }); + return; + } + if (type === 'flush') { + const paths = this.currentPaths.slice(); + queueMicrotask(() => this.emit('message', { type: 'result', data: { paths } })); + } + } + async terminate(): Promise { + this.emit('exit', 0); + return 0; + } + unref(): void {} + } + return { ...actual, Worker: CapturingWorker }; +}); + +const setConstrainedMemory = (value: number): (() => void) => { + const desc = Object.getOwnPropertyDescriptor(process, 'constrainedMemory'); + Object.defineProperty(process, 'constrainedMemory', { configurable: true, value: () => value }); + return () => { + if (desc) Object.defineProperty(process, 'constrainedMemory', desc); + else delete (process as { constrainedMemory?: unknown }).constrainedMemory; + }; +}; + +describe('resolveWorkerHeapCapMb (#2649 per-worker heap cap)', () => { + let initialOverride: string | undefined; + let restoreConstrained: (() => void) | undefined; + + beforeEach(() => { + initialOverride = process.env.GITNEXUS_WORKER_HEAP_MB; + delete process.env.GITNEXUS_WORKER_HEAP_MB; + // Unconstrained by default so the mocked 32GB totalmem governs. + restoreConstrained = setConstrainedMemory(0); + workerCtorOptions.length = 0; + vi.resetModules(); + }); + + afterEach(() => { + if (initialOverride === undefined) delete process.env.GITNEXUS_WORKER_HEAP_MB; + else process.env.GITNEXUS_WORKER_HEAP_MB = initialOverride; + restoreConstrained?.(); + restoreConstrained = undefined; + }); + + it('splits half of RAM across the pool, clamped to the 4096 ceiling', async () => { + const { resolveWorkerHeapCapMb } = + await import('../../src/core/ingestion/workers/worker-pool.js'); + // 32GB -> half = 16384MB; /16 workers = 1024; /4 workers = 4096 (at ceiling); + // /2 workers = 8192 -> clamped to 4096. + expect([16, 4, 2].map((n) => resolveWorkerHeapCapMb(n))).toEqual([1024, 4096, 4096]); + }); + + it('never drops below the 512MB floor on small shares', async () => { + const { resolveWorkerHeapCapMb } = + await import('../../src/core/ingestion/workers/worker-pool.js'); + // 32GB half-share across 64 workers = 256 -> floored to 512. + expect(resolveWorkerHeapCapMb(64)).toBe(512); + }); + + it('GITNEXUS_WORKER_HEAP_MB overrides the formula', async () => { + process.env.GITNEXUS_WORKER_HEAP_MB = '768'; + const { resolveWorkerHeapCapMb } = + await import('../../src/core/ingestion/workers/worker-pool.js'); + expect([1, 16].map((n) => resolveWorkerHeapCapMb(n))).toEqual([768, 768]); + }); + + it('warns when a floored pool would overcommit a tiny container (#2649 review)', async () => { + // 2GB cgroup limit, pool of 8: every worker floors at 512MB, so the pool + // may commit 4096MB against a 2048MB container — the warn must name it. + restoreConstrained?.(); + restoreConstrained = setConstrainedMemory(2 * 1024 * 1024 * 1024); + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-worker-overcommit-')); + const workerPath = path.join(tempDir, 'fake-worker.js'); + fs.writeFileSync(workerPath, '// fake worker path for createWorkerPool'); + try { + const { _captureLogger } = await import('../../src/core/logger.js'); + const { createWorkerPool } = await import('../../src/core/ingestion/workers/worker-pool.js'); + const cap = _captureLogger(); + const pool = createWorkerPool(pathToFileURL(workerPath) as URL, 8, { shutdownDrainMs: 25 }); + await pool.terminate(); + cap.restore(); + const warn = cap.records().find((r) => r.msg.includes('may overcommit memory')); + expect(warn?.msg).toContain('GITNEXUS_WORKER_POOL_SIZE'); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }); + + it('honors a real cgroup limit instead of host RAM (#2649 review — container overcommit)', async () => { + // 8GB cgroup limit on the mocked 32GB host, pool of 4: the cap must come + // from the container (8192/2/4 = 1024), not the host (32768/2/4 = 4096 — + // which would let one worker outgrow a quarter of the whole container). + restoreConstrained?.(); + restoreConstrained = setConstrainedMemory(8 * 1024 * 1024 * 1024); + const { resolveWorkerHeapCapMb } = + await import('../../src/core/ingestion/workers/worker-pool.js'); + expect(resolveWorkerHeapCapMb(4)).toBe(1024); + }); + + it('wires the cap into the production Worker resourceLimits (#2649 review)', async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-worker-limits-')); + const workerPath = path.join(tempDir, 'fake-worker.js'); + fs.writeFileSync(workerPath, '// fake worker path for createWorkerPool'); + try { + const { createWorkerPool, resolveWorkerHeapCapMb } = + await import('../../src/core/ingestion/workers/worker-pool.js'); + const pool = createWorkerPool(pathToFileURL(workerPath) as URL, 2, { + shutdownDrainMs: 25, + }); + try { + await pool.dispatch<{ path: string; content: string }, { paths: string[] }>([ + { path: 'src/a.ts', content: 'const a = 1;' }, + ]); + } finally { + await pool.terminate(); + } + expect(workerCtorOptions.length).toBeGreaterThan(0); + expect(workerCtorOptions[0]).toMatchObject({ + resourceLimits: { stackSizeMb: 16, maxOldGenerationSizeMb: resolveWorkerHeapCapMb(2) }, + }); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }); +}); diff --git a/gitnexus/test/unit/worker-pool-stall-credit.test.ts b/gitnexus/test/unit/worker-pool-stall-credit.test.ts new file mode 100644 index 000000000..b57b0a70c --- /dev/null +++ b/gitnexus/test/unit/worker-pool-stall-credit.test.ts @@ -0,0 +1,150 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { EventEmitter } from 'node:events'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; + +import { createWorkerPool } from '../../src/core/ingestion/workers/worker-pool.js'; +import { _captureLogger } from '../../src/core/logger.js'; + +// First worker never answers its sub-batch (looks idle); every later worker +// completes normally so the dispatch still resolves after the retire path. +class StallThenHealthyWorker extends EventEmitter { + static instances: StallThenHealthyWorker[] = []; + + readonly id: number; + private currentPaths: string[] = []; + + constructor() { + super(); + this.id = StallThenHealthyWorker.instances.length; + StallThenHealthyWorker.instances.push(this); + queueMicrotask(() => this.emit('message', { type: 'ready' })); + } + + postMessage(msg: unknown): void { + if (msg === null || typeof msg !== 'object') return; + const type = (msg as { type?: unknown }).type; + if (type === 'sub-batch') { + const files = (msg as { files?: Array<{ path: string }> }).files ?? []; + this.currentPaths = files.map((file) => file.path); + if (this.id === 0) return; + queueMicrotask(() => { + this.emit('message', { type: 'progress', filesProcessed: this.currentPaths.length }); + this.emit('message', { type: 'sub-batch-done' }); + }); + return; + } + if (type === 'flush') { + const paths = this.currentPaths.slice(); + queueMicrotask(() => this.emit('message', { type: 'result', data: { paths } })); + } + } + + async terminate(): Promise { + this.emit('exit', 0); + return 0; + } + + unref(): void {} +} + +let tempDir: string; +let workerUrl: URL; + +beforeEach(() => { + StallThenHealthyWorker.instances = []; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-worker-stall-credit-')); + const workerPath = path.join(tempDir, 'fake-worker.js'); + fs.writeFileSync(workerPath, '// fake worker path for createWorkerPool'); + workerUrl = pathToFileURL(workerPath) as URL; +}); + +afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); +}); + +const dispatchWithProbe = async (stallMsProbe: () => number) => { + const cap = _captureLogger(); + const pool = createWorkerPool(workerUrl, 1, { + subBatchIdleTimeoutMs: 30, + maxTimeoutRetries: 1, + timeoutBackoffFactor: 2, + shutdownDrainMs: 25, + stallMsProbe, + workerFactory: () => + new StallThenHealthyWorker() as unknown as import('node:worker_threads').Worker, + }); + try { + const results = await pool.dispatch<{ path: string; content: string }, { paths: string[] }>([ + { path: 'src/starved.ts', content: 'const x = 1;' }, + ]); + return { results, records: cap.records() }; + } finally { + cap.restore(); + await pool.terminate(); + } +}; + +describe('worker pool GC-stall credit (#2649)', () => { + it('credits a main-thread stall >= half the budget with one re-arm before retiring', async () => { + // Monotonic fake stall clock: every read advances 20ms, so each armed + // 30ms window observes ~tens of ms of "stall" — always above the 15ms + // credit threshold. Only ONE credit may be spent regardless. + let stall = 0; + const { results, records } = await dispatchWithProbe(() => { + stall += 20; + return stall; + }); + + expect(results).toEqual([{ paths: ['src/starved.ts'] }]); + const creditWarns = records.filter((r) => + r.msg.includes('overlapped a main-thread stall (GC pressure); re-arming once'), + ); + const timeoutWarns = records.filter((r) => r.msg.includes('parse job idle timeout')); + expect({ credits: creditWarns.length, timeoutsAtLeast: timeoutWarns.length >= 1 }).toEqual({ + credits: 1, + timeoutsAtLeast: true, + }); + }); + + it('does not credit when the main thread was responsive (probe reads zero stall)', async () => { + const { results, records } = await dispatchWithProbe(() => 0); + + expect(results).toEqual([{ paths: ['src/starved.ts'] }]); + const creditWarns = records.filter((r) => + r.msg.includes('overlapped a main-thread stall (GC pressure); re-arming once'), + ); + expect(creditWarns).toEqual([]); + }); +}); + +describe('startHeartbeatStallTracker (#2649 review — the production probe itself)', () => { + it('accumulates observed stalls, ignores on-time ticks, and freezes after stop()', async () => { + const { startHeartbeatStallTracker } = + await import('../../src/core/ingestion/workers/worker-pool.js'); + vi.useFakeTimers(); + try { + const tracker = startHeartbeatStallTracker(); + // Two on-time ticks: zero drift, nothing accumulates. + vi.advanceTimersByTime(500); + const afterOnTime = tracker.read(); + // Simulate a ~2s main-thread stall: jump the wall clock, then let the + // delayed tick observe the drift. + vi.setSystemTime(Date.now() + 2000); + vi.advanceTimersByTime(250); + const afterStall = tracker.read(); + tracker.stop(); + vi.setSystemTime(Date.now() + 2000); + vi.advanceTimersByTime(500); + expect({ + afterOnTime, + stallSeen: afterStall >= 1500, + frozenAfterStop: tracker.read() === afterStall, + }).toEqual({ afterOnTime: 0, stallSeen: true, frozenAfterStop: true }); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/gitnexus/vitest.config.ts b/gitnexus/vitest.config.ts index 84922760e..d5326509e 100644 --- a/gitnexus/vitest.config.ts +++ b/gitnexus/vitest.config.ts @@ -9,6 +9,12 @@ export default defineConfig({ pool: 'forks', globals: true, teardownTimeout: 3000, + // E2E harnesses pin a small NODE_OPTIONS heap so spawned CLI children + // stay light; without this opt-out the #2649 auto-heap override would + // respawn every such child with a RAM-sized cap. Children inherit it via + // the harnesses' `{ ...process.env }` spreads. Tests that exercise the + // respawn behavior itself delete GITNEXUS_MEMORY in their own setup. + env: { GITNEXUS_MEMORY: 'off' }, // N-API destructors can crash worker forks on macOS during process exit. // This is independent of the QueryResult lifetime fix in @ladybugdb/core 0.15.2 — // it's a vitest forks + native addon interaction where destructors run in