From 99a7ea57a6e17363ef5a434fe19a60fcc7f43d63 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 15 Nov 2019 09:21:34 -0500 Subject: [PATCH] [7.4 backport] Add warnings for potential ergonomics failures for JDK8/Windows (#49043) * 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. On 7.4, we also warn for io.netty.allocator.type. * 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. * Adjust warning for io.netty.allocator.type --- .../tools/launchers/JvmErgonomics.java | 21 ++++++++++++++++++- .../tools/launchers/JvmErgonomicsTests.java | 11 +++++++--- 2 files changed, 28 insertions(+), 4 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 d18ac681d751e..c4ae40469b989 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; @@ -57,7 +59,18 @@ static List choose(final List userDefinedJvmOptions) throws Inte final Map> finalJvmOptions = finalJvmOptions(userDefinedJvmOptions); final long heapSize = extractHeapSize(finalJvmOptions); final Map systemProperties = extractSystemProperties(userDefinedJvmOptions); + + 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 (systemProperties.containsKey("io.netty.allocator.type") == false) { + if (System.getProperty("os.name").startsWith("Windows") && JavaVersion.majorVersion(JavaVersion.CURRENT) == 8) { + Launchers.errPrintln("Warning: io.netty.allocator.type may have been miscalculated due to JDK-8074459."); + Launchers.errPrintln(" Please use a newer version of Java or set io.netty.allocator.type explicitly"); + } if (heapSize <= 1 << 30) { ergonomicChoices.add("-Dio.netty.allocator.type=unpooled"); } else { @@ -65,7 +78,13 @@ static List choose(final List userDefinedJvmOptions) throws Inte } } final long maxDirectMemorySize = extractMaxDirectMemorySize(finalJvmOptions); - if (maxDirectMemorySize == 0) { + 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: 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 ecda6e1f4d830..08d8551d8482b 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; @@ -56,8 +57,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)); @@ -132,6 +133,7 @@ public void testPooledMemoryChoiceOnSmallHeap() throws InterruptedException, IOE } public void testPooledMemoryChoiceOnNotSmallHeap() 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 String largeHeap = randomFrom(Arrays.asList("1025M", "2048M", "2G", "8G")); assertThat( @@ -140,6 +142,7 @@ public void testPooledMemoryChoiceOnNotSmallHeap() throws InterruptedException, } 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)); @@ -156,8 +159,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=")))); }