Skip to content

Commit

Permalink
Warn when MaxDirectMemorySize may be incorrect (Windows/JDK8 only iss…
Browse files Browse the repository at this point in the history
…ue) (#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.
  • Loading branch information
williamrandolph committed Oct 25, 2019
1 parent d27a307 commit 1dd7ab8
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -58,6 +60,11 @@ static List<String> choose(final List<String> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -123,8 +123,9 @@ public void testExtractNoSystemProperties() {
Map<String, String> 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<String, String> heapMaxDirectMemorySize = new HashMap<>();
heapMaxDirectMemorySize.put("64M", Long.toString((64L << 20) / 2));
Expand Down

0 comments on commit 1dd7ab8

Please sign in to comment.