From 0254f8a2e3ab7a4e5f768d68bd6d6ce57641ab44 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 21 Oct 2019 13:39:32 -0400 Subject: [PATCH 1/5] Mute test that fails with Java 8 on Windows 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. This commit mutes the test for that specific circumstance, but I will need to do some further investigation of the consequences of this behavior. --- .../elasticsearch/tools/launchers/JvmErgonomicsTests.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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..5ac7d0a0b590e 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,12 @@ 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")); + // This test fails on Windows due to a bug in JDK 8; it is fixed in JDK 9 + // A symptom of the bug is that 4g is cast to a 32-bit int and printed as 0 + assumeTrue("Fails with JDK 8 on systems where memory sizes are printed as 32-bit integers", + JavaVersion.majorVersion(JavaVersion.CURRENT) > 8 || + 0L == JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-XX:MaxHeapSize=4g"))) + ); assertThat( JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.emptyList())), greaterThan(0L)); From 706ec7ee30d795d98795aa1e208dfeb4ded2176e Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 21 Oct 2019 15:26:01 -0400 Subject: [PATCH 2/5] Simplify to match other muted tests --- .../elasticsearch/tools/launchers/JvmErgonomicsTests.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) 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 5ac7d0a0b590e..6d10bec259522 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,12 +56,7 @@ public void testExtractValidHeapSizeUsingMaxHeapSize() throws InterruptedExcepti } public void testExtractValidHeapSizeNoOptionPresent() throws InterruptedException, IOException { - // This test fails on Windows due to a bug in JDK 8; it is fixed in JDK 9 - // A symptom of the bug is that 4g is cast to a 32-bit int and printed as 0 - assumeTrue("Fails with JDK 8 on systems where memory sizes are printed as 32-bit integers", - JavaVersion.majorVersion(JavaVersion.CURRENT) > 8 || - 0L == JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-XX:MaxHeapSize=4g"))) - ); + assumeFalse(System.getProperty("os.name").startsWith("Windows") && JavaVersion.majorVersion(JavaVersion.CURRENT) == 8); assertThat( JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.emptyList())), greaterThan(0L)); From 318e6b87bc63d830fe8fc7c2e5ba2e7af138526b Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 22 Oct 2019 11:27:39 -0400 Subject: [PATCH 3/5] Add broad warning for Windows/JDK8 --- .../org/elasticsearch/tools/launchers/JvmErgonomics.java | 6 ++++++ 1 file changed, 6 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 3fd40273449bf..74cb4d781c847 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,10 @@ 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(" Please use a newer version of Java or set MaxDirectMemorySize explicitly"); + } ergonomicChoices.add("-XX:MaxDirectMemorySize=" + heapSize / 2); } return ergonomicChoices; From a804c7674eeb5d188efdb11d8da7f362401329e5 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 23 Oct 2019 17:18:14 -0400 Subject: [PATCH 4/5] Add context to comments and warnings --- .../java/org/elasticsearch/tools/launchers/JvmErgonomics.java | 1 + .../org/elasticsearch/tools/launchers/JvmErgonomicsTests.java | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) 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 74cb4d781c847..84a444d4c0fb7 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 @@ -62,6 +62,7 @@ static List choose(final List userDefinedJvmOptions) throws Inte 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. See https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8074459"); Launchers.errPrintln(" Please use a newer version of Java or set MaxDirectMemorySize explicitly"); } ergonomicChoices.add("-XX:MaxDirectMemorySize=" + heapSize / 2); 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 f2a96a8315a91..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,6 +56,7 @@ public void testExtractValidHeapSizeUsingMaxHeapSize() throws InterruptedExcepti } public void testExtractValidHeapSizeNoOptionPresent() 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); assertThat( JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.emptyList())), @@ -122,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 b91f2b7d5a2335c33aac7d26b8ffea1057341e4d Mon Sep 17 00:00:00 2001 From: William Brafford Date: Thu, 24 Oct 2019 11:09:37 -0400 Subject: [PATCH 5/5] Reference JDK bug number instead of issue tracker link --- .../java/org/elasticsearch/tools/launchers/JvmErgonomics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 84a444d4c0fb7..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 @@ -62,7 +62,7 @@ static List choose(final List userDefinedJvmOptions) throws Inte 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. See https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8074459"); + 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);