fix(docker): drop arm64 Next.js build to 1 page-data worker, the 2-worker fix still OOMs - #11647
Open
PauloFH wants to merge 3 commits into
Open
fix(docker): drop arm64 Next.js build to 1 page-data worker, the 2-worker fix still OOMs#11647PauloFH wants to merge 3 commits into
PauloFH wants to merge 3 commits into
Conversation
…rker fix still OOMs The 2026-08-25 fix (7 workers -> 2, diegosouzapw#10060) modeled 2 workers as safe (6144 + 2x2560 = 11264 MB, under the 12288 MB budget) but did not hold: 60+ consecutive "Build Docker (linux/arm64)" runs from 2026-08-24 23:xx UTC through 2026-08-26 14:xx UTC (release/v3.8.50, release/v3.8.51, PR diegosouzapw#11442) all died with the same `ResourceExhausted: ... cannot allocate memory` right after "Collecting page data using 2 workers". WORKER_PEAK_MB=2560 undercounted the real per-worker RSS. Drop OMNIROUTE_BUILD_WORKERS to the test's floor (2 -> 1 actual worker) and raise WORKER_PEAK_MB to 4096 so tests/unit/docker-build-memory-budget.test.ts actually rejects the 2-worker config it previously passed by mistake. New worst case: 6144 + 1x4096 = 10240 MB, ~2 GB under the 12288 MB budget.
…ngle-worker fix)
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The documented OMNIROUTE_BUILD_WORKERS default in docs/guides/DOCKER_GUIDE.md is now out of sync with the new Dockerfile default (and the PR description also needs updating to reflect the added changelog fragment).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Reduces memory pressure in the Docker next build step on GitHub’s 16 GB ubuntu-24.04-arm runners by forcing a single Next.js page-data worker, and tightens the unit-test memory model so it would have rejected the previously “passing” (but still OOMing) 2-worker configuration.
Changes:
- Drop
OMNIROUTE_BUILD_WORKERSdefault3 → 2(Next page-data workers2 → 1) inDockerfile. - Raise
WORKER_PEAK_MB2560 → 4096in the regression test so the budget model rejects the prior 2-worker setup. - Add a changelog fragment documenting the operational fix.
File summaries
| File | Description |
|---|---|
tests/unit/docker-build-memory-budget.test.ts |
Updates the inferred per-worker peak RSS used by the memory-budget regression guard. |
Dockerfile |
Lowers the derived Next.js page-data worker count to 1 via CIRCLE_NODE_TOTAL. |
changelog.d/fixes/11647-docker-arm64-single-worker.md |
Adds a changelog fragment describing the arm64 Docker build fix and the updated budget model. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
rumman02
added a commit
to rumman02/OmniRoute
that referenced
this pull request
Aug 26, 2026
…workers OOM the 16 GB runners First tier run (32983445193) failed on both amd64 and arm64 with ResourceExhausted right after 'Collecting page data using 2 workers' — the Dockerfile default (WORKERS=3) no longer fits the 16 GB GitHub runners on this codebase. Inherited from the red base tip (issue diegosouzapw#11449); upstream is fixing the same signature in PR diegosouzapw#11647 (arm64). Passed as a per-workflow build-arg rather than editing the shared Dockerfile ARG so this branch stays conflict-free when the upstream fix lands (same value → the override becomes a no-op). 1 worker: worst case 6144 + 1x4096 = 10240 MB, ~2 GB under the 12288 MB budget.
…ault Copilot review on diegosouzapw#11647 flagged the guide as out of sync with the Dockerfile's new default (3 -> 2, i.e. 2 -> 1 actual page-data worker). Also documents that 2 workers (the previous default) still OOM'd on the arm64 runner, which is why the default sits at the test's floor.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Build Docker (linux/arm64)has been red on every push torelease/v3.8.51(andrelease/v3.8.50) since ~2026-08-24 23:xx UTC — 60+ consecutive failures, including on draft #11442 — always the same signature:right after
Collecting page data using 2 workers, on the nativeubuntu-24.04-armGitHub-hosted runner.What changed
The 2026-08-25 fix (
OMNIROUTE_BUILD_WORKERS8→3, i.e. 7→2 actual Next.js page-data workers, #10060) modeled 2 workers as fitting the 16 GB runner's budget (tests/unit/docker-build-memory-budget.test.ts:6144 + 2×2560 = 11264 MBunder the12288 MBbudget) — but that model'sWORKER_PEAK_MB=2560inference undercounted the real per-worker RSS, so the test kept passing a config that was red on every real run.This PR:
OMNIROUTE_BUILD_WORKERS3 → 2(Next derivesworkers = CIRCLE_NODE_TOTAL - 1, so this is2 → 1actual worker) — the floor the regression test still allows.WORKER_PEAK_MB2560 → 4096in the regression test so it now correctly rejects the previous 2-worker config (6144 + 2×4096 = 14336 MB > 12288 MBbudget) instead of silently passing a red config.6144 + 1×4096 = 10240 MB, ~2 GB under the 12288 MB budget (vs. the previous ~1 GB modeled margin that turned out to be negative in practice).Dockerfile, the regression test, anddocs/guides/DOCKER_GUIDE.md(matching the existing comment convention for these ARGs —DOCKER_GUIDE.mdwas flagged as out of sync by Copilot review and is now fixed too).changelog.d/fixes/11647-docker-arm64-single-worker.mdperchangelog.d/README.md.Verification
node --experimental-strip-types --test tests/unit/docker-build-memory-budget.test.ts→ 3/3 pass with the new Dockerfile values.WORKER_PEAK_MBwould have failed the previous (2-worker) config, so this is a real regression guard, not just a number bump.ubuntu-24.04-armbuild — thedocker-publish.ymlworkflow only triggers onpushtomain/release/v*(not on PR from a fork), so this PR's checks can't exercise the actual arm64 build. This is the most defensible next step per the codebase's own documented playbook (same lever as the 8→3 fix), but treat a real push torelease/v3.8.51as the actual test.Scope
Dockerfile,tests/unit/docker-build-memory-budget.test.ts,docs/guides/DOCKER_GUIDE.md,changelog.d/.Dockerfile.bundoes not useOMNIROUTE_BUILD_WORKERS/OMNIROUTE_BUILD_MEMORY_MBor run this build step, so it's unaffected and untouched.Known unrelated CI red on this PR
Docs Gates (fast-path),No new ESLint warnings, and 4×Unit Tests fast-pathare currently failing on this PR, but none touch anything this diff changed:354documented vs356live) inREADME.md/PROVIDER_REFERENCE.md/SVG diagrams — unrelated to Docker build args.There are suppressions left that do not occur anymore(staleeslint-suppressions.jsonentries) — unrelated to this diff.chatCore/Codex-passthrough andlive-ws-eaddrinuse-6324tests — unrelated to the Dockerfile/build-memory-budget test touched here.Consistent with the already-open
release/v3.8.51base-red issue (#11449). Not fixing these here to keep this PR scoped to the arm64 OOM.