From 69954be7355183201ef58f8bcc760ec8b78c7d0a Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 25 Oct 2019 13:27:43 -0400 Subject: [PATCH 1/2] Warn when MaxDirectMemorySize may be incorrect (Windows/JDK8 only issue) (#48365) Our JVM ergonomics extract max heap size from JDK PrintFlagsFinal output. On JDK 8, there is a system-dependent bug where memory sizes are cast to 32-bit integers. On affected systems (namely, Windows), when 1/4 of physical memory is more than the maximum integer value, the output of PrintFlagsFinal will be inaccurate. In the pathological case, where the max heap size would be a multiple of 4g, the test will fail. The practical effect of this bug, beyond test failures, is that we may set MaxDirectMemorySize to an incorrect value on Windows. This commit adds a warning about this situation during startup. --- .../org/elasticsearch/tools/launchers/JvmErgonomics.java | 7 +++++++ .../elasticsearch/tools/launchers/JvmErgonomicsTests.java | 7 ++++--- 2 files changed, 11 insertions(+), 3 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 3fd40273449bf..b94859fa250e6 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,6 +19,8 @@ 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; @@ -58,6 +60,11 @@ static List choose(final List userDefinedJvmOptions) throws Inte final long heapSize = extractHeapSize(finalJvmOptions); final long maxDirectMemorySize = extractMaxDirectMemorySize(finalJvmOptions); if (maxDirectMemorySize == 0) { + if (System.getProperty("os.name").startsWith("Windows") && JavaVersion.majorVersion(JavaVersion.CURRENT) == 8) { + Launchers.errPrintln("Warning: with JDK 8 on Windows, Elasticsearch may miscalculate MaxDirectMemorySize"); + Launchers.errPrintln(" due to a JDK issue (JDK-8074459)."); + Launchers.errPrintln(" Please use a newer version of Java or set MaxDirectMemorySize explicitly"); + } ergonomicChoices.add("-XX:MaxDirectMemorySize=" + heapSize / 2); } return ergonomicChoices; 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 7337558d083c3..5fdd9923b80f4 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 @@ -56,8 +56,8 @@ public void testExtractValidHeapSizeUsingMaxHeapSize() throws InterruptedExcepti } public void testExtractValidHeapSizeNoOptionPresent() throws InterruptedException, IOException { - // Muting on Windows, awaitsfix: https://github.com/elastic/elasticsearch/issues/47384 - assumeFalse(System.getProperty("os.name").startsWith("Windows")); + // Muted for jdk8/Windows, see: https://github.com/elastic/elasticsearch/issues/47384 + assumeFalse(System.getProperty("os.name").startsWith("Windows") && JavaVersion.majorVersion(JavaVersion.CURRENT) == 8); assertThat( JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.emptyList())), greaterThan(0L)); @@ -123,8 +123,9 @@ public void testExtractNoSystemProperties() { Map parsedSystemProperties = JvmErgonomics.extractSystemProperties(Arrays.asList("-Xms1024M", "-Xmx1024M")); assertTrue(parsedSystemProperties.isEmpty()); } - + public void testMaxDirectMemorySizeChoice() throws InterruptedException, IOException { + // Muted for jdk8/Windows, see: https://github.com/elastic/elasticsearch/issues/47384 assumeFalse(System.getProperty("os.name").startsWith("Windows") && JavaVersion.majorVersion(JavaVersion.CURRENT) == 8); final Map heapMaxDirectMemorySize = new HashMap<>(); heapMaxDirectMemorySize.put("64M", Long.toString((64L << 20) / 2)); From 69da652e38c591dfa24e201f9bdef8d5371e5852 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Thu, 31 Oct 2019 15:10:17 -0400 Subject: [PATCH 2/2] Don't drop user's MaxDirectMemorySize flag on jdk8/windows (#48657) * Always pass user-specified MaxDirectMemorySize We had been testing whether a user had passed a value for MaxDirectMemorySize by parsing the output of "java -XX:PrintFlagsFinal -version". If MaxDirectMemorySize equals zero, we set it to half of max heap. The problem is that on Windows with JDK 8, a JDK bug incorrectly truncates values over 4g and returns multiples of 4g as zero. In order to always respect the user-defined settings, we need to check our input to see if an "-XX:MaxDirectMemorySize" value has been passed. * Always warn for Windows/jdk8 ergo issue Even if a user has set MaxDirectMemorySize, they aren't future-proof for this JDK bug. With this change, we issue a general warning for the windows/JDK8 issue, and a specific warning if MaxDirectMemorySize is unset. --- .../tools/launchers/JvmErgonomics.java | 16 ++++++++++++---- .../tools/launchers/JvmErgonomicsTests.java | 5 ++++- 2 files changed, 16 insertions(+), 5 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 b94859fa250e6..54c432d28c7cb 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 @@ -59,12 +59,20 @@ static List choose(final List userDefinedJvmOptions) throws Inte final Map> finalJvmOptions = finalJvmOptions(userDefinedJvmOptions); final long heapSize = extractHeapSize(finalJvmOptions); final long maxDirectMemorySize = extractMaxDirectMemorySize(finalJvmOptions); - if (maxDirectMemorySize == 0) { + + if (System.getProperty("os.name").startsWith("Windows") && JavaVersion.majorVersion(JavaVersion.CURRENT) == 8) { + Launchers.errPrintln("Warning: with JDK 8 on Windows, Elasticsearch may be unable to derive correct"); + Launchers.errPrintln(" ergonomic settings due to a JDK issue (JDK-8074459). Please use a newer"); + Launchers.errPrintln(" version of Java."); + } + + if (maxDirectMemorySize == 0 && userDefinedJvmOptions.stream().noneMatch(s -> s.startsWith("-XX:MaxDirectMemorySize"))) { + if (System.getProperty("os.name").startsWith("Windows") && JavaVersion.majorVersion(JavaVersion.CURRENT) == 8) { - Launchers.errPrintln("Warning: with JDK 8 on Windows, Elasticsearch may miscalculate MaxDirectMemorySize"); - Launchers.errPrintln(" due to a JDK issue (JDK-8074459)."); - Launchers.errPrintln(" Please use a newer version of Java or set MaxDirectMemorySize explicitly"); + Launchers.errPrintln("Warning: MaxDirectMemorySize may have been miscalculated due to JDK-8074459."); + Launchers.errPrintln(" Please use a newer version of Java or set MaxDirectMemorySize explicitly."); } + ergonomicChoices.add("-XX:MaxDirectMemorySize=" + heapSize / 2); } return ergonomicChoices; 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 5fdd9923b80f4..69cf2b437c431 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 @@ -25,6 +25,7 @@ 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.anyOf; @@ -142,8 +143,10 @@ public void testMaxDirectMemorySizeChoice() throws InterruptedException, IOExcep } public void testMaxDirectMemorySizeChoiceWhenSet() throws InterruptedException, IOException { + List derivedSettingList = JvmErgonomics.choose(Arrays.asList("-Xms5g", "-Xmx5g", "-XX:MaxDirectMemorySize=4g")); assertThat( - JvmErgonomics.choose(Arrays.asList("-Xms1g", "-Xmx1g", "-XX:MaxDirectMemorySize=1g")), + derivedSettingList, + // if MaxDirectMemorySize is set, we shouldn't derive our own value for it everyItem(not(startsWith("-XX:MaxDirectMemorySize=")))); }