perf(docker): pin JVM container memory limits to stop host oversubscription - #1410
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughCompose now assigns configurable memory limits to the application-server, application-worker, and webhook-server containers. The changeset documents defaults, environment overrides, and host-memory sizing guidance. ChangesJVM container memory limits
Estimated code review effort: 2 (Simple) | ~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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@docker/compose.app.yaml`:
- Around line 96-100: The memory budget must include worker sandbox concurrency.
In docker/compose.app.yaml at lines 96-100 and 299-300, cap
SANDBOX_MAX_CONCURRENT or lower the JVM memory limits so the combined allocation
remains within the host budget; retain the webhook memory limit in
docker/compose.core.yaml lines 11-16. Update .changeset/jvm-memory-limits.md
lines 12-16 with the sandbox ceiling and minimum recommended host RAM.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9e15a97-01f1-475c-9ea7-4405bbfb4297
📒 Files selected for processing (3)
.changeset/jvm-memory-limits.mddocker/compose.app.yamldocker/compose.core.yaml
| # Pin the container memory limit so the Paketo memory calculator sizes -Xmx for THIS container | ||
| # instead of the whole host. Unpinned, co-located JVMs each size their heap to the full host and | ||
| # their sum oversubscribes RAM (→ swap → GC thrash). This role hosts the NATS sync consumers, so | ||
| # it legitimately needs the most headroom — size it to the box, leaving room for webhook + db. | ||
| mem_limit: ${APPLICATION_SERVER_MEM_LIMIT:-6g} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== docker/compose.app.yaml (relevant ranges) ==\n'
sed -n '80,115p;285,310p' docker/compose.app.yaml
printf '\n== docker/compose.core.yaml ==\n'
sed -n '1,35p' docker/compose.core.yaml
printf '\n== .changeset/jvm-memory-limits.md ==\n'
sed -n '1,80p' .changeset/jvm-memory-limits.md
printf '\n== Search for sandbox/container memory settings and host sizing guidance ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/.git/**' -e 'sandbox.*mem|mem.*sandbox|memory.*sandbox|APPLICATION_WORKER_MEM_LIMIT|WEBHOOK_SERVER_MEM_LIMIT|APPLICATION_SERVER_MEM_LIMIT|comfortably under host|leave room|host RAM|physical RAM|5 GiB|4 GiB|sandbox container' .Repository: ls1intum/Hephaestus
Length of output: 10907
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== sandbox concurrency / memory config ==\n'
sed -n '1,120p' server/src/main/java/de/tum/cit/aet/hephaestus/agent/sandbox/SandboxProperties.java
printf '\n== single-host compose guidance ==\n'
sed -n '60,105p' docker/self-host/compose.single-host.yaml
printf '\n== self-host env example ==\n'
sed -n '90,120p' docker/self-host/.env.example
printf '\n== install docs memory guidance ==\n'
sed -n '20,45p' docs/admin/install.mdx
printf '\n== search for maxConcurrentContainers ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/.git/**' 'maxConcurrentContainers|concurrent sandbox|sandbox concurrency|4 GiB per sandbox|up to 4 GiB|one concurrent agent sandbox|five 4 GiB' server docs dockerRepository: ls1intum/Hephaestus
Length of output: 11915
The memory budget still misses worker sandboxes.
application-server + application-worker + webhook-server already reserve 11.5 GiB, and the worker still allows 5 concurrent sandboxes at 4 GiB each. That can push a shared host well past the “comfortably under RAM” guidance.
docker/compose.app.yaml#L96-L100/#L299-L300: capSANDBOX_MAX_CONCURRENThere or lower the JVM limits.docker/compose.core.yaml#L11-L16: keep the webhook limit in the aggregate budget..changeset/jvm-memory-limits.md#L12-L16: add the sandbox ceiling and minimum host RAM to the operator guidance.
📍 Affects 3 files
docker/compose.app.yaml#L96-L100(this comment)docker/compose.app.yaml#L299-L300docker/compose.core.yaml#L11-L16.changeset/jvm-memory-limits.md#L12-L16
🤖 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 `@docker/compose.app.yaml` around lines 96 - 100, The memory budget must
include worker sandbox concurrency. In docker/compose.app.yaml at lines 96-100
and 299-300, cap SANDBOX_MAX_CONCURRENT or lower the JVM memory limits so the
combined allocation remains within the host budget; retain the webhook memory
limit in docker/compose.core.yaml lines 11-16. Update
.changeset/jvm-memory-limits.md lines 12-16 with the sandbox ceiling and minimum
recommended host RAM.
81d3f36 to
f66c063
Compare
…iption The application-server, application-worker and webhook-server containers had no memory limit, so the Paketo memory calculator sized each JVM's heap against the whole host. On a shared box the heaps summed past physical RAM (observed: an 8.4 GiB heap on webhook-server whose live set is ~150 MiB, plus 5.36 GiB on the app-server, on an 11 GiB host), driving the machine into swap and leaving the application server near-constantly in G1 concurrent GC and unresponsive. Pin an explicit, per-service, env-overridable mem_limit so each JVM sizes its heap for its own container. Defaults leave headroom for co-located services. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012stCYrymLyX9VZ8PrPUs88
f66c063 to
7e218fe
Compare
📚 Documentation Preview
|
Motivation
Staging's application server was intermittently very slow and its host was swapping. Live profiling on the staging box traced it to a memory-management root cause, not a "needs more RAM" problem:
mem_limit=0on every JVM service). With no cgroup limit, the Paketo memory calculator sizes-Xmxagainst the whole host.webhook-server:-Xmx8.4 GiB — but a full-GC class histogram showed a live set of ~150 MiB. Pure waste.application-server:-Xmx5.36 GiB.G1 ConcurrentGC (3 cores pegged) walking swapped pages → slow, unresponsive API.Capping
webhook-serverlive (to 1.5 GiB) during the investigation immediately dropped host memory use from 9.0 → 6.3 GiB, drained swap, and relieved the app — confirming oversubscription as the cause.Change
Add an explicit, per-service, environment-overridable
mem_limitto the three JVM containers so each sizes its heap for its own container instead of the host:webhook-server1536mWEBHOOK_SERVER_MEM_LIMITapplication-server6gAPPLICATION_SERVER_MEM_LIMITapplication-worker4gAPPLICATION_WORKER_MEM_LIMITDefaults keep the sum comfortably under an 11 GiB host with room for Postgres/NATS. Larger hosts can raise them.
Follow-up (not in this PR)
Profiling also found the app-server holds ~1.8 GiB of heap as buffered NATS JetStream messages — the
githubstream has 2,000,000 messages / 30 GiB across 94 consumers, andmaxAckPending(500) is a per-consumer cap with no aggregate bound, so in-flight memory scales with consumer count. Worth a dedicated PR to bound total in-flight (and it needs load testing), so it's intentionally out of scope here.Testing
docker compose config.🤖 Generated with Claude Code
Summary by CodeRabbit
WEBHOOK_SERVER_MEM_LIMIT)APPLICATION_SERVER_MEM_LIMIT)APPLICATION_WORKER_MEM_LIMIT)