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" 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..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 @@ -38,6 +38,37 @@ private Path newJfrPath(String prefix) throws Exception { return Files.createTempFile(rootDir, prefix, ".jfr"); } + /** + * 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 availableMemoryUpperBoundMb() throws Exception { + long bytes = Long.MAX_VALUE; + for (String line : Files.readAllLines(Paths.get("/proc/meminfo"))) { + if (line.startsWith("MemTotal:")) { + bytes = Long.parseLong(line.replaceAll("[^0-9]", "")) * 1024; + break; + } + } + 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); + } + /** * nosanity=true bypasses sanity checks. The profiler must start successfully on any host. */ @@ -98,7 +129,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 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. */ @@ -120,7 +151,18 @@ 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 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(), line -> LineConsumerResult.CONTINUE, line -> LineConsumerResult.CONTINUE); assertTrue(result.inTime, "forked JVM did not exit in time");