Fix SanityCheckTest OOM on JDK 8 and a false crash in flake_report - #815
Merged
Merged
Conversation
On JDK 8, G1 allocates and clears card-granularity bitmaps spanning the whole reserved heap, one per parallel GC thread. With -Xmx900g the forked JVM touches ~225MB per ParallelGCThreads on top of a ~360MB base (1.3GB at 4 threads, 4GB at 16, measured on 8u504), enough to OOM-kill a 6GB CI container. JDK 11 stays around 140MB regardless. The check only compares its heap-based estimate against MemTotal (or a smaller container limit), so -Xmx just past MemTotal fails it just as deterministically. A single GC thread bounds the remaining per-thread cost on hosts with a lot of RAM. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The memory sanity check compares its estimate against the smaller of MemTotal and the container memory limit. On a Kubernetes node MemTotal is the whole host (~380GB on the EL7 runner) while the pod is limited to 6GB, so sizing -Xmx from MemTotal alone still forked a ~383g JVM. Take the cgroup namespace's root limit into account too; the native check takes the minimum over the process's whole cgroup ancestry, which includes that root, so -Xmx just past this bound still always trips the check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Gradle --info prints each test JVM's full command line, including -XX:ErrorFile=build/hs_err_pid%p.log. The crash marker matched any "hs_err_pid", so every EL7 functional run (the only cell running with --info) was classified as cut short and its quarantined failures gated the job. A real crash report names the file with an actual pid, so match hs_err_pid followed by digits. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52497b051b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
CI Test ResultsRun: #35912535325 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-09-23 20:51:50 UTC |
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Please try again by commenting @autotest review.
Contributor
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
1 participant
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.
Summary
Two fixes found while getting the EL7 functional job (#805, #810) green.
SanityCheckTest forked a JVM that OOM-killed 6GB containers on JDK 8
mem_sanity_check_failure_is_recorded_in_jfrforks a JVM with-XX:+UseG1GC -Xmx900gso the memory sanity check fails. The check only reads flags, but JDK 8's G1 allocates and clears card-granularity bitmaps spanning the whole reserved heap, one per parallel GC thread. Peak cgroup memory of that fork, measured on Temurin 8u504 in an EL7 container:-Xmx8g-Xmx900g, 1 GC thread-Xmx900g, 4 GC threads-Xmx900g, 16 GC threads-Xmx900g, any thread countOn the 6GB EL7 pod this OOM-killed the whole job mid-suite.
The check compares its estimate (at least 1.3 ×
-Xmx) against the smaller ofMemTotaland the container memory limit. So-Xmxnow comes from that same bound plus 1g: about 7g on a 6GB pod on a ~380GB node. The fork also gets-XX:ParallelGCThreads=1, which bounds the remaining cost where the bound is large. The check still fails deterministically.flake_report.py read the
-XX:ErrorFiletemplate as a JVM crashThe crash marker matched any
hs_err_pid. Gradle--infoprints each test JVM's command line, including-XX:ErrorFile=build/hs_err_pid%p.log. Every run with--infowas therefore classified as cut short, and its quarantined failures still gated the job. The marker now requires an actual pid (hs_err_pid\d+), which is how a real crash report names its file.Test plan
test_quarantine.sh: new case for the-XX:ErrorFiletemplate; it fails before the fix and passes after. All 60 cases pass, the other CI script test suites pass, andquarantine.py validatepasses.MemTotalwithout one.🤖 Generated with Claude Code