diff --git a/src/main/java/io/cryostat/net/reports/SubprocessReportGenerator.java b/src/main/java/io/cryostat/net/reports/SubprocessReportGenerator.java index 0a9ddf6bfb..687bfa9064 100644 --- a/src/main/java/io/cryostat/net/reports/SubprocessReportGenerator.java +++ b/src/main/java/io/cryostat/net/reports/SubprocessReportGenerator.java @@ -45,6 +45,7 @@ import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; import java.time.Duration; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -135,8 +136,7 @@ CompletableFuture exec(Path recording, Path saveFile, Duration timeout) // See https://github.com/cryostatio/cryostat/issues/287 .jvmArgs( createJvmArgs( - Integer.parseInt( - env.getEnv(SUBPROCESS_MAX_HEAP_ENV, "200")))) + Integer.parseInt(env.getEnv(SUBPROCESS_MAX_HEAP_ENV, "0")))) .processArgs(createProcessArgs(recording, saveFile)) .exec(); return CompletableFuture.supplyAsync( @@ -205,18 +205,18 @@ Path copyRecordingToFile(JFRConnection conn, String recordingName, Path path) th } private List createJvmArgs(int maxHeapMegabytes) throws IOException { - // These JVM flags must be kept in-sync with the flags set on the parent process in - // entrypoint.sh in order to keep the auth and certs setup consistent - return List.of( - String.format("-Xmx%dM", maxHeapMegabytes), - "-XX:+ExitOnOutOfMemoryError", - // use EpsilonGC since we're a one-shot process and report generation doesn't - // allocate that much memory beyond the initial recording load - no point in - // over-complicating memory allocation. Preferable to just complete the request - // quickly, or if we're running up against the memory limit, fail early - "-XX:+UnlockExperimentalVMOptions", - "-XX:+UseEpsilonGC", - "-XX:+AlwaysPreTouch"); + List args = new ArrayList<>(); + if (maxHeapMegabytes > 0) { + args.add(String.format("-Xms%dM", maxHeapMegabytes)); + args.add(String.format("-Xmx%dM", maxHeapMegabytes)); + } + args.add("-XX:+ExitOnOutOfMemoryError"); + // use Serial GC since we have a small heap and likely little garbage to clean, + // and low GC overhead is more important here than minimizing pause time since the + // result will end up cached for subsequent user accesses so long as the process + // succeeds in the end + args.add("-XX:+UseSerialGC"); + return args; } private List createProcessArgs(Path recording, Path saveFile) { diff --git a/src/test/java/io/cryostat/net/reports/SubprocessReportGeneratorTest.java b/src/test/java/io/cryostat/net/reports/SubprocessReportGeneratorTest.java index 3cf57d01ab..c6c8525fca 100644 --- a/src/test/java/io/cryostat/net/reports/SubprocessReportGeneratorTest.java +++ b/src/test/java/io/cryostat/net/reports/SubprocessReportGeneratorTest.java @@ -118,7 +118,10 @@ void setup() throws Exception { .thenReturn(javaProcessBuilder); Mockito.lenient().when(javaProcessBuilder.exec()).thenReturn(proc); Mockito.lenient() - .when(env.getEnv(SubprocessReportGenerator.SUBPROCESS_MAX_HEAP_ENV, "200")) + .when( + env.getEnv( + Mockito.eq(SubprocessReportGenerator.SUBPROCESS_MAX_HEAP_ENV), + Mockito.anyString())) .thenReturn("200"); this.generator = new SubprocessReportGenerator( @@ -190,12 +193,27 @@ void shouldSetJvmArgs() throws Exception { Mockito.verify(javaProcessBuilder).jvmArgs(captor.capture()); List expected = - List.of( - "-Xmx200M", - "-XX:+ExitOnOutOfMemoryError", - "-XX:+UnlockExperimentalVMOptions", - "-XX:+UseEpsilonGC", - "-XX:+AlwaysPreTouch"); + List.of("-Xms200M", "-Xmx200M", "-XX:+ExitOnOutOfMemoryError", "-XX:+UseSerialGC"); + MatcherAssert.assertThat(captor.getValue(), Matchers.equalTo(expected)); + } + + @Test + void shouldSetJvmArgsWithoutReportMaxHeapEnvVar() throws Exception { + Path dest = Mockito.mock(Path.class); + Mockito.when(dest.toAbsolutePath()).thenReturn(dest); + Mockito.when(dest.toString()).thenReturn("/dest/somefile.tmp"); + Mockito.when( + env.getEnv( + Mockito.eq(SubprocessReportGenerator.SUBPROCESS_MAX_HEAP_ENV), + Mockito.anyString())) + .thenReturn("0"); + + generator.exec(recordingFile, dest, Duration.ofSeconds(10)); + + ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); + Mockito.verify(javaProcessBuilder).jvmArgs(captor.capture()); + + List expected = List.of("-XX:+ExitOnOutOfMemoryError", "-XX:+UseSerialGC"); MatcherAssert.assertThat(captor.getValue(), Matchers.equalTo(expected)); }