fix(web): scale web heap limit to container memory - #1569
Conversation
Node's default old-space limit caps at ~4GiB regardless of how much memory the container actually has. On larger deployments the web process fills that ceiling and V8 falls into back-to-back full mark-compact collections, which block the event loop for seconds at a time and slow every route at once. Set --max-old-space-size-percentage on the web program so the heap scales with the container's memory limit. Node reads the cgroup limit, so this is proportional on every deployment size. Scoped to [program:web] via supervisord so the backend and zoekt are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe web process now sets Node.js’s maximum heap size to 50% of container memory. The changelog documents removal of the approximately 4 GiB heap cap. ChangesWeb runtime configuration
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
supervisord.conf (2)
19-19: 🩺 Stability & Availability | 🔵 TrivialMeasure aggregate cgroup memory before relying on 75%.
--max-old-space-size-percentage=75limits V8 old-space, not total web RSS. Becausebackendandzoektshare the container, measure aggregate memory during representative load. Lower the percentage if usage approaches the cgroup limit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supervisord.conf` at line 19, Measure aggregate cgroup memory usage for both backend and zoekt under representative load before finalizing the NODE_OPTIONS max-old-space-size-percentage setting. If combined RSS approaches the container limit, lower the 75% value in the supervisord environment configuration.
19-19: 🩺 Stability & Availability | 🔵 TrivialSize the container for all supervised processes
--max-old-space-size-percentage=75limits V8 old space for the web process. It does not limit total process memory. Account for backend, Zoekт, and native allocations when setting the container memory limit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supervisord.conf` at line 19, Update the supervisord environment configuration to size the container memory limit for all supervised processes, including the web process, backend, Zoekт, and native allocations, rather than relying only on NODE_OPTIONS V8 old-space limits. Set the container-level limit using the deployment’s established memory configuration mechanism.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@supervisord.conf`:
- Line 19: Measure aggregate cgroup memory usage for both backend and zoekt
under representative load before finalizing the NODE_OPTIONS
max-old-space-size-percentage setting. If combined RSS approaches the container
limit, lower the 75% value in the supervisord environment configuration.
- Line 19: Update the supervisord environment configuration to size the
container memory limit for all supervised processes, including the web process,
backend, Zoekт, and native allocations, rather than relying only on NODE_OPTIONS
V8 old-space limits. Set the container-level limit using the deployment’s
established memory configuration mechanism.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e35bc427-d40a-4dde-9331-c2d1584c00c1
📒 Files selected for processing (2)
CHANGELOG.mdsupervisord.conf
At a 16GiB limit this gives the web process an 8384MiB ceiling (up from 4288MiB), which still resolves the GC thrash while leaving roughly 7.7GiB for page cache. zoekt currently uses ~5.8GiB of file cache for its EFS-backed index, and 75% would have squeezed that to ~3.6GiB. 50% also matches Node's own default curve below the ~4GiB cap, so deployments at or under 8GiB are unaffected by this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
supervisord.conf (1)
19-19: 🩺 Stability & Availability | 🔵 TrivialVerify that Node.js uses the container cgroup limit.
Node.js defines this percentage using “available system memory.” The existing cgroup reader in
packages/web/src/features/billing/systemInfo.tsdoes not prove that the Node.js process uses the same limit. Measurev8.getHeapStatistics().heap_size_limitinside the 16 GiB production container and compare it with the cgroup limit before relying on the stated 8,384 MiB ceiling. (nodejs.org)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supervisord.conf` at line 19, Verify the effective Node.js heap limit in the 16 GiB production container by measuring v8.getHeapStatistics().heap_size_limit and comparing it with the cgroup limit reported by the existing systemInfo logic. Update the supervisord NODE_OPTIONS percentage or document the validated ceiling only after confirming Node.js uses the container cgroup limit; do not rely on the stated 8,384 MiB value without this measurement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@supervisord.conf`:
- Line 19: Verify the effective Node.js heap limit in the 16 GiB production
container by measuring v8.getHeapStatistics().heap_size_limit and comparing it
with the cgroup limit reported by the existing systemInfo logic. Update the
supervisord NODE_OPTIONS percentage or document the validated ceiling only after
confirming Node.js uses the container cgroup limit; do not rely on the stated
8,384 MiB value without this measurement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 00bbde17-f52d-422a-955f-7e1ccb98cea9
📒 Files selected for processing (1)
supervisord.conf
1 participant
Fixes SOU-1933
Problem
Node's default old-space limit caps at roughly 4 GiB no matter how much memory the container has.
app.sourcebot.devruns with a 16 GiB limit, so the web process was capped at 4288 MiB — about a quarter of what it was allocated.Once
next-serverreaches that ceiling, V8 stops doing cheap incremental sweeps and runs full stop-the-world mark-compact collections back to back. That blocks the single event loop, so every route slows simultaneously and the health endpoint stops answering.Evidence from production:
GET /(the slowest in 7 days; p50 is 58 ms) whose two dominant spans wereprisma:client:operationat 1398 ms and 1248 ms — withprisma:engine:querychildren of 2.16 ms and 1.13 ms. The database did ~2 ms of work; the rest was the event loop being blocked./api/source2.3 s, changelog 2.0 s, commits 1.7 s, repos 1.7 s.next-serverwas at 4.79 GiBRssAnonagainst the 4.19 GiB V8 ceiling.Liveness probe failedevents in 20 hours, and the pod has restarted 5 times. The container is killed roughly every 6 hours as the heap fills.Change
Set
--max-old-space-size-percentage=50on the[program:web]block. Node resolves the percentage against the cgroup memory limit rather than host RAM, so it stays proportional at every deployment size.Measured
heap_size_limit, in MiB:pct=50(this PR)pct=75Those numbers imply Node's default is about 50% of the cgroup limit, capped at ~4 GiB. Two consequences:
app.sourcebot.devand not on typical self-hosted deployments.Scoping it to
[program:web]via supervisord'senvironment=keeps the backend worker and zoekt on their existing defaults.Tradeoffs worth reviewing
Why 50 and not more. At a 16 GiB limit, 50% leaves roughly 7.7 GiB for page cache, comfortably above the ~5.8 GiB of file cache zoekt currently uses for its EFS-backed index. 75% would cut that to ~3.6 GiB, and EFS major faults are network round-trips, so search latency would likely suffer. 75% also raises the ceiling for small self-hosted installs (a 2 GiB deployment would go from 1120 to 1632 MiB) for no benefit to them.
This is a mitigation, not a cure. The heap grows at roughly 700–800 MiB/hour with no observed plateau, which is what produces the ~6 hour restart cadence. A higher ceiling extends that to ~11 hours; it does not stop it. If the growth is reclaimable garbage, this change genuinely fixes the problem. If it is a leak, the ceiling only delays the wall, and each full GC gets longer because GC cost scales with live heap size. Determining which requires post-GC heap floor data over time — see the follow-ups.
NODE_OPTIONSset at the container level no longer reaches the web process (the backend and zoekt still see it), and because this flag outranks--max-old-space-sizeregardless of order, an operator's own heap sizing will not take effect fornext-server. Worth calling out for self-hosters.Follow-ups
timeoutSeconds: 1withfailureThreshold: 5. A 1 second timeout on a Next.js health endpoint will trip on any GC pause, which is what converts "briefly slow" into "dead container". Raising it is independent of this change and arguably higher-impact.Test plan
sh -nandbusybox sh -n(the container's actual shell) both cleansupervisord.confparses; supervisord's own parser yields{'NODE_OPTIONS': '--max-old-space-size-percentage=50'}backendandzoektconfirmed to have noenvironmentlineheap_size_limitin the running web process and watch restart cadence🤖 Generated with Claude Code
Summary by CodeRabbit
Note
Medium Risk
Changes production memory allocation for the web process and overrides container-level
NODE_OPTIONSfor that process only. Incorrect sizing could pressure zoekt page cache or delay OOM/restart behavior if heap growth is a leak.Overview
Raises the Next.js web process heap ceiling on large containers by setting
NODE_OPTIONS=--max-old-space-size-percentage=50in the[program:web]supervisord block.This removes Node's ~4 GiB default old-space cap so the heap scales with the cgroup memory limit (e.g. ~8 GiB on a 16 GiB deployment), reducing stop-the-world GC pauses. Backend and zoekt keep their existing defaults; smaller deployments at or below ~8 GiB are unchanged.
Reviewed by Cursor Bugbot for commit b27910b. Bugbot is set up for automated code reviews on this repo. Configure here.