From 303a9dd37e4c16949631f2180fca78eb4913b6ff Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Wed, 23 Sep 2026 15:19:49 +0200 Subject: [PATCH 1/3] test: size SanityCheckTest's forced -Xmx from MemTotal instead of 900g 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) --- .../profiler/sanity/SanityCheckTest.java | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/sanity/SanityCheckTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/sanity/SanityCheckTest.java index 0d1b783e25..3e3225e338 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/sanity/SanityCheckTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/sanity/SanityCheckTest.java @@ -38,6 +38,19 @@ private Path newJfrPath(String prefix) throws Exception { return Files.createTempFile(rootDir, prefix, ".jfr"); } + /** + * Total physical memory in MB as seen by {@code OS::getRamSize()} (the {@code MemTotal} line + * of {@code /proc/meminfo}), which bounds the memory sanity check's available-memory figure. + */ + private static long memTotalMb() throws Exception { + for (String line : Files.readAllLines(Paths.get("/proc/meminfo"))) { + if (line.startsWith("MemTotal:")) { + return Long.parseLong(line.replaceAll("[^0-9]", "")) / 1024; + } + } + throw new IllegalStateException("MemTotal not found in /proc/meminfo"); + } + /** * nosanity=true bypasses sanity checks. The profiler must start successfully on any host. */ @@ -98,7 +111,7 @@ void sanity_checks_run_once() throws Exception { } /** - * A forced -Xmx far larger than any real host's RAM makes the memory sanity check fail + * A forced -Xmx larger than the host's total RAM makes the memory sanity check fail * deterministically, regardless of the actual host resources. The check is advisory, so * the profiler must still start, and the JFR recording's settings must show the failure. */ @@ -120,7 +133,17 @@ void mem_sanity_check_failure_is_recorded_in_jfr() throws Exception { // to a fraction of -Xmx regardless of -Xms, so -Xms8m alone still eagerly // commits ~28g and OOMs before the sanity check runs. G1 sizes its initial // commit in fixed-size regions independent of -Xmx, avoiding that. - LaunchResult result = launch("profiler", Arrays.asList("-XX:+UseG1GC", "-Xmx900g", "-Xms8m"), + // + // JDK 8's G1 still allocates and clears card-granularity bitmaps spanning the + // whole reserved heap, one per parallel GC thread: measured on 8u504, a + // -Xmx900g fork touches ~225MB per ParallelGCThreads on top of a ~360MB base + // (1.3GB at 4 threads, 4GB at 16), enough to OOM-kill a 6GB CI container. + // -Xmx is therefore sized just past MemTotal -- the check's upper bound never + // exceeds it, and its estimate is at least 1.3x -Xmx -- and a single GC thread + // keeps the per-thread cost bounded on hosts with a lot of RAM. + long xmxMb = memTotalMb() + 1024; + LaunchResult result = launch("profiler", + Arrays.asList("-XX:+UseG1GC", "-XX:ParallelGCThreads=1", "-Xmx" + xmxMb + "m", "-Xms8m"), "start,jfr,file=" + forkedJfr.toAbsolutePath(), line -> LineConsumerResult.CONTINUE, line -> LineConsumerResult.CONTINUE); assertTrue(result.inTime, "forked JVM did not exit in time"); From 2564dd411850a92bb1fef101f2dbaa85b747cf60 Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Wed, 23 Sep 2026 15:51:53 +0200 Subject: [PATCH 2/3] test: bound SanityCheckTest's forced -Xmx by the cgroup memory limit 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) --- .../profiler/sanity/SanityCheckTest.java | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/sanity/SanityCheckTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/sanity/SanityCheckTest.java index 3e3225e338..aa2e660332 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/sanity/SanityCheckTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/sanity/SanityCheckTest.java @@ -39,16 +39,34 @@ private Path newJfrPath(String prefix) throws Exception { } /** - * Total physical memory in MB as seen by {@code OS::getRamSize()} (the {@code MemTotal} line - * of {@code /proc/meminfo}), which bounds the memory sanity check's available-memory figure. + * An upper bound, in MB, on the memory sanity check's available-memory figure: the smaller of + * {@code MemTotal} ({@code OS::getRamSize()}) and this cgroup namespace's root memory limit. + * The native check takes the minimum over the process's whole cgroup ancestry, which includes + * that root, so its figure can only be smaller than this. */ - private static long memTotalMb() throws Exception { + private static long availableMemoryUpperBoundMb() throws Exception { + long bytes = Long.MAX_VALUE; for (String line : Files.readAllLines(Paths.get("/proc/meminfo"))) { if (line.startsWith("MemTotal:")) { - return Long.parseLong(line.replaceAll("[^0-9]", "")) / 1024; + bytes = Long.parseLong(line.replaceAll("[^0-9]", "")) * 1024; + break; } } - throw new IllegalStateException("MemTotal not found in /proc/meminfo"); + for (String limitFile : new String[] { + "/sys/fs/cgroup/memory.max", "/sys/fs/cgroup/memory/memory.limit_in_bytes"}) { + Path path = Paths.get(limitFile); + if (Files.isReadable(path)) { + String value = new String(Files.readAllBytes(path)).trim(); + // cgroup v2 spells "unlimited" as "max"; v1 uses a near-Long.MAX_VALUE number. + if (value.matches("[0-9]+")) { + bytes = Math.min(bytes, Long.parseLong(value)); + } + } + } + if (bytes == Long.MAX_VALUE) { + throw new IllegalStateException("MemTotal not found in /proc/meminfo"); + } + return bytes / (1024 * 1024); } /** @@ -111,7 +129,7 @@ void sanity_checks_run_once() throws Exception { } /** - * A forced -Xmx larger than the host's total RAM makes the memory sanity check fail + * A forced -Xmx larger than the memory available to the process makes the memory sanity check fail * deterministically, regardless of the actual host resources. The check is advisory, so * the profiler must still start, and the JFR recording's settings must show the failure. */ @@ -138,10 +156,11 @@ void mem_sanity_check_failure_is_recorded_in_jfr() throws Exception { // whole reserved heap, one per parallel GC thread: measured on 8u504, a // -Xmx900g fork touches ~225MB per ParallelGCThreads on top of a ~360MB base // (1.3GB at 4 threads, 4GB at 16), enough to OOM-kill a 6GB CI container. - // -Xmx is therefore sized just past MemTotal -- the check's upper bound never - // exceeds it, and its estimate is at least 1.3x -Xmx -- and a single GC thread - // keeps the per-thread cost bounded on hosts with a lot of RAM. - long xmxMb = memTotalMb() + 1024; + // -Xmx is therefore sized just past the check's available-memory figure (a + // 6GB-limited pod on a 380GB node needs ~7g, not 900g) -- its estimate is at least + // 1.3x -Xmx -- and a single GC thread keeps the per-thread cost bounded where + // that figure is large. + long xmxMb = availableMemoryUpperBoundMb() + 1024; LaunchResult result = launch("profiler", Arrays.asList("-XX:+UseG1GC", "-XX:ParallelGCThreads=1", "-Xmx" + xmxMb + "m", "-Xms8m"), "start,jfr,file=" + forkedJfr.toAbsolutePath(), From 52497b051b3a45e9b972093b462b48252eae9fc8 Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Wed, 23 Sep 2026 15:45:14 +0200 Subject: [PATCH 3/3] fix: don't read the -XX:ErrorFile hs_err_pid%p template as a crash 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) --- .github/scripts/flake_report.py | 6 ++++-- .github/scripts/tests/test_quarantine.sh | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/.github/scripts/flake_report.py b/.github/scripts/flake_report.py index 71398df825..67a6cc8ade 100755 --- a/.github/scripts/flake_report.py +++ b/.github/scripts/flake_report.py @@ -111,10 +111,12 @@ def cmd_count(args): _NON_TEST_TASK_FAILURE_RE = re.compile(r"Execution failed for task '([^']+)'") # The JVM's own crash banner -- reliable on every platform, including musl: -# nothing but a real crash prints this. +# nothing but a real crash prints this. The report file name only counts with +# an actual pid: Gradle --info also prints every test JVM's command line, whose +# -XX:ErrorFile=...hs_err_pid%p.log template is configuration, not a crash. _CRASH_RE = re.compile( r"A fatal error has been detected by the Java Runtime Environment" - r"|hs_err_pid" + r"|hs_err_pid\d+" ) # Gradle reporting that a forked test JVM died. Reliable for the glibc/macOS diff --git a/.github/scripts/tests/test_quarantine.sh b/.github/scripts/tests/test_quarantine.sh index c7395c3d60..078e5bc238 100755 --- a/.github/scripts/tests/test_quarantine.sh +++ b/.github/scripts/tests/test_quarantine.sh @@ -725,6 +725,30 @@ assert d['final_attempt_cut_short'], d " "$CASE/out.json" || fail "a genuine musl crash was excused by the quarantine list" pass "a genuine crash banner still gates a quarantined musl failure" +# Gradle --info prints each test JVM's full command line, which carries the +# -XX:ErrorFile=...hs_err_pid%p.log template. That is configuration, not a +# crash, and must not stop the list from excusing a quarantined failure. +CASE="$TEMP_DIR/case-quarantined-errorfile-flag" +mkdir -p "$CASE/flake-evidence/attempt-1" +write_failure_xml "$CASE/flake-evidence/attempt-1" "com.dd.WobblyTest" "sometimesFails" "boom" +cat > "$CASE/attempt.log" <<'EOS' +Starting process 'Gradle Test Executor 1'. Command: /jdk/bin/java -XX:ErrorFile=build/hs_err_pid%p.log -Xmx1536m +> Task :ddprof-test:testRelease FAILED +EOS +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "el7-8-release-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 1 --attempt-log "$CASE/attempt.log" \ + --final-attempt-exit-code 1 --test-task-pattern test \ + --out "$CASE/out.json" >/dev/null 2>&1 +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['gates'] is False, 'the ErrorFile flag must not read as a crash: %r' % d['gate_reason'] +assert not d['final_attempt_cut_short'], d +" "$CASE/out.json" || fail "the -XX:ErrorFile hs_err_pid%p template was read as a crash" +pass "the -XX:ErrorFile hs_err_pid%p template does not gate a quarantined failure" + # Same intent, without the log saying so: the final attempt reached fewer tests # than an earlier one managed, so it stopped early. CASE="$TEMP_DIR/case-quarantined-but-short-run"