From a9984eea21b61a619dd64986e991de9e248e3682 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 8 May 2019 13:59:46 -0400 Subject: [PATCH 1/5] Remove manual parsing of JVM options This commit removes manual parsing of JVM options when calculating ergonomics. This is to avoid a situation that we parse values differently than the JVM would. In fact, we already have a bug along these lines today. It is possible to start the JVM with the same flag multiple times on the command line. In this case, the last value wins. For example, -Xmx1g -Xmx2g would start the JVM with a heap size of two gigabytes. Our JVM ergonomics ignores this possibility and instead the first value is winning! Our strategy to avoid manual parsing of the JVM options is to start yet another JVM with the same command line flags as presented and request that the JVM tell us what values it would start with. This ensures that we have the correct values when making ergonomic decisions. Moreover, our strategy also is ignoring ES_JAVA_OPTS which could override the heap size as well leading to incorrect ergonomic choices. This commit address this issue too. --- distribution/src/bin/elasticsearch | 2 +- .../src/bin/elasticsearch-service.bat | 2 +- distribution/src/bin/elasticsearch.bat | 2 +- .../tools/launchers/JvmErgonomics.java | 103 +++++++++++------- .../tools/launchers/JvmOptionsParser.java | 16 ++- .../tools/launchers/JvmErgonomicsTests.java | 67 +++++++++--- 6 files changed, 125 insertions(+), 67 deletions(-) diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch index 8bdea4950cb75..6843607efa19b 100755 --- a/distribution/src/bin/elasticsearch +++ b/distribution/src/bin/elasticsearch @@ -18,7 +18,7 @@ source "`dirname "$0"`"/elasticsearch-env ES_JVM_OPTIONS="$ES_PATH_CONF"/jvm.options JVM_OPTIONS=`"$JAVA" -cp "$ES_CLASSPATH" org.elasticsearch.tools.launchers.JvmOptionsParser "$ES_JVM_OPTIONS"` -ES_JAVA_OPTS="${JVM_OPTIONS//\$\{ES_TMPDIR\}/$ES_TMPDIR} $ES_JAVA_OPTS" +ES_JAVA_OPTS="${JVM_OPTIONS//\$\{ES_TMPDIR\}/$ES_TMPDIR}" # manual parsing to find out, if process should be detached if ! echo $* | grep -E '(^-d |-d$| -d |--daemonize$|--daemonize )' > /dev/null; then diff --git a/distribution/src/bin/elasticsearch-service.bat b/distribution/src/bin/elasticsearch-service.bat index 7a0be55c4f565..2f9c280743dfb 100644 --- a/distribution/src/bin/elasticsearch-service.bat +++ b/distribution/src/bin/elasticsearch-service.bat @@ -112,7 +112,7 @@ if not "%ES_JAVA_OPTS%" == "" set ES_JAVA_OPTS=%ES_JAVA_OPTS: =;% @setlocal for /F "usebackq delims=" %%a in (`"%JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_JVM_OPTIONS!" || echo jvm_options_parser_failed"`) do set JVM_OPTIONS=%%a -@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!% %ES_JAVA_OPTS% +@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!% if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" ( exit /b 1 diff --git a/distribution/src/bin/elasticsearch.bat b/distribution/src/bin/elasticsearch.bat index ecbbad826e797..f14185ddc4a27 100644 --- a/distribution/src/bin/elasticsearch.bat +++ b/distribution/src/bin/elasticsearch.bat @@ -44,7 +44,7 @@ IF ERRORLEVEL 1 ( set ES_JVM_OPTIONS=%ES_PATH_CONF%\jvm.options @setlocal for /F "usebackq delims=" %%a in (`CALL %JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_JVM_OPTIONS!" ^|^| echo jvm_options_parser_failed`) do set JVM_OPTIONS=%%a -@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!% %ES_JAVA_OPTS% +@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!% if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" ( exit /b 1 diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java index 761cd9e1be5db..44391e41af9cf 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java @@ -19,24 +19,27 @@ package org.elasticsearch.tools.launchers; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Tunes Elasticsearch JVM settings based on inspection of provided JVM options. */ final class JvmErgonomics { - private static final long KB = 1024L; - - private static final long MB = 1024L * 1024L; - - private static final long GB = 1024L * 1024L * 1024L; - private JvmErgonomics() { throw new AssertionError("No instances intended"); @@ -48,48 +51,65 @@ private JvmErgonomics() { * @param userDefinedJvmOptions A list of JVM options that have been defined by the user. * @return A list of additional JVM options to set. */ - static List choose(List userDefinedJvmOptions) { - List ergonomicChoices = new ArrayList<>(); - Long heapSize = extractHeapSize(userDefinedJvmOptions); - Map systemProperties = extractSystemProperties(userDefinedJvmOptions); - if (heapSize != null) { - if (systemProperties.containsKey("io.netty.allocator.type") == false) { - if (heapSize <= 1 * GB) { - ergonomicChoices.add("-Dio.netty.allocator.type=unpooled"); - } else { - ergonomicChoices.add("-Dio.netty.allocator.type=pooled"); - } + static List choose(final List userDefinedJvmOptions) throws InterruptedException, IOException { + final List ergonomicChoices = new ArrayList<>(); + final Map> finalJvmOptions = finalJvmOptions(userDefinedJvmOptions); + final long heapSize = extractHeapSize(finalJvmOptions); + final Map systemProperties = extractSystemProperties(userDefinedJvmOptions); + if (systemProperties.containsKey("io.netty.allocator.type") == false) { + if (heapSize <= 1 << 30) { + ergonomicChoices.add("-Dio.netty.allocator.type=unpooled"); + } else { + ergonomicChoices.add("-Dio.netty.allocator.type=pooled"); } } return ergonomicChoices; } - private static final Pattern MAX_HEAP_SIZE = Pattern.compile("^(-Xmx|-XX:MaxHeapSize=)(?\\d+)(?\\w)?$"); + private static final Pattern OPTION = + Pattern.compile("^\\s*\\S+\\s+(?\\S+)\\s+:?=\\s+(?\\S+)?\\s+\\{[^}]+?\\}\\s+\\{[^}]+}"); - // package private for testing - static Long extractHeapSize(List userDefinedJvmOptions) { - for (String jvmOption : userDefinedJvmOptions) { - final Matcher matcher = MAX_HEAP_SIZE.matcher(jvmOption); - if (matcher.matches()) { - final long size = Long.parseLong(matcher.group("size")); - final String unit = matcher.group("unit"); - if (unit == null) { - return size; - } else { - switch (unit.toLowerCase(Locale.ROOT)) { - case "k": - return size * KB; - case "m": - return size * MB; - case "g": - return size * GB; - default: - throw new IllegalArgumentException("Unknown unit [" + unit + "] for max heap size in [" + jvmOption + "]"); - } - } - } + static Map> finalJvmOptions( + final List userDefinedJvmOptions) throws InterruptedException, IOException { + return flagsFinal(userDefinedJvmOptions).stream() + .map(OPTION::matcher).filter(Matcher::matches) + .collect(Collectors.toUnmodifiableMap(m -> m.group("flag"), m -> Optional.ofNullable(m.group("value")))); + } + + private static List flagsFinal(final List userDefinedJvmOptions) throws InterruptedException, IOException { + final Path java = Path.of(System.getProperty("java.home"), "bin", "java"); + final List command = + Stream.of(Stream.of(java.toString()), userDefinedJvmOptions.stream(), List.of("-XX:+PrintFlagsFinal", "-version").stream()) + .reduce(Stream::concat) + .get() + .collect(Collectors.toUnmodifiableList()); + final Process process = new ProcessBuilder().command(command).start(); + final List output = readLinesFromInputStream(process.getInputStream()); + final List error = readLinesFromInputStream(process.getErrorStream()); + final int status = process.waitFor(); + if (status != 0) { + final String message = String.format( + Locale.ROOT, + "starting java failed with [%d]\noutput:\n%s\nerror:\n%s", + status, + String.join("\n", output), + String.join("\n", error)); + throw new RuntimeException(message); + } else { + return output; + } + } + + private static List readLinesFromInputStream(final InputStream is) throws IOException { + try (InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8); + BufferedReader br = new BufferedReader(isr)) { + return br.lines().collect(Collectors.toUnmodifiableList()); } - return null; + } + + // package private for testing + static Long extractHeapSize(final Map> finalJvmOptions) { + return Long.parseLong(finalJvmOptions.get("MaxHeapSize").get()); } private static final Pattern SYSTEM_PROPERTY = Pattern.compile("^-D(?[\\w+].*?)=(?.*)$"); @@ -105,4 +125,5 @@ static Map extractSystemProperties(List userDefinedJvmOp } return systemProperties; } + } 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 d74f106c50ba4..266390e32cd1e 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 @@ -19,12 +19,14 @@ package org.elasticsearch.tools.launchers; +import org.elasticsearch.tools.java_version_checker.JavaVersion; + import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; import java.util.ArrayList; @@ -37,8 +39,7 @@ import java.util.TreeMap; import java.util.regex.Matcher; import java.util.regex.Pattern; - -import org.elasticsearch.tools.java_version_checker.JavaVersion; +import java.util.stream.Collectors; /** * Parses JVM options from a file and prints a single line with all JVM options to standard output. @@ -51,14 +52,14 @@ final class JvmOptionsParser { * * @param args the args to the program which should consist of a single option, the path to the JVM options */ - public static void main(final String[] args) throws IOException { + public static void main(final String[] args) throws InterruptedException, IOException { if (args.length != 1) { throw new IllegalArgumentException("expected one argument specifying path to jvm.options but was " + Arrays.toString(args)); } final List jvmOptions = new ArrayList<>(); final SortedMap invalidLines = new TreeMap<>(); try (InputStream is = Files.newInputStream(Paths.get(args[0])); - Reader reader = new InputStreamReader(is, Charset.forName("UTF-8")); + Reader reader = new InputStreamReader(is, StandardCharsets.UTF_8); BufferedReader br = new BufferedReader(reader)) { parse( JavaVersion.majorVersion(JavaVersion.CURRENT), @@ -78,7 +79,10 @@ public void accept(final int lineNumber, final String line) { } if (invalidLines.isEmpty()) { - List ergonomicJvmOptions = JvmErgonomics.choose(jvmOptions); + // now append the JVM options from ES_JAVA_OPTS + final String environmentJvmOptions = System.getenv("ES_JAVA_OPTS"); + jvmOptions.addAll(Arrays.stream(environmentJvmOptions.split("\\s+")).collect(Collectors.toList())); + final List ergonomicJvmOptions = JvmErgonomics.choose(jvmOptions); jvmOptions.addAll(ergonomicJvmOptions); final String spaceDelimitedJvmOptions = spaceDelimitJvmOptions(jvmOptions); Launchers.outPrintln(spaceDelimitedJvmOptions); diff --git a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmErgonomicsTests.java b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmErgonomicsTests.java index 4b075d78b70a8..b5b6699f4716f 100644 --- a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmErgonomicsTests.java +++ b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmErgonomicsTests.java @@ -19,38 +19,70 @@ package org.elasticsearch.tools.launchers; +import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.hasToString; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; public class JvmErgonomicsTests extends LaunchersTestCase { - public void testExtractValidHeapSize() { - assertEquals(Long.valueOf(1024), JvmErgonomics.extractHeapSize(Collections.singletonList("-Xmx1024"))); - assertEquals(Long.valueOf(2L * 1024 * 1024 * 1024), JvmErgonomics.extractHeapSize(Collections.singletonList("-Xmx2g"))); - assertEquals(Long.valueOf(32 * 1024 * 1024), JvmErgonomics.extractHeapSize(Collections.singletonList("-Xmx32M"))); - assertEquals(Long.valueOf(32 * 1024 * 1024), JvmErgonomics.extractHeapSize(Collections.singletonList("-XX:MaxHeapSize=32M"))); + + public void testExtractValidHeapSizeUsingXmx() throws InterruptedException, IOException { + assertThat( + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx2g"))), + equalTo(2L << 30)); + } + + public void testExtractValidHeapSizeUsingMaxHeapSize() throws InterruptedException, IOException { + assertThat( + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-XX:MaxHeapSize=2g"))), + equalTo(2L << 30)); + } + + public void testExtractValidHeapSizeNoOptionPresent() throws InterruptedException, IOException { + assertThat( + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.emptyList())), + greaterThan(0L)); + } + + public void testHeapSizeInvalid() throws InterruptedException, IOException { + try { + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx2Z"))); + fail("expected starting java to fail"); + } catch (final RuntimeException e) { + assertThat(e, hasToString(containsString(("starting java failed")))); + assertThat(e, hasToString(containsString(("Invalid maximum heap size: -Xmx2Z")))); + } } - public void testExtractInvalidHeapSize() { + public void testHeapSizeTooSmall() throws InterruptedException, IOException { try { - JvmErgonomics.extractHeapSize(Collections.singletonList("-Xmx2T")); - fail("Expected IllegalArgumentException to be raised"); - } catch (IllegalArgumentException expected) { - assertEquals("Unknown unit [T] for max heap size in [-Xmx2T]", expected.getMessage()); + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx1024"))); + fail("expected starting java to fail"); + } catch (final RuntimeException e) { + assertThat(e, hasToString(containsString(("starting java failed")))); + assertThat(e, hasToString(containsString(("Too small maximum heap")))); } } - public void testExtractNoHeapSize() { - assertNull("No spaces allowed", JvmErgonomics.extractHeapSize(Collections.singletonList("-Xmx 1024"))); - assertNull("JVM option is not present", JvmErgonomics.extractHeapSize(Collections.singletonList(""))); - assertNull("Multiple JVM options per line", JvmErgonomics.extractHeapSize(Collections.singletonList("-Xms2g -Xmx2g"))); + public void testHeapSizeWithSpace() throws InterruptedException, IOException { + try { + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx 1024"))); + fail("expected starting java to fail"); + } catch (final RuntimeException e) { + assertThat(e, hasToString(containsString(("starting java failed")))); + assertThat(e, hasToString(containsString(("Invalid maximum heap size: -Xmx 1024")))); + } } public void testExtractSystemProperties() { @@ -69,15 +101,16 @@ public void testExtractNoSystemProperties() { assertTrue(parsedSystemProperties.isEmpty()); } - public void testLittleMemoryErgonomicChoices() { + public void testLittleMemoryErgonomicChoices() throws InterruptedException, IOException { String smallHeap = randomFrom(Arrays.asList("64M", "512M", "1024M", "1G")); List expectedChoices = Collections.singletonList("-Dio.netty.allocator.type=unpooled"); assertEquals(expectedChoices, JvmErgonomics.choose(Arrays.asList("-Xms" + smallHeap, "-Xmx" + smallHeap))); } - public void testPlentyMemoryErgonomicChoices() { + public void testPlentyMemoryErgonomicChoices() throws InterruptedException, IOException { String largeHeap = randomFrom(Arrays.asList("1025M", "2048M", "2G", "8G")); List expectedChoices = Collections.singletonList("-Dio.netty.allocator.type=pooled"); assertEquals(expectedChoices, JvmErgonomics.choose(Arrays.asList("-Xms" + largeHeap, "-Xmx" + largeHeap))); } + } From 5973254aaab7f622ffeaf0d487fad4f4226d6461 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 8 May 2019 14:49:54 -0400 Subject: [PATCH 2/5] Fix NPE --- .../org/elasticsearch/tools/launchers/JvmOptionsParser.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 266390e32cd1e..50e8ddeefafaa 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 @@ -81,7 +81,9 @@ public void accept(final int lineNumber, final String line) { if (invalidLines.isEmpty()) { // now append the JVM options from ES_JAVA_OPTS final String environmentJvmOptions = System.getenv("ES_JAVA_OPTS"); - jvmOptions.addAll(Arrays.stream(environmentJvmOptions.split("\\s+")).collect(Collectors.toList())); + if (environmentJvmOptions != null) { + jvmOptions.addAll(Arrays.stream(environmentJvmOptions.split("\\s+")).collect(Collectors.toList())); + } final List ergonomicJvmOptions = JvmErgonomics.choose(jvmOptions); jvmOptions.addAll(ergonomicJvmOptions); final String spaceDelimitedJvmOptions = spaceDelimitJvmOptions(jvmOptions); From 4b4d5a6c87f071853f378c6dfa0724dc067b57ca Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 8 May 2019 15:07:55 -0400 Subject: [PATCH 3/5] Small refactor --- .../java/org/elasticsearch/tools/launchers/JvmErgonomics.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java index 44391e41af9cf..8eb40b9c8491a 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java @@ -77,9 +77,9 @@ static Map> finalJvmOptions( } private static List flagsFinal(final List userDefinedJvmOptions) throws InterruptedException, IOException { - final Path java = Path.of(System.getProperty("java.home"), "bin", "java"); + final String java = Path.of(System.getProperty("java.home"), "bin", "java").toString(); final List command = - Stream.of(Stream.of(java.toString()), userDefinedJvmOptions.stream(), List.of("-XX:+PrintFlagsFinal", "-version").stream()) + Stream.of(Stream.of(java), userDefinedJvmOptions.stream(), Stream.of("-XX:+PrintFlagsFinal"), Stream.of("-version")) .reduce(Stream::concat) .get() .collect(Collectors.toUnmodifiableList()); From 8aed51e030e004711098d6a3e8918de4b917a904 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 8 May 2019 20:08:28 -0400 Subject: [PATCH 4/5] Add comment --- .../org/elasticsearch/tools/launchers/JvmErgonomics.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java index 8eb40b9c8491a..12757c970496a 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java @@ -77,6 +77,14 @@ static Map> finalJvmOptions( } private static List flagsFinal(final List userDefinedJvmOptions) throws InterruptedException, IOException { + /* + * To deduce the final set of JVM options that Elasticsearch is going to start with, we start a separate Java process with the JVM + * options that we would pass on the command line. For this Java process we will add two additional flags, -XX:+PrintFlagsFinal and + * -version. This causes the Java process that we start to parse the JVM options into their final values, display them on standard + * output, print the version to standard error, and then exit. The JVM itself never bootstraps, and therefore this process is + * lightweight. By doing this, we get the JVM options parsed exactly as the JVM that we are going to execute would parse them + * without having to implement our own JVM option parsing logic. + */ final String java = Path.of(System.getProperty("java.home"), "bin", "java").toString(); final List command = Stream.of(Stream.of(java), userDefinedJvmOptions.stream(), Stream.of("-XX:+PrintFlagsFinal"), Stream.of("-version")) From 3911d101b8ed417c8c3a7f4377fabab4a4c570df Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 8 May 2019 23:32:59 -0400 Subject: [PATCH 5/5] Fix handling of ES_JAVA_OPTS being empty --- .../org/elasticsearch/tools/launchers/JvmOptionsParser.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 50e8ddeefafaa..7894cab72a1ee 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 @@ -37,6 +37,7 @@ import java.util.Map; import java.util.SortedMap; import java.util.TreeMap; +import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -82,7 +83,9 @@ public void accept(final int lineNumber, final String line) { // now append the JVM options from ES_JAVA_OPTS final String environmentJvmOptions = System.getenv("ES_JAVA_OPTS"); if (environmentJvmOptions != null) { - jvmOptions.addAll(Arrays.stream(environmentJvmOptions.split("\\s+")).collect(Collectors.toList())); + jvmOptions.addAll(Arrays.stream(environmentJvmOptions.split("\\s+")) + .filter(Predicate.not(String::isBlank)) + .collect(Collectors.toUnmodifiableList())); } final List ergonomicJvmOptions = JvmErgonomics.choose(jvmOptions); jvmOptions.addAll(ergonomicJvmOptions);