diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java index dcb14dd949472..42ffcaa2ca5cd 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java @@ -51,7 +51,7 @@ */ final class JvmOptionsParser { - private static class JvmOptionsFileParserException extends Exception { + static class JvmOptionsFileParserException extends Exception { private final Path jvmOptionsFile; @@ -127,6 +127,29 @@ private List jvmOptions(final Path config, final String esJavaOpts, fina throws InterruptedException, IOException, JvmOptionsFileParserException { + + final List jvmOptions = readJvmOptionsFiles(config); + + if (esJavaOpts != null) { + jvmOptions.addAll( + Arrays.stream(esJavaOpts.split("\\s+")).filter(Predicate.not(String::isBlank)).collect(Collectors.toUnmodifiableList()) + ); + } + + final List substitutedJvmOptions = substitutePlaceholders(jvmOptions, Collections.unmodifiableMap(substitutions)); + final List ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions); + final List systemJvmOptions = SystemJvmOptions.systemJvmOptions(); + final List finalJvmOptions = new ArrayList<>( + systemJvmOptions.size() + substitutedJvmOptions.size() + ergonomicJvmOptions.size() + ); + finalJvmOptions.addAll(systemJvmOptions); // add the system JVM options first so that they can be overridden + finalJvmOptions.addAll(substitutedJvmOptions); + finalJvmOptions.addAll(ergonomicJvmOptions); + + return finalJvmOptions; + } + + List readJvmOptionsFiles(final Path config) throws IOException, JvmOptionsFileParserException { final ArrayList jvmOptionsFiles = new ArrayList<>(); jvmOptionsFiles.add(config.resolve("jvm.options")); @@ -154,24 +177,7 @@ private List jvmOptions(final Path config, final String esJavaOpts, fina throw new JvmOptionsFileParserException(jvmOptionsFile, invalidLines); } } - - if (esJavaOpts != null) { - jvmOptions.addAll( - Arrays.stream(esJavaOpts.split("\\s+")).filter(Predicate.not(String::isBlank)).collect(Collectors.toUnmodifiableList()) - ); - } - - final List substitutedJvmOptions = substitutePlaceholders(jvmOptions, Collections.unmodifiableMap(substitutions)); - final List ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions); - final List systemJvmOptions = SystemJvmOptions.systemJvmOptions(); - final List finalJvmOptions = new ArrayList<>( - systemJvmOptions.size() + substitutedJvmOptions.size() + ergonomicJvmOptions.size() - ); - finalJvmOptions.addAll(systemJvmOptions); // add the system JVM options first so that they can be overridden - finalJvmOptions.addAll(substitutedJvmOptions); - finalJvmOptions.addAll(ergonomicJvmOptions); - - return finalJvmOptions; + return jvmOptions; } static List substitutePlaceholders(final List jvmOptions, final Map substitutions) { diff --git a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmOptionsParserTests.java b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmOptionsParserTests.java index 953328b13c493..00ec30dfee30d 100644 --- a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmOptionsParserTests.java +++ b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmOptionsParserTests.java @@ -22,6 +22,10 @@ import java.io.BufferedReader; import java.io.IOException; import java.io.StringReader; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -31,7 +35,10 @@ import java.util.concurrent.atomic.AtomicBoolean; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; @@ -149,6 +156,108 @@ public void testComplexOptions() throws IOException { } } + public void testMissingRootJvmOptions() throws IOException, JvmOptionsParser.JvmOptionsFileParserException { + final Path config = newTempDir(); + try { + final JvmOptionsParser parser = new JvmOptionsParser(); + parser.readJvmOptionsFiles(config); + fail("expected no such file exception, the root jvm.options file does not exist"); + } catch (final NoSuchFileException expected) { + // this is expected, the root JVM options file must exist + } + } + + public void testReadRootJvmOptions() throws IOException, JvmOptionsParser.JvmOptionsFileParserException { + final Path config = newTempDir(); + final Path rootJvmOptions = config.resolve("jvm.options"); + Files.write(rootJvmOptions, List.of("# comment", "-Xms256m", "-Xmx256m"), StandardOpenOption.CREATE_NEW, StandardOpenOption.APPEND); + if (randomBoolean()) { + // an empty jvm.options.d directory should be irrelevant + Files.createDirectory(config.resolve("jvm.options.d")); + } + final JvmOptionsParser parser = new JvmOptionsParser(); + final List jvmOptions = parser.readJvmOptionsFiles(config); + assertThat(jvmOptions, contains("-Xms256m", "-Xmx256m")); + } + + public void testReadJvmOptionsDirectory() throws IOException, JvmOptionsParser.JvmOptionsFileParserException { + final Path config = newTempDir(); + Files.createDirectory(config.resolve("jvm.options.d")); + Files.write( + config.resolve("jvm.options"), + List.of("# comment", "-Xms256m", "-Xmx256m"), + StandardOpenOption.CREATE_NEW, + StandardOpenOption.APPEND + ); + Files.write( + config.resolve("jvm.options.d").resolve("heap.options"), + List.of("# comment", "-Xms384m", "-Xmx384m"), + StandardOpenOption.CREATE_NEW, + StandardOpenOption.APPEND + ); + final JvmOptionsParser parser = new JvmOptionsParser(); + final List jvmOptions = parser.readJvmOptionsFiles(config); + assertThat(jvmOptions, contains("-Xms256m", "-Xmx256m", "-Xms384m", "-Xmx384m")); + } + + public void testReadJvmOptionsDirectoryInOrder() throws IOException, JvmOptionsParser.JvmOptionsFileParserException { + final Path config = newTempDir(); + Files.createDirectory(config.resolve("jvm.options.d")); + Files.write( + config.resolve("jvm.options"), + List.of("# comment", "-Xms256m", "-Xmx256m"), + StandardOpenOption.CREATE_NEW, + StandardOpenOption.APPEND + ); + Files.write( + config.resolve("jvm.options.d").resolve("first.options"), + List.of("# comment", "-Xms384m", "-Xmx384m"), + StandardOpenOption.CREATE_NEW, + StandardOpenOption.APPEND + ); + Files.write( + config.resolve("jvm.options.d").resolve("second.options"), + List.of("# comment", "-Xms512m", "-Xmx512m"), + StandardOpenOption.CREATE_NEW, + StandardOpenOption.APPEND + ); + final JvmOptionsParser parser = new JvmOptionsParser(); + final List jvmOptions = parser.readJvmOptionsFiles(config); + assertThat(jvmOptions, contains("-Xms256m", "-Xmx256m", "-Xms384m", "-Xmx384m", "-Xms512m", "-Xmx512m")); + } + + public void testReadJvmOptionsDirectoryIgnoresFilesNotNamedOptions() throws IOException, + JvmOptionsParser.JvmOptionsFileParserException { + final Path config = newTempDir(); + Files.createFile(config.resolve("jvm.options")); + Files.createDirectory(config.resolve("jvm.options.d")); + Files.write( + config.resolve("jvm.options.d").resolve("heap.not-named-options"), + List.of("# comment", "-Xms256m", "-Xmx256m"), + StandardOpenOption.CREATE_NEW, + StandardOpenOption.APPEND + ); + final JvmOptionsParser parser = new JvmOptionsParser(); + final List jvmOptions = parser.readJvmOptionsFiles(config); + assertThat(jvmOptions, empty()); + } + + public void testFileContainsInvalidLinesThrowsParserException() throws IOException { + final Path config = newTempDir(); + final Path rootJvmOptions = config.resolve("jvm.options"); + Files.write(rootJvmOptions, List.of("XX:+UseG1GC"), StandardOpenOption.CREATE_NEW, StandardOpenOption.APPEND); + try { + final JvmOptionsParser parser = new JvmOptionsParser(); + parser.readJvmOptionsFiles(config); + fail("expected JVM options file parser exception, XX:+UseG1GC is improperly formatted"); + } catch (final JvmOptionsParser.JvmOptionsFileParserException expected) { + assertThat(expected.jvmOptionsFile(), equalTo(rootJvmOptions)); + assertThat(expected.invalidLines().entrySet(), hasSize(1)); + assertThat(expected.invalidLines(), hasKey(1)); + assertThat(expected.invalidLines().get(1), equalTo("XX:+UseG1GC")); + } + } + private void assertExpectedJvmOptions(final int javaMajorVersion, final BufferedReader br, final List expectedJvmOptions) throws IOException { final Map seenJvmOptions = new HashMap<>();