Skip to content

fix(docker): re-derive build memory budget from measured per-process RSS (#7518) - #11663

Merged
diegosouzapw merged 1 commit into
release/v3.8.51from
fix/7518-docker-build-memory-budget
Aug 26, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.51from
fix/7518-docker-build-memory-budget

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Refs #7518

⚠️ base-red inherited: #11449 — ESLint errors (5), docs-sync migrations count

Scope

This PR addresses only the residual, currently-live defect in #7518 — the Docker
build's memory budget model, not the original complaint (default branch / Docker Hub
image), which the owner's 2026-08-23 comment already confirmed is fixed and out of
scope here.

Root cause

tests/unit/docker-build-memory-budget.test.ts (added by PR #11419, merged
2026-08-24) modeled the Docker build's worst-case memory as
heapMb + workers * WORKER_PEAK_MB with WORKER_PEAK_MB = 2560 — a value the
test's own comment labeled an inference ("7 workers didn't fit" → per-worker
peak "somewhere north of ~1.8 GB"), not a measurement. It also implicitly assumed
the parent next build process's RSS tracked OMNIROUTE_BUILD_MEMORY_MB (the V8
heap ceiling).

The issue owner's own live VPS reproduction (2026-08-24 04:49 UTC) measured the
real number directly via dmesg's OOM-killer report:
next-build (v16) ... anon-rss:4522744kB (~4.5 GB) per process, independent
of the NODE_OPTIONS --max-old-space-size V8 heap ceiling — Turbopack itself is
native/Rust and compiles outside the V8 heap, and the figure applies to the
parent process too, not just workers.

Recomputing the budget with the measured ~4.5 GB/process figure against the
current default (OMNIROUTE_BUILD_WORKERS=3, i.e. 1 parent + 2 workers) gives a
worst case of 3 * 4500 = 13500 MB, which exceeds the 12288 MB (75% of a 16 GB
GitHub Actions runner) budget — exactly matching the still-live "Publish to
Docker Hub" failures (cannot allocate memory at "Collecting page data using 2
workers") even after #11419 merged.

Fix

  • Lowered Dockerfile's ARG OMNIROUTE_BUILD_WORKERS default 3 -> 2 (1 parent +
    1 worker = 2 processes × 4.5 GB = 9 GB, fits the 12.288 GB budget with headroom).
  • Re-derived tests/unit/docker-build-memory-budget.test.ts's budget model on the
    measured ~4.5 GB/process figure, applied uniformly to every process (parent +
    workers) instead of conflating the parent process with the V8 heap ceiling.
  • Updated docs/guides/DOCKER_GUIDE.md's build-arg table with the corrected
    default and the measured-RSS rationale.
  • Checked Dockerfile.bun: its builder stage runs bun run --quiet build
    directly with no OMNIROUTE_BUILD_WORKERS/CIRCLE_NODE_TOTAL wiring, so the
    worker-pool pattern this issue is about does not apply there — no change needed.

Regression test (TDD, Hard Rule #18)

Re-confirmed RED against the unfixed code with a repro test using the plan-file's
exact model (measured RSS applied per-process, including the parent):

✖ [repro #7518] worker pool sized off the MEASURED per-process RSS still fits a 16 GB runner
  AssertionError: 3 processes (1 parent + 2 workers) x 4500 MB measured RSS
  = 13500 MB exceeds the 12288 MB budget on a 16384 MB runner.

After the fix, tests/unit/docker-build-memory-budget.test.ts (re-derived on the
measured figure, now the permanent regression guard) passes GREEN:

✔ the Docker build's worker pool is derived from OMNIROUTE_BUILD_WORKERS
✔ worker count × measured per-process RSS fits a 16 GB GitHub runner
✔ the worker pool does not oversubscribe the runner's 4 vCPU
ℹ tests 3 / pass 3 / fail 0

All 8 sibling Dockerfile-touching test files (docker-base-path-patch,
docker-ensure-base-path, docker-healthcheck-3151, docker-healthcheck-base-path,
docker-llmlingua-optionals-9166, dockerfile-base-path-arg,
dockerfile-better-sqlite3-node-gyp-6700, dockerfile-build-heap-4076,
dockerfile-dashboard-embed-arg-10273, dockerfile-npm-bundled-cve-patch,
tls-client-node-docker-binary-7802) still pass: 41/41.

Gates run

  • node scripts/check/check-file-size.mjs — OK
  • node scripts/check/check-changelog-integrity.mjs — OK
  • node scripts/check/check-docs-sync.mjs — PASS
  • npx eslint --suppressions-location config/quality/eslint-suppressions.json tests/unit/docker-build-memory-budget.test.ts — clean, exit 0
  • node scripts/check/check-complexity.mjs / check-cognitive-complexity.mjs — scan only src/, open-sse/, electron/, bin/; this diff touches none of those paths, so these gates are structurally unaffected
  • npm run typecheck:core — the changed file is not in tsconfig.typecheck-core.json's curated file list; the test file itself type-checks cleanly under tsx (it ran successfully via node --import tsx/esm --test)
  • Husky pre-commit hooks (lint-staged, check-docs-sync, any-budget, tracked-artifacts) passed on commit

Remaining follow-up (not in this PR)

Per the plan-file's own checklist: after merging, watch the next few real
"Publish to Docker Hub" runs on the default release branch to confirm the OOM is
actually gone in production CI (not just the unit-test budget model, which has
now under-predicted reality twice), then comment on #7518 with the run links
before considering it durably closed.

…RSS (#7518)

PR #11419 lowered OMNIROUTE_BUILD_WORKERS 8->3 using an INFERRED per-worker
peak (WORKER_PEAK_MB=2560) and treated the parent `next build` process's
RSS as bounded by OMNIROUTE_BUILD_MEMORY_MB (the V8 heap ceiling). Both
assumptions were wrong: the issue owner's own VPS reproduction (dmesg
OOM-killer report) measured the real per-process RSS directly at ~4.5 GB,
independent of NODE_OPTIONS, since Turbopack compiles in native/Rust memory
outside the V8 heap -- and that applies to the parent process too.

With OMNIROUTE_BUILD_WORKERS=3 (1 parent + 2 workers), the measured worst
case is 3 x 4500MB = 13500MB against a 12288MB (75% of 16GB) budget --
still over, matching the still-live "Publish to Docker Hub" CI failures
after #11419 merged (run 32907937950, 2026-08-25).

Lowers OMNIROUTE_BUILD_WORKERS to 2 (1 parent + 1 worker = 9000MB, fits
with headroom) and re-derives tests/unit/docker-build-memory-budget.test.ts's
model on the measured figure, applied uniformly to every process instead of
conflating the parent process with the V8 heap ceiling.
@diegosouzapw
diegosouzapw merged commit 124f4cf into release/v3.8.51 Aug 26, 2026
16 of 22 checks passed
@diegosouzapw

Copy link
Copy Markdown
Collaborator Author

Validated in local merge-train (train tip a3a96af7308584900f0fab3ab4afd8499391230a, boarded #11663 #11664 #11665 #11667 #11668 #11669 #11670 #11678 on .113 /srv/omniroute-train) — typecheck:core, file-size, complexity, cognitive-complexity, changelog-integrity all GREEN. npm run test:unit showed 21 failures across 15 test files, none touching this PR's diff; reproduced identically (same test names, same asserts) on origin/release/v3.8.51 alone with zero PRs boarded (targeted re-run, 21/94 fail, same failure set) — confirmed base-red/environment-load drift per merge-gates.md §3, not introduced by any of the 8 boarded PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants