From 0098632dca165a583e0b46f0eba38c99d0c648aa Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Fri, 4 Dec 2020 14:34:56 -0800 Subject: [PATCH 01/35] Autodetermine heap settings based on node roles and total system memory This commit expands our JVM egonomics to also automatically determine appropriate heap size based on the total available system memory as well as the roles assigned to the node. Role determination is done via a naive parsing of elasticsearch.yml. No settings validation is done and only the 'node.roles' setting is taken into consideration. For heap purposes a node falls into one of four (4) categories: 1. A 'master-only' node. This is a node with only the 'master' role. 2. A 'ml-only' node. Similarly, a node with only the 'ml' role. 3. A 'data' node. This is basically the 'other' case. A node with any set of roles other than only master or only ml is considered a 'data' node, to include things like coordinating-only or "tie-breaker" nodes. 4. Unknown. This is the case if legacy settings are used. In this scenario we fallback to the old default heap options of 1GB. In all cases we short-circuit if a user provides explicit heap options so we only ever auto-determine heap if no existing heap options exist. Starting with this commit the default heap settings (1GB) are now removed from the default jvm.options which means we'll start auto- setting heap as the new default. --- distribution/src/config/jvm.options | 4 +- distribution/tools/launchers/build.gradle | 3 +- .../launchers/DefaultSystemMemoryInfo.java | 51 ++++ .../tools/launchers/JvmOptionsParser.java | 2 + .../tools/launchers/MachineDependentHeap.java | 238 ++++++++++++++++++ .../tools/launchers/SystemMemoryInfo.java | 42 ++++ .../launchers/MachineDependentHeapTests.java | 129 ++++++++++ .../tools/launchers/NodeRoleParserTests.java | 126 ++++++++++ .../test/resources/config/elasticsearch.yml | 1 + 9 files changed, 593 insertions(+), 3 deletions(-) create mode 100644 distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/DefaultSystemMemoryInfo.java create mode 100644 distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java create mode 100644 distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/SystemMemoryInfo.java create mode 100644 distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java create mode 100644 distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java create mode 100644 distribution/tools/launchers/src/test/resources/config/elasticsearch.yml diff --git a/distribution/src/config/jvm.options b/distribution/src/config/jvm.options index 99580b65c5934..e938b42e459d0 100644 --- a/distribution/src/config/jvm.options +++ b/distribution/src/config/jvm.options @@ -36,8 +36,8 @@ # Xms represents the initial size of the JVM heap # Xmx represents the maximum size of the JVM heap --Xms${heap.min} --Xmx${heap.max} +# -Xms${heap.min} +# -Xmx${heap.max} diff --git a/distribution/tools/launchers/build.gradle b/distribution/tools/launchers/build.gradle index 789eeb3abab7f..d45f3a1880ce1 100644 --- a/distribution/tools/launchers/build.gradle +++ b/distribution/tools/launchers/build.gradle @@ -22,6 +22,7 @@ apply plugin: 'elasticsearch.build' dependencies { compileOnly project(':distribution:tools:java-version-checker') + compileOnly "org.yaml:snakeyaml:${versions.snakeyaml}" testImplementation "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}" testImplementation "junit:junit:${versions.junit}" testImplementation "org.hamcrest:hamcrest:${versions.hamcrest}" @@ -44,4 +45,4 @@ tasks.named("testingConventions").configure { ["javadoc", "loggerUsageCheck", "jarHell"].each { tsk -> tasks.named(tsk).configure { enabled = false } -} \ No newline at end of file +} diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/DefaultSystemMemoryInfo.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/DefaultSystemMemoryInfo.java new file mode 100644 index 0000000000000..93897d8cf8fb1 --- /dev/null +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/DefaultSystemMemoryInfo.java @@ -0,0 +1,51 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.tools.launchers; + +import com.sun.management.OperatingSystemMXBean; +import org.elasticsearch.tools.java_version_checker.JavaVersion; +import org.elasticsearch.tools.java_version_checker.SuppressForbidden; + +import java.lang.management.ManagementFactory; + +/** + * A {@link SystemMemoryInfo} which delegates to {@link OperatingSystemMXBean}. + * + *

Prior to JDK 14 {@link OperatingSystemMXBean} did not take into consideration container memory limits when reporting total system + * memory. Therefore attempts to use this implementation on earlier JDKs will result in an {@link SystemMemoryInfoException}. + */ +@SuppressForbidden(reason = "Using com.sun internals is the only way to query total system memory") +public final class DefaultSystemMemoryInfo implements SystemMemoryInfo { + private final OperatingSystemMXBean operatingSystemMXBean; + + public DefaultSystemMemoryInfo() { + this.operatingSystemMXBean = (OperatingSystemMXBean) ManagementFactory.getOperatingSystemMXBean(); + } + + @Override + @SuppressWarnings("deprecation") + public long availableSystemMemory() throws SystemMemoryInfoException { + if (JavaVersion.majorVersion(JavaVersion.CURRENT) < 14) { + throw new SystemMemoryInfoException("The minimum required Java version is 14 to use " + this.getClass().getName()); + } + + return operatingSystemMXBean.getTotalPhysicalMemorySize(); + } +} 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 5f51bc2083b47..c8e26691fe2af 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 @@ -134,6 +134,7 @@ private List jvmOptions(final Path config, Path plugins, final String es throws InterruptedException, IOException, JvmOptionsFileParserException { final List jvmOptions = readJvmOptionsFiles(config); + final MachineDependentHeap machineDependentHeap = new MachineDependentHeap(new DefaultSystemMemoryInfo()); if (esJavaOpts != null) { jvmOptions.addAll( @@ -142,6 +143,7 @@ private List jvmOptions(final Path config, Path plugins, final String es } final List substitutedJvmOptions = substitutePlaceholders(jvmOptions, Collections.unmodifiableMap(substitutions)); + substitutedJvmOptions.addAll(machineDependentHeap.determineHeapSettings(config, substitutedJvmOptions)); final List ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions); final List systemJvmOptions = SystemJvmOptions.systemJvmOptions(); final List bootstrapOptions = BootstrapJvmOptions.bootstrapJvmOptions(plugins); diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java new file mode 100644 index 0000000000000..756809b951faa --- /dev/null +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java @@ -0,0 +1,238 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.tools.launchers; + +import org.yaml.snakeyaml.Yaml; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; + +import static java.lang.Math.max; +import static java.lang.Math.min; + +/** + * Determines optimal default heap settings based on available system memory and assigned node roles. + */ +public final class MachineDependentHeap { + private static final long GB = 1024L * 1024L * 1024L; // 1GB + private static final long MAX_HEAP_SIZE = GB * 31; // 31GB + private static final long MAX_ML_HEAP_SIZE = GB * 2; // 2GB + private static final long MIN_HEAP_SIZE = 1024 * 1024 * 128; // 128MB + private static final int DEFAULT_HEAP_SIZE_MB = 1024; + private static final String ELASTICSEARCH_YML = "elasticsearch.yml"; + + private final SystemMemoryInfo systemMemoryInfo; + + public MachineDependentHeap(SystemMemoryInfo systemMemoryInfo) { + this.systemMemoryInfo = systemMemoryInfo; + } + + /** + * Calculate heap options. + * + * @param configDir path to config directory + * @param userDefinedJvmOptions JVM arguments provided by the user + * @return final heap options, or an empty collection if user provided heap options are to be used + * @throws IOException if unable to load elasticsearch.yml + */ + public List determineHeapSettings(Path configDir, List userDefinedJvmOptions) throws IOException { + if (userDefinedJvmOptions.stream().anyMatch(s -> s.startsWith("-Xms") || s.startsWith("-Xmx"))) { + // User has explicitly set memory settings so we use those + return Collections.emptyList(); + } + + Path config = configDir.resolve(ELASTICSEARCH_YML); + try (InputStream in = Files.newInputStream(config)) { + return determineHeapSettings(in); + } + } + + List determineHeapSettings(InputStream config) { + MachineNodeRole nodeRole = NodeRoleParser.parse(config); + + try { + long availableSystemMemory = systemMemoryInfo.availableSystemMemory(); + return options(nodeRole.heap(availableSystemMemory)); + } catch (SystemMemoryInfo.SystemMemoryInfoException e) { + // If unable to determine system memory (ex: incompatible jdk version) fallback to defaults + return options(DEFAULT_HEAP_SIZE_MB); + } + } + + private static List options(int heapSize) { + return List.of("-Xms" + heapSize + "m", "-Xmx" + heapSize + "m"); + } + + /** + * Parses role information from elasticsearch.yml and determines machine node role. + */ + static class NodeRoleParser { + private static final Set LEGACY_ROLE_SETTINGS = Set.of( + "node.master", + "node.ingest", + "node.data", + "node.voting_only", + "node.ml", + "node.transform", + "node.remote_cluster_client" + ); + + @SuppressWarnings("unchecked") + public static MachineNodeRole parse(InputStream config) { + Yaml yaml = new Yaml(); + Map root = yaml.load(config); + + if (root != null) { + Map map = flatten(root, null); + + if (hasLegacySettings(map.keySet())) { + // We don't attempt to auto-determine heap if legacy role settings are used + return MachineNodeRole.UNKNOWN; + } else { + List roles = null; + try { + if (map.containsKey("node.roles")) { + roles = (List) map.get("node.roles"); + } + } catch (ClassCastException ex) { + throw new IllegalStateException("Unable to parse elasticsearch.yml. Expected 'node.roles' to be a list."); + } + + if (roles == null || roles.isEmpty()) { + // If roles are missing or empty (coordinating node) assume defaults and consider this a data node + return MachineNodeRole.DATA; + } else if (containsOnly(roles, "master", "voting_only")) { + return MachineNodeRole.MASTER_ONLY; + } else if (containsOnly(roles, "ml", "voting_only")) { + return MachineNodeRole.ML_ONLY; + } else { + return MachineNodeRole.DATA; + } + } + } else { // if the config is completely empty, then assume defaults and consider this a data node + return MachineNodeRole.DATA; + } + } + + /** + * Flattens a nested configuration structure. This creates a consistent way of referencing settings from a config file that uses + * a mix of object and flat setting notation. The returned map is a single-level deep structure of dot-notation property names + * to values. + * + *

No attempt is made to deterministically deal with duplicate settings, nor are they explicitly disallowed. + * + * @param config nested configuration map + * @param parentPath parent node path or {@code null} if parsing the root node + * @return flattened configuration map + */ + @SuppressWarnings("unchecked") + private static Map flatten(Map config, String parentPath) { + Map flatMap = new HashMap<>(); + String prefix = parentPath != null ? parentPath + "." : ""; + + for (Map.Entry entry : config.entrySet()) { + if (entry.getValue() instanceof Map) { + flatMap.putAll(flatten((Map) entry.getValue(), prefix + entry.getKey())); + } else { + flatMap.put(prefix + entry.getKey(), entry.getValue()); + } + } + + return flatMap; + } + + @SuppressWarnings("unchecked") + private static boolean containsOnly(Collection collection, T... items) { + return Arrays.asList(items).containsAll(collection); + } + + private static boolean hasLegacySettings(Set keys) { + return LEGACY_ROLE_SETTINGS.stream().anyMatch(keys::contains); + } + } + + enum MachineNodeRole { + /** + * Master-only node. + * + *

Heap is computed as 60% of total system memory up to a maximum of 31 gigabytes. + */ + MASTER_ONLY(m -> mb(min((long) (m * .6), MAX_HEAP_SIZE))), + + /** + * Machine learning only node. + * + *

Heap is computed as: + *

+ */ + ML_ONLY(m -> mb(m < (GB * 2) ? (long) (m * .4) : (long) min(m * .25, MAX_ML_HEAP_SIZE))), + + /** + * Data node. Essentially any node that isn't a master or ML only node. + * + *

Heap is computed as: + *

+ */ + DATA(m -> mb(m < GB ? max((long) (m * .4), MIN_HEAP_SIZE) : min((long) (m * .5), MAX_HEAP_SIZE))), + + /** + * Unknown role node. + * + *

Hard-code heap to a default of 1 gigabyte. + */ + UNKNOWN(m -> DEFAULT_HEAP_SIZE_MB); + + private final Function formula; + + MachineNodeRole(Function formula) { + this.formula = formula; + } + + /** + * Determine the appropriate heap size for the given role and available system memory. + * + * @param systemMemory total available system memory in bytes + * @return recommended heap size in megabytes + */ + public int heap(long systemMemory) { + return formula.apply(systemMemory); + } + + private static int mb(long bytes) { + return (int) (bytes / (1024 * 1024)); + } + } +} diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/SystemMemoryInfo.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/SystemMemoryInfo.java new file mode 100644 index 0000000000000..0a386435d3566 --- /dev/null +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/SystemMemoryInfo.java @@ -0,0 +1,42 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.tools.launchers; + +/** + * Determines available system memory that could be allocated for Elasticsearch, to include JVM heap and other native processes. + * The "available system memory" is defined as the total system memory which is visible to the Elasticsearch process. For instances + * in which Elasticsearch is running in a containerized environment (i.e. Docker) this is expected to be the limits set for the container, + * not the host system. + */ +public interface SystemMemoryInfo { + + /** + * + * @return total system memory available to heap or native process allocation in bytes + * @throws SystemMemoryInfoException if unable to determine available system memory + */ + long availableSystemMemory() throws SystemMemoryInfoException; + + class SystemMemoryInfoException extends Exception { + public SystemMemoryInfoException(String message) { + super(message); + } + } +} diff --git a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java new file mode 100644 index 0000000000000..41385726ee304 --- /dev/null +++ b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java @@ -0,0 +1,129 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.tools.launchers; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collections; +import java.util.List; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; +import static org.junit.Assert.assertThat; + +public class MachineDependentHeapTests extends LaunchersTestCase { + + public void testDetermineHeapSize() throws IOException { + MachineDependentHeap heap = new MachineDependentHeap(systemMemoryInGigabytes(8)); + List options = heap.determineHeapSettings(configPath(), Collections.emptyList()); + assertThat(options, containsInAnyOrder("-Xmx4096m", "-Xms4096m")); + } + + public void testUserPassedHeapArgs() throws IOException { + MachineDependentHeap heap = new MachineDependentHeap(systemMemoryInGigabytes(8)); + List options = heap.determineHeapSettings(configPath(), List.of("-Xmx4g")); + assertThat(options, empty()); + + options = heap.determineHeapSettings(configPath(), List.of("-Xms4g")); + assertThat(options, empty()); + } + + public void testMasterOnlyOptions() { + List options = calculateHeap(16, "master"); + assertThat(options, containsInAnyOrder("-Xmx9830m", "-Xms9830m")); + + options = calculateHeap(64, "master"); + assertThat(options, containsInAnyOrder("-Xmx31744m", "-Xms31744m")); + } + + public void testMlOnlyOptions() { + List options = calculateHeap(1, "ml"); + assertThat(options, containsInAnyOrder("-Xmx409m", "-Xms409m")); + + options = calculateHeap(4, "ml"); + assertThat(options, containsInAnyOrder("-Xmx1024m", "-Xms1024m")); + + options = calculateHeap(32, "ml"); + assertThat(options, containsInAnyOrder("-Xmx2048m", "-Xms2048m")); + } + + public void testDataNodeOptions() { + List options = calculateHeap(1, "data"); + assertThat(options, containsInAnyOrder("-Xmx512m", "-Xms512m")); + + options = calculateHeap(8, "data"); + assertThat(options, containsInAnyOrder("-Xmx4096m", "-Xms4096m")); + + options = calculateHeap(64, "data"); + assertThat(options, containsInAnyOrder("-Xmx31744m", "-Xms31744m")); + + options = calculateHeap(0.5, "data"); + assertThat(options, containsInAnyOrder("-Xmx204m", "-Xms204m")); + + options = calculateHeap(0.2, "data"); + assertThat(options, containsInAnyOrder("-Xmx128m", "-Xms128m")); + } + + public void testFallbackOptions() throws IOException { + MachineDependentHeap machineDependentHeap = new MachineDependentHeap(errorThrowingMemoryInfo()); + List options = machineDependentHeap.determineHeapSettings(configPath(), Collections.emptyList()); + assertThat(options, containsInAnyOrder("-Xmx1024m", "-Xms1024m")); + } + + private static List calculateHeap(double memoryInGigabytes, String... roles) { + MachineDependentHeap machineDependentHeap = new MachineDependentHeap(systemMemoryInGigabytes(memoryInGigabytes)); + String configYaml = "node.roles: [" + String.join(",", roles) + "]"; + return calculateHeap(machineDependentHeap, configYaml); + } + + private static List calculateHeap(MachineDependentHeap machineDependentHeap, String configYaml) { + try (InputStream in = new ByteArrayInputStream(configYaml.getBytes(StandardCharsets.UTF_8))) { + return machineDependentHeap.determineHeapSettings(in); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private static SystemMemoryInfo systemMemoryInGigabytes(double gigabytes) { + return () -> (long) (gigabytes * 1024 * 1024 * 1024); + } + + private static SystemMemoryInfo errorThrowingMemoryInfo() { + return () -> { + throw new SystemMemoryInfo.SystemMemoryInfoException("something went wrong"); + }; + } + + private static Path configPath() { + URL resource = MachineDependentHeapTests.class.getResource("/config/elasticsearch.yml"); + try { + return Paths.get(resource.toURI()).getParent(); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + } +} diff --git a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java new file mode 100644 index 0000000000000..352459206332e --- /dev/null +++ b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java @@ -0,0 +1,126 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.tools.launchers; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.util.function.Consumer; + +import static org.elasticsearch.tools.launchers.MachineDependentHeap.MachineNodeRole.DATA; +import static org.elasticsearch.tools.launchers.MachineDependentHeap.MachineNodeRole.MASTER_ONLY; +import static org.elasticsearch.tools.launchers.MachineDependentHeap.MachineNodeRole.ML_ONLY; +import static org.elasticsearch.tools.launchers.MachineDependentHeap.MachineNodeRole.UNKNOWN; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.fail; + +public class NodeRoleParserTests extends LaunchersTestCase { + + public void testMasterOnlyNode() { + MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> sb.append("node.roles: [master]")); + assertEquals(nodeRole, MASTER_ONLY); + + nodeRole = parseConfig(sb -> sb.append("node.roles: [master, voting_only]")); + assertEquals(nodeRole, MASTER_ONLY); + + nodeRole = parseConfig(sb -> sb.append("node.roles: [master, some_other_role]")); + assertNotEquals(nodeRole, MASTER_ONLY); + } + + public void testMlOnlyNode() { + MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> sb.append("node.roles: [ml]")); + assertEquals(nodeRole, ML_ONLY); + + nodeRole = parseConfig(sb -> sb.append("node.roles: [ml, voting_only]")); + assertEquals(nodeRole, ML_ONLY); + + nodeRole = parseConfig(sb -> sb.append("node.roles: [ml, some_other_role]")); + assertNotEquals(nodeRole, ML_ONLY); + } + + public void testDataNode() { + MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> {}); + assertEquals(nodeRole, DATA); + + nodeRole = parseConfig(sb -> sb.append("node.roles: []")); + assertEquals(nodeRole, DATA); + + nodeRole = parseConfig(sb -> sb.append("node.roles: [some_unknown_role]")); + assertEquals(nodeRole, DATA); + + nodeRole = parseConfig(sb -> sb.append("node.roles: [master, ingest]")); + assertEquals(nodeRole, DATA); + + nodeRole = parseConfig(sb -> sb.append("node.roles: [ml, master]")); + assertEquals(nodeRole, DATA); + } + + public void testLegacySettings() { + MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> sb.append("node.ml: true")); + assertEquals(nodeRole, UNKNOWN); + + nodeRole = parseConfig(sb -> sb.append("node.master: true")); + assertEquals(nodeRole, UNKNOWN); + + nodeRole = parseConfig(sb -> sb.append("node.data: false")); + assertEquals(nodeRole, UNKNOWN); + + nodeRole = parseConfig(sb -> sb.append("node.ingest: false")); + assertEquals(nodeRole, UNKNOWN); + } + + public void testYamlSyntax() { + MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> { + sb.append("node:\n"); + sb.append(" roles:\n"); + sb.append(" - master"); + }); + assertEquals(nodeRole, MASTER_ONLY); + + nodeRole = parseConfig(sb -> { + sb.append("node:\n"); + sb.append(" roles: [ml]"); + }); + assertEquals(nodeRole, ML_ONLY); + } + + public void testInvalidRoleSyntax() { + try { + parseConfig(sb -> sb.append("node.roles: foo")); + fail("expected config parse exception"); + } catch (IllegalStateException expected) { + assertEquals(expected.getMessage(), "Unable to parse elasticsearch.yml. Expected 'node.roles' to be a list."); + } + } + + private static MachineDependentHeap.MachineNodeRole parseConfig(Consumer action) { + StringBuilder sb = new StringBuilder(); + action.accept(sb); + + try (InputStream config = new ByteArrayInputStream(sb.toString().getBytes(StandardCharsets.UTF_8))) { + return MachineDependentHeap.NodeRoleParser.parse(config); + } catch (IOException ex) { + throw new UncheckedIOException(ex); + } + } +} diff --git a/distribution/tools/launchers/src/test/resources/config/elasticsearch.yml b/distribution/tools/launchers/src/test/resources/config/elasticsearch.yml new file mode 100644 index 0000000000000..4f436a154a112 --- /dev/null +++ b/distribution/tools/launchers/src/test/resources/config/elasticsearch.yml @@ -0,0 +1 @@ +node.roles: [] From 045ddea34d0e032772e1aedc8afd8fa045c7360c Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Fri, 4 Dec 2020 14:55:28 -0800 Subject: [PATCH 02/35] Fix spotless --- .../tools/launchers/MachineDependentHeapTests.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java index 41385726ee304..e1f6e96a5a7bd 100644 --- a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java +++ b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java @@ -113,9 +113,7 @@ private static SystemMemoryInfo systemMemoryInGigabytes(double gigabytes) { } private static SystemMemoryInfo errorThrowingMemoryInfo() { - return () -> { - throw new SystemMemoryInfo.SystemMemoryInfoException("something went wrong"); - }; + return () -> { throw new SystemMemoryInfo.SystemMemoryInfoException("something went wrong"); }; } private static Path configPath() { From c57b8a9b470b376d6f8fdde98a465c3950764713 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Fri, 4 Dec 2020 16:18:46 -0800 Subject: [PATCH 03/35] Fix existing packaging tests --- .../elasticsearch/packaging/test/ArchiveTests.java | 2 +- .../elasticsearch/packaging/test/PackageTests.java | 5 +++-- .../org/elasticsearch/packaging/util/DockerRun.java | 11 +++++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index d7dc6294046b1..32f14a9f96b99 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -311,7 +311,7 @@ public void test73CustomJvmOptionsDirectoryFilesWithoutOptionsExtensionIgnored() startElasticsearch(); final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes")); - assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":1073741824")); + assertThat(nodesResponse, not(containsString("\"heap_init_in_bytes\":536870912"))); stopElasticsearch(); } finally { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java index 91e805f59e8e4..8f13f90fffbcb 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java @@ -300,12 +300,13 @@ public void test81CustomPathConfAndJvmOptions() throws Exception { stopElasticsearch(); withCustomConfig(tempConf -> { - append(installation.envFile, "ES_JAVA_OPTS=-XX:-UseCompressedOops"); + append(installation.envFile, "ES_JAVA_OPTS=-Xmx512m -Xms512m -XX:-UseCompressedOops"); startElasticsearch(); final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes")); - assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":1073741824")); + assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":536870912")); + assertThat(nodesResponse, containsString("\"heap_max_in_bytes\":536870912")); assertThat(nodesResponse, containsString("\"using_compressed_ordinary_object_pointers\":\"false\"")); stopElasticsearch(); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java index dadf779b34df4..78b877d5e6d1e 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java @@ -38,6 +38,7 @@ public class DockerRun { private Integer uid; private Integer gid; private final List extraArgs = new ArrayList<>(); + private String memory = "2g"; // default to 2g memory limit private DockerRun() {} @@ -75,6 +76,11 @@ public DockerRun uid(Integer uid, Integer gid) { return this; } + public DockerRun memory(String memoryLimit) { + this.memory = memoryLimit; + return this; + } + public DockerRun extraArgs(String... args) { Collections.addAll(this.extraArgs, args); return this; @@ -88,6 +94,11 @@ String build() { // Run the container in the background cmd.add("--detach"); + // Limit container memory + if (memory != null) { + cmd.add("-m " + memory); + } + this.envVars.forEach((key, value) -> cmd.add("--env " + key + "=\"" + value + "\"")); // The container won't run without configuring discovery From 372d7e833c04d53c9380bd7ab308d2dac806e6b2 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Fri, 4 Dec 2020 17:06:58 -0800 Subject: [PATCH 04/35] More packaging test fixes --- .../org/elasticsearch/packaging/test/PackageTests.java | 2 +- .../java/org/elasticsearch/packaging/util/DockerRun.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java index 8f13f90fffbcb..6dc1b4df603ac 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java @@ -300,7 +300,7 @@ public void test81CustomPathConfAndJvmOptions() throws Exception { stopElasticsearch(); withCustomConfig(tempConf -> { - append(installation.envFile, "ES_JAVA_OPTS=-Xmx512m -Xms512m -XX:-UseCompressedOops"); + append(installation.envFile, "ES_JAVA_OPTS=\"-Xmx512m -Xms512m -XX:-UseCompressedOops\""); startElasticsearch(); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java index 78b877d5e6d1e..040655773d37a 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java @@ -77,7 +77,9 @@ public DockerRun uid(Integer uid, Integer gid) { } public DockerRun memory(String memoryLimit) { - this.memory = memoryLimit; + if (memoryLimit != null) { + this.memory = memoryLimit; + } return this; } @@ -95,9 +97,7 @@ String build() { cmd.add("--detach"); // Limit container memory - if (memory != null) { - cmd.add("-m " + memory); - } + cmd.add("-m " + memory); this.envVars.forEach((key, value) -> cmd.add("--env " + key + "=\"" + value + "\"")); From 6c49f4864fb83b0b90dafe0b0b83a9ed9d5f86c5 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Fri, 4 Dec 2020 17:40:57 -0800 Subject: [PATCH 05/35] More fixes --- .../test/java/org/elasticsearch/packaging/test/PackageTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java index 6dc1b4df603ac..fa70f8d1cb4f2 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java @@ -306,7 +306,6 @@ public void test81CustomPathConfAndJvmOptions() throws Exception { final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes")); assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":536870912")); - assertThat(nodesResponse, containsString("\"heap_max_in_bytes\":536870912")); assertThat(nodesResponse, containsString("\"using_compressed_ordinary_object_pointers\":\"false\"")); stopElasticsearch(); From afe45b7e4a09d17ea5038a46b05189b58bc2ff97 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Fri, 4 Dec 2020 18:58:26 -0800 Subject: [PATCH 06/35] Set heap size when starting windows service in packaging tests --- .../elasticsearch/packaging/test/WindowsServiceTests.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java index 05caad826fa3a..d163711d3bc00 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java @@ -215,6 +215,11 @@ public void assertStartedAndStop() throws Exception { ); } + public void test22SetDefaultJvmArgs() { + append(installation.config.resolve("jvm.options.d/heap.options"), "-Xmx1g" + System.lineSeparator()); + append(installation.config.resolve("jvm.options.d/heap.options"), "-Xms1g" + System.lineSeparator()); + } + public void test30StartStop() throws Exception { sh.run(serviceScript + " install"); assertCommand(serviceScript + " start"); From bf25202cf78c5b360fc7e2d1cff286a62b44b56f Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Fri, 4 Dec 2020 22:23:57 -0800 Subject: [PATCH 07/35] Add some better exception handling for poorly formatted config files --- .../tools/launchers/MachineDependentHeap.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java index 756809b951faa..0c71c75710661 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java @@ -20,6 +20,7 @@ package org.elasticsearch.tools.launchers; import org.yaml.snakeyaml.Yaml; +import org.yaml.snakeyaml.error.YAMLException; import java.io.IOException; import java.io.InputStream; @@ -107,7 +108,15 @@ static class NodeRoleParser { @SuppressWarnings("unchecked") public static MachineNodeRole parse(InputStream config) { Yaml yaml = new Yaml(); - Map root = yaml.load(config); + Map root; + try { + root = yaml.load(config); + } catch (ClassCastException ex) { + // Strangely formatted config, so just return defaults and let startup settings validation catch the problem + return MachineNodeRole.UNKNOWN; + } catch (YAMLException ex) { + throw new IllegalStateException("Unable to parse elasticsearch.yml:", ex); + } if (root != null) { Map map = flatten(root, null); From 6ba5c66318d81adc42aca83a54e82ecb066391cb Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Dec 2020 16:13:49 -0800 Subject: [PATCH 08/35] restore heap defaults in jvm.options --- distribution/src/config/jvm.options | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/distribution/src/config/jvm.options b/distribution/src/config/jvm.options index e938b42e459d0..99580b65c5934 100644 --- a/distribution/src/config/jvm.options +++ b/distribution/src/config/jvm.options @@ -36,8 +36,8 @@ # Xms represents the initial size of the JVM heap # Xmx represents the maximum size of the JVM heap -# -Xms${heap.min} -# -Xmx${heap.max} +-Xms${heap.min} +-Xmx${heap.max} From 8e6966c9ebf7afe7fd6fc1bfe8bc18f73fa9427a Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 9 Dec 2020 23:30:30 -0800 Subject: [PATCH 09/35] Reuse findFinalOptions from ergonomics --- .../tools/launchers/JvmErgonomics.java | 98 +------------ .../tools/launchers/JvmOption.java | 129 ++++++++++++++++++ .../tools/launchers/MachineDependentHeap.java | 7 +- .../tools/launchers/JvmErgonomicsTests.java | 18 +-- 4 files changed, 146 insertions(+), 106 deletions(-) create mode 100644 distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java 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 c74cdd525c683..ea10820fb32dc 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,22 +19,13 @@ 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. @@ -53,9 +44,9 @@ private JvmErgonomics() { */ 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 long maxDirectMemorySize = extractMaxDirectMemorySize(finalJvmOptions); + final Map finalJvmOptions = JvmOption.findFinalOptions(userDefinedJvmOptions); + final long heapSize = JvmOption.extractMaxHeapSize(finalJvmOptions); + final long maxDirectMemorySize = JvmOption.extractMaxDirectMemorySize(finalJvmOptions); if (maxDirectMemorySize == 0) { ergonomicChoices.add("-XX:MaxDirectMemorySize=" + heapSize / 2); } @@ -78,89 +69,6 @@ static List choose(final List userDefinedJvmOptions) throws Inte return ergonomicChoices; } - private static final Pattern OPTION = Pattern.compile( - "^\\s*\\S+\\s+(?\\S+)\\s+:?=\\s+(?\\S+)?\\s+\\{[^}]+?\\}\\s+\\{(?[^}]+)}" - ); - - private static class JvmOption { - private final String value; - private final String origin; - - JvmOption(String value, String origin) { - this.value = value; - this.origin = origin; - } - - public Optional getValue() { - return Optional.ofNullable(value); - } - - public String getMandatoryValue() { - return value; - } - - public boolean isCommandLineOrigin() { - return "command line".equals(this.origin); - } - } - - 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 -> new JvmOption(m.group("value"), m.group("origin")))); - } - - 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("-Xshare:off"), - Stream.of("-XX:+PrintFlagsFinal"), - Stream.of("-version") - ).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()); - } - } - - // package private for testing - static Long extractHeapSize(final Map finalJvmOptions) { - return Long.parseLong(finalJvmOptions.get("MaxHeapSize").getMandatoryValue()); - } - - static long extractMaxDirectMemorySize(final Map finalJvmOptions) { - return Long.parseLong(finalJvmOptions.get("MaxDirectMemorySize").getMandatoryValue()); - } - // Tune G1GC options for heaps < 8GB static boolean tuneG1GCForSmallHeap(final long heapSize) { return heapSize < 8L << 30; diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java new file mode 100644 index 0000000000000..8d6ff672c632e --- /dev/null +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java @@ -0,0 +1,129 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +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.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; + +class JvmOption { + private final String value; + private final String origin; + + JvmOption(String value, String origin) { + this.value = value; + this.origin = origin; + } + + public Optional getValue() { + return Optional.ofNullable(value); + } + + public String getMandatoryValue() { + return value; + } + + public boolean isCommandLineOrigin() { + return "command line".equals(this.origin); + } + + private static final Pattern OPTION = Pattern.compile( + "^\\s*\\S+\\s+(?\\S+)\\s+:?=\\s+(?\\S+)?\\s+\\{[^}]+?\\}\\s+\\{(?[^}]+)}" + ); + + public static Long extractMaxHeapSize(final Map finalJvmOptions) { + return Long.parseLong(finalJvmOptions.get("MaxHeapSize").getMandatoryValue()); + } + + public static boolean isMaxHeapSpecified(final Map finalJvmOptions) { + return finalJvmOptions.get("MaxHeapSize").isCommandLineOrigin(); + } + + public static boolean isMinHeapSpecified(final Map finalJvmOptions) { + return finalJvmOptions.get("MinHeapSize").isCommandLineOrigin(); + } + + public static long extractMaxDirectMemorySize(final Map finalJvmOptions) { + return Long.parseLong(finalJvmOptions.get("MaxDirectMemorySize").getMandatoryValue()); + } + + /** + * Determine the options present when invoking a JVM with the given user defined options. + */ + public static Map findFinalOptions(final List userDefinedJvmOptions) + throws InterruptedException, IOException { + return flagsFinal(userDefinedJvmOptions).stream() + .map(OPTION::matcher) + .filter(Matcher::matches) + .collect(Collectors.toUnmodifiableMap(m -> m.group("flag"), m -> new JvmOption(m.group("value"), m.group("origin")))); + } + + 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("-Xshare:off"), + Stream.of("-XX:+PrintFlagsFinal"), + Stream.of("-version") + ).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()); + } + } +} diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java index 0c71c75710661..d817f146f85f9 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java @@ -63,8 +63,11 @@ public MachineDependentHeap(SystemMemoryInfo systemMemoryInfo) { * @return final heap options, or an empty collection if user provided heap options are to be used * @throws IOException if unable to load elasticsearch.yml */ - public List determineHeapSettings(Path configDir, List userDefinedJvmOptions) throws IOException { - if (userDefinedJvmOptions.stream().anyMatch(s -> s.startsWith("-Xms") || s.startsWith("-Xmx"))) { + public List determineHeapSettings(Path configDir, List userDefinedJvmOptions) + throws IOException, InterruptedException { + // TODO: this could be more efficient, to only parse final options once + final Map finalJvmOptions = JvmOption.findFinalOptions(userDefinedJvmOptions); + if (JvmOption.isMaxHeapSpecified(finalJvmOptions) || JvmOption.isMinHeapSpecified(finalJvmOptions)) { // User has explicitly set memory settings so we use those return Collections.emptyList(); } 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 4eb7bc7d138d1..5bca2e69ec52c 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 @@ -42,23 +42,23 @@ public class JvmErgonomicsTests extends LaunchersTestCase { public void testExtractValidHeapSizeUsingXmx() throws InterruptedException, IOException { - assertThat(JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx2g"))), equalTo(2L << 30)); + assertThat(JvmOption.extractMaxHeapSize(JvmOption.findFinalOptions(Collections.singletonList("-Xmx2g"))), equalTo(2L << 30)); } public void testExtractValidHeapSizeUsingMaxHeapSize() throws InterruptedException, IOException { assertThat( - JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-XX:MaxHeapSize=2g"))), + JvmOption.extractMaxHeapSize(JvmOption.findFinalOptions(Collections.singletonList("-XX:MaxHeapSize=2g"))), equalTo(2L << 30) ); } public void testExtractValidHeapSizeNoOptionPresent() throws InterruptedException, IOException { - assertThat(JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.emptyList())), greaterThan(0L)); + assertThat(JvmOption.extractMaxHeapSize(JvmOption.findFinalOptions(Collections.emptyList())), greaterThan(0L)); } public void testHeapSizeInvalid() throws InterruptedException, IOException { try { - JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx2Z"))); + JvmOption.extractMaxHeapSize(JvmOption.findFinalOptions(Collections.singletonList("-Xmx2Z"))); fail("expected starting java to fail"); } catch (final RuntimeException e) { assertThat(e, hasToString(containsString(("starting java failed")))); @@ -68,7 +68,7 @@ public void testHeapSizeInvalid() throws InterruptedException, IOException { public void testHeapSizeTooSmall() throws InterruptedException, IOException { try { - JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx1024"))); + JvmOption.extractMaxHeapSize(JvmOption.findFinalOptions(Collections.singletonList("-Xmx1024"))); fail("expected starting java to fail"); } catch (final RuntimeException e) { assertThat(e, hasToString(containsString(("starting java failed")))); @@ -78,7 +78,7 @@ public void testHeapSizeTooSmall() throws InterruptedException, IOException { public void testHeapSizeWithSpace() throws InterruptedException, IOException { try { - JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx 1024"))); + JvmOption.extractMaxHeapSize(JvmOption.findFinalOptions(Collections.singletonList("-Xmx 1024"))); fail("expected starting java to fail"); } catch (final RuntimeException e) { assertThat(e, hasToString(containsString(("starting java failed")))); @@ -88,15 +88,15 @@ public void testHeapSizeWithSpace() throws InterruptedException, IOException { public void testMaxDirectMemorySizeUnset() throws InterruptedException, IOException { assertThat( - JvmErgonomics.extractMaxDirectMemorySize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx1g"))), + JvmOption.extractMaxDirectMemorySize(JvmOption.findFinalOptions(Collections.singletonList("-Xmx1g"))), equalTo(0L) ); } public void testMaxDirectMemorySizeSet() throws InterruptedException, IOException { assertThat( - JvmErgonomics.extractMaxDirectMemorySize( - JvmErgonomics.finalJvmOptions(Arrays.asList("-Xmx1g", "-XX:MaxDirectMemorySize=512m")) + JvmOption.extractMaxDirectMemorySize( + JvmOption.findFinalOptions(Arrays.asList("-Xmx1g", "-XX:MaxDirectMemorySize=512m")) ), equalTo(512L << 20) ); From e3726e7afb59403f23130dd7ec1cee4735f592d5 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 9 Dec 2020 23:41:03 -0800 Subject: [PATCH 10/35] cleanup role responses --- .../tools/launchers/MachineDependentHeap.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java index d817f146f85f9..70e40c480ad1b 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java @@ -114,11 +114,9 @@ public static MachineNodeRole parse(InputStream config) { Map root; try { root = yaml.load(config); - } catch (ClassCastException ex) { + } catch (YAMLException|ClassCastException ex) { // Strangely formatted config, so just return defaults and let startup settings validation catch the problem return MachineNodeRole.UNKNOWN; - } catch (YAMLException ex) { - throw new IllegalStateException("Unable to parse elasticsearch.yml:", ex); } if (root != null) { @@ -134,15 +132,15 @@ public static MachineNodeRole parse(InputStream config) { roles = (List) map.get("node.roles"); } } catch (ClassCastException ex) { - throw new IllegalStateException("Unable to parse elasticsearch.yml. Expected 'node.roles' to be a list."); + return MachineNodeRole.UNKNOWN; } if (roles == null || roles.isEmpty()) { // If roles are missing or empty (coordinating node) assume defaults and consider this a data node return MachineNodeRole.DATA; - } else if (containsOnly(roles, "master", "voting_only")) { + } else if (containsOnly(roles, "master")) { return MachineNodeRole.MASTER_ONLY; - } else if (containsOnly(roles, "ml", "voting_only")) { + } else if (containsOnly(roles, "ml")) { return MachineNodeRole.ML_ONLY; } else { return MachineNodeRole.DATA; From 2efc239694714b29e57022f371723c0eff288e4c Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 10 Dec 2020 00:10:36 -0800 Subject: [PATCH 11/35] address tests --- .../launchers/MachineDependentHeapTests.java | 6 +- .../tools/launchers/NodeRoleParserTests.java | 71 +++++++++---------- .../packaging/test/ArchiveTests.java | 7 +- 3 files changed, 37 insertions(+), 47 deletions(-) diff --git a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java index e1f6e96a5a7bd..70def70253fa2 100644 --- a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java +++ b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/MachineDependentHeapTests.java @@ -37,13 +37,13 @@ public class MachineDependentHeapTests extends LaunchersTestCase { - public void testDetermineHeapSize() throws IOException { + public void testDefaultHeapSize() throws Exception { MachineDependentHeap heap = new MachineDependentHeap(systemMemoryInGigabytes(8)); List options = heap.determineHeapSettings(configPath(), Collections.emptyList()); assertThat(options, containsInAnyOrder("-Xmx4096m", "-Xms4096m")); } - public void testUserPassedHeapArgs() throws IOException { + public void testUserPassedHeapArgs() throws Exception { MachineDependentHeap heap = new MachineDependentHeap(systemMemoryInGigabytes(8)); List options = heap.determineHeapSettings(configPath(), List.of("-Xmx4g")); assertThat(options, empty()); @@ -88,7 +88,7 @@ public void testDataNodeOptions() { assertThat(options, containsInAnyOrder("-Xmx128m", "-Xms128m")); } - public void testFallbackOptions() throws IOException { + public void testFallbackOptions() throws Exception { MachineDependentHeap machineDependentHeap = new MachineDependentHeap(errorThrowingMemoryInfo()); List options = machineDependentHeap.determineHeapSettings(configPath(), Collections.emptyList()); assertThat(options, containsInAnyOrder("-Xmx1024m", "-Xms1024m")); diff --git a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java index 352459206332e..4e1ee1bc18e08 100644 --- a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java +++ b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java @@ -30,97 +30,90 @@ import static org.elasticsearch.tools.launchers.MachineDependentHeap.MachineNodeRole.MASTER_ONLY; import static org.elasticsearch.tools.launchers.MachineDependentHeap.MachineNodeRole.ML_ONLY; import static org.elasticsearch.tools.launchers.MachineDependentHeap.MachineNodeRole.UNKNOWN; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.fail; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; public class NodeRoleParserTests extends LaunchersTestCase { - public void testMasterOnlyNode() { + public void testMasterOnlyNode() throws IOException { MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> sb.append("node.roles: [master]")); - assertEquals(nodeRole, MASTER_ONLY); - - nodeRole = parseConfig(sb -> sb.append("node.roles: [master, voting_only]")); - assertEquals(nodeRole, MASTER_ONLY); + assertThat(nodeRole, equalTo(MASTER_ONLY)); nodeRole = parseConfig(sb -> sb.append("node.roles: [master, some_other_role]")); - assertNotEquals(nodeRole, MASTER_ONLY); + assertThat(nodeRole, not(equalTo(MASTER_ONLY))); } - public void testMlOnlyNode() { + public void testMlOnlyNode() throws IOException { MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> sb.append("node.roles: [ml]")); - assertEquals(nodeRole, ML_ONLY); - - nodeRole = parseConfig(sb -> sb.append("node.roles: [ml, voting_only]")); - assertEquals(nodeRole, ML_ONLY); + assertThat(nodeRole, equalTo(ML_ONLY)); nodeRole = parseConfig(sb -> sb.append("node.roles: [ml, some_other_role]")); - assertNotEquals(nodeRole, ML_ONLY); + assertThat(nodeRole, not(equalTo(ML_ONLY))); } - public void testDataNode() { + public void testDataNode() throws IOException { MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> {}); - assertEquals(nodeRole, DATA); + assertThat(nodeRole, equalTo(DATA)); nodeRole = parseConfig(sb -> sb.append("node.roles: []")); - assertEquals(nodeRole, DATA); + assertThat(nodeRole, equalTo(DATA)); nodeRole = parseConfig(sb -> sb.append("node.roles: [some_unknown_role]")); - assertEquals(nodeRole, DATA); + assertThat(nodeRole, equalTo(DATA)); nodeRole = parseConfig(sb -> sb.append("node.roles: [master, ingest]")); - assertEquals(nodeRole, DATA); + assertThat(nodeRole, equalTo(DATA)); nodeRole = parseConfig(sb -> sb.append("node.roles: [ml, master]")); - assertEquals(nodeRole, DATA); + assertThat(nodeRole, equalTo(DATA)); } - public void testLegacySettings() { + public void testLegacySettings() throws IOException { MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> sb.append("node.ml: true")); - assertEquals(nodeRole, UNKNOWN); + assertThat(nodeRole, equalTo(UNKNOWN)); nodeRole = parseConfig(sb -> sb.append("node.master: true")); - assertEquals(nodeRole, UNKNOWN); + assertThat(nodeRole, equalTo(UNKNOWN)); nodeRole = parseConfig(sb -> sb.append("node.data: false")); - assertEquals(nodeRole, UNKNOWN); + assertThat(nodeRole, equalTo(UNKNOWN)); nodeRole = parseConfig(sb -> sb.append("node.ingest: false")); - assertEquals(nodeRole, UNKNOWN); + assertThat(nodeRole, equalTo(UNKNOWN)); } - public void testYamlSyntax() { + public void testYamlSyntax() throws IOException { MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> { sb.append("node:\n"); sb.append(" roles:\n"); sb.append(" - master"); }); - assertEquals(nodeRole, MASTER_ONLY); + assertThat(nodeRole, equalTo(MASTER_ONLY)); nodeRole = parseConfig(sb -> { sb.append("node:\n"); sb.append(" roles: [ml]"); }); - assertEquals(nodeRole, ML_ONLY); + assertThat(nodeRole, equalTo(ML_ONLY)); } - public void testInvalidRoleSyntax() { - try { - parseConfig(sb -> sb.append("node.roles: foo")); - fail("expected config parse exception"); - } catch (IllegalStateException expected) { - assertEquals(expected.getMessage(), "Unable to parse elasticsearch.yml. Expected 'node.roles' to be a list."); - } + public void testInvalidYaml() throws IOException { + MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> sb.append("notyaml")); + assertThat(nodeRole, equalTo(UNKNOWN)); + } + + public void testInvalidRoleSyntax() throws IOException { + MachineDependentHeap.MachineNodeRole nodeRole = parseConfig(sb -> sb.append("node.roles: foo")); + assertThat(nodeRole, equalTo(UNKNOWN)); } - private static MachineDependentHeap.MachineNodeRole parseConfig(Consumer action) { + private static MachineDependentHeap.MachineNodeRole parseConfig(Consumer action) throws IOException { StringBuilder sb = new StringBuilder(); action.accept(sb); try (InputStream config = new ByteArrayInputStream(sb.toString().getBytes(StandardCharsets.UTF_8))) { return MachineDependentHeap.NodeRoleParser.parse(config); - } catch (IOException ex) { - throw new UncheckedIOException(ex); } } } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index 32f14a9f96b99..5362057415822 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -306,13 +306,10 @@ public void test72CustomJvmOptionsDirectoryFilesAreProcessedInSortedOrder() thro public void test73CustomJvmOptionsDirectoryFilesWithoutOptionsExtensionIgnored() throws Exception { final Path jvmOptionsIgnored = installation.config(Paths.get("jvm.options.d", "jvm.options.ignored")); try { - append(jvmOptionsIgnored, "-Xms512\n-Xmx512m\n"); + append(jvmOptionsIgnored, "-Xthis_is_not_a_valid_option\n"); startElasticsearch(); - - final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes")); - assertThat(nodesResponse, not(containsString("\"heap_init_in_bytes\":536870912"))); - + ServerUtils.runElasticsearchTests(); stopElasticsearch(); } finally { rm(jvmOptionsIgnored); From 5daf41c8567939999e9dc0b0ce8facb06ca36ac0 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 10 Dec 2020 00:30:14 -0800 Subject: [PATCH 12/35] docker test --- .../java/org/elasticsearch/packaging/test/DockerTests.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index 7e54cb3188eb3..07e60b8e1260a 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -685,6 +685,13 @@ public void test140CgroupOsStatsAreAvailable() throws Exception { assertThat("Failed to find [cpuacct] in node OS cgroup stats", cgroupStats.get("cpuacct"), not(nullValue())); } + public void test150MachineDependentHeap() throws Exception { + runContainer(distribution(), builder().memory("942m")); + final Result containerLogs = getContainerLogs(); + assertThat(containerLogs.stdout, containsString("-Xmx471")); + assertThat(containerLogs.stdout, containsString("-Xms471")); + } + /** * Check that the UBI images has the correct license information in the correct place. */ From c6f432a559c40f5b99831b380fcf4a1bcd1026fe Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 10 Dec 2020 12:04:21 +0000 Subject: [PATCH 13/35] Fix up Docker test --- .../packaging/test/DockerTests.java | 44 +++++++++++++++++-- .../packaging/util/DockerRun.java | 2 +- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index 07e60b8e1260a..08dca51bc1bb3 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.packaging.test; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.http.client.fluent.Request; import org.elasticsearch.packaging.util.Distribution; import org.elasticsearch.packaging.util.Installation; @@ -35,8 +36,10 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.Set; import static java.nio.file.attribute.PosixFilePermissions.fromString; @@ -66,6 +69,7 @@ import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -685,11 +689,43 @@ public void test140CgroupOsStatsAreAvailable() throws Exception { assertThat("Failed to find [cpuacct] in node OS cgroup stats", cgroupStats.get("cpuacct"), not(nullValue())); } + /** + * Check that when available system memory is constrained by Docker, the machine-dependant heap sizing + * logic sets the correct heap size, based on the container limits. + */ public void test150MachineDependentHeap() throws Exception { - runContainer(distribution(), builder().memory("942m")); - final Result containerLogs = getContainerLogs(); - assertThat(containerLogs.stdout, containsString("-Xmx471")); - assertThat(containerLogs.stdout, containsString("-Xms471")); + // Start by ensuring `jvm.options` doesn't define any heap options + final Path jvmOptionsPath = tempDir.resolve("jvm.options"); + final Path containerJvmOptionsPath = installation.config("jvm.options"); + copyFromContainer(containerJvmOptionsPath, jvmOptionsPath); + + final List jvmOptions = Files.readAllLines(jvmOptionsPath) + .stream() + .filter(line -> (line.startsWith("-Xms") || line.startsWith("-Xmx")) == false) + .collect(Collectors.toList()); + + Files.writeString(jvmOptionsPath, String.join("\n", jvmOptions)); + + // Now run the container, being explicit about the available memory + runContainer(distribution(), builder().memory("942m").volumes(Map.of(jvmOptionsPath, containerJvmOptionsPath))); + waitForElasticsearch(installation); + + // Grab the container output and find the line where it print the JVM arguments. This will + // let us see what the automatic heap sizing calculated. + final Optional jvmArgumentsLine = getContainerLogs().stdout.lines() + .filter(line -> line.contains("JVM arguments")) + .findFirst(); + assertThat("Failed to find jvmArguments in container logs", jvmArgumentsLine.isPresent(), is(true)); + + final JsonNode jsonNode = new ObjectMapper().readTree(jvmArgumentsLine.get()); + + final String argsStr = jsonNode.get("message").textValue(); + final List xArgs = Arrays.stream(argsStr.substring(1, argsStr.length() - 1).split(",\\s*")) + .filter(arg -> arg.startsWith("-X")) + .collect(Collectors.toList()); + + // This is roughly 0.4 * 942 + assertThat(xArgs, hasItems("-Xms376", "-Xmx376")); } /** diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java index 040655773d37a..947df3d371c80 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/DockerRun.java @@ -97,7 +97,7 @@ String build() { cmd.add("--detach"); // Limit container memory - cmd.add("-m " + memory); + cmd.add("--memory " + memory); this.envVars.forEach((key, value) -> cmd.add("--env " + key + "=\"" + value + "\"")); From d44c784910cd45cecf7d54ca5800b707db84636d Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 10 Dec 2020 12:26:04 +0000 Subject: [PATCH 14/35] More fixes --- .../java/org/elasticsearch/packaging/test/DockerTests.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index 08dca51bc1bb3..013d6aeffa4b2 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -38,9 +38,11 @@ import java.nio.file.Path; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import static java.nio.file.attribute.PosixFilePermissions.fromString; import static org.elasticsearch.packaging.util.Docker.chownWithPrivilegeEscalation; @@ -725,7 +727,7 @@ public void test150MachineDependentHeap() throws Exception { .collect(Collectors.toList()); // This is roughly 0.4 * 942 - assertThat(xArgs, hasItems("-Xms376", "-Xmx376")); + assertThat(xArgs, hasItems("-Xms376m", "-Xmx376m")); } /** From 8ca205399baef8e825a0cc8d16a0b0f43952da6e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 10 Dec 2020 16:12:13 -0800 Subject: [PATCH 15/35] checkstyle --- .../org/elasticsearch/tools/launchers/NodeRoleParserTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java index 4e1ee1bc18e08..0ae805ef9fe7e 100644 --- a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java +++ b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/NodeRoleParserTests.java @@ -22,7 +22,6 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; -import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.util.function.Consumer; From f3d025bb2a336efbef9d12921761acb165c91368 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 11 Dec 2020 10:47:11 -0800 Subject: [PATCH 16/35] spotless --- .../org/elasticsearch/tools/launchers/JvmOption.java | 4 ++-- .../tools/launchers/MachineDependentHeap.java | 5 ++--- .../tools/launchers/JvmErgonomicsTests.java | 9 ++------- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java index 8d6ff672c632e..821406350532e 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java @@ -78,8 +78,8 @@ public static long extractMaxDirectMemorySize(final Map final /** * Determine the options present when invoking a JVM with the given user defined options. */ - public static Map findFinalOptions(final List userDefinedJvmOptions) - throws InterruptedException, IOException { + public static Map findFinalOptions(final List userDefinedJvmOptions) throws InterruptedException, + IOException { return flagsFinal(userDefinedJvmOptions).stream() .map(OPTION::matcher) .filter(Matcher::matches) diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java index 70e40c480ad1b..8ebf90e70e8cb 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java @@ -63,8 +63,7 @@ public MachineDependentHeap(SystemMemoryInfo systemMemoryInfo) { * @return final heap options, or an empty collection if user provided heap options are to be used * @throws IOException if unable to load elasticsearch.yml */ - public List determineHeapSettings(Path configDir, List userDefinedJvmOptions) - throws IOException, InterruptedException { + public List determineHeapSettings(Path configDir, List userDefinedJvmOptions) throws IOException, InterruptedException { // TODO: this could be more efficient, to only parse final options once final Map finalJvmOptions = JvmOption.findFinalOptions(userDefinedJvmOptions); if (JvmOption.isMaxHeapSpecified(finalJvmOptions) || JvmOption.isMinHeapSpecified(finalJvmOptions)) { @@ -114,7 +113,7 @@ public static MachineNodeRole parse(InputStream config) { Map root; try { root = yaml.load(config); - } catch (YAMLException|ClassCastException ex) { + } catch (YAMLException | ClassCastException ex) { // Strangely formatted config, so just return defaults and let startup settings validation catch the problem return MachineNodeRole.UNKNOWN; } 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 5bca2e69ec52c..7262c03e19169 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 @@ -87,17 +87,12 @@ public void testHeapSizeWithSpace() throws InterruptedException, IOException { } public void testMaxDirectMemorySizeUnset() throws InterruptedException, IOException { - assertThat( - JvmOption.extractMaxDirectMemorySize(JvmOption.findFinalOptions(Collections.singletonList("-Xmx1g"))), - equalTo(0L) - ); + assertThat(JvmOption.extractMaxDirectMemorySize(JvmOption.findFinalOptions(Collections.singletonList("-Xmx1g"))), equalTo(0L)); } public void testMaxDirectMemorySizeSet() throws InterruptedException, IOException { assertThat( - JvmOption.extractMaxDirectMemorySize( - JvmOption.findFinalOptions(Arrays.asList("-Xmx1g", "-XX:MaxDirectMemorySize=512m")) - ), + JvmOption.extractMaxDirectMemorySize(JvmOption.findFinalOptions(Arrays.asList("-Xmx1g", "-XX:MaxDirectMemorySize=512m"))), equalTo(512L << 20) ); } From ec827f0d03766376f1d394f001b300ad6b9f5919 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 12 Dec 2020 11:52:18 -0800 Subject: [PATCH 17/35] null guard --- .../java/org/elasticsearch/tools/launchers/JvmOption.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java index 821406350532e..0e1039e868c8b 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java @@ -64,11 +64,13 @@ public static Long extractMaxHeapSize(final Map finalJvmOptio } public static boolean isMaxHeapSpecified(final Map finalJvmOptions) { - return finalJvmOptions.get("MaxHeapSize").isCommandLineOrigin(); + JvmOption maxHeapSize = finalJvmOptions.get("MaxHeapSize"); + return maxHeapSize != null && maxHeapSize.isCommandLineOrigin(); } public static boolean isMinHeapSpecified(final Map finalJvmOptions) { - return finalJvmOptions.get("MinHeapSize").isCommandLineOrigin(); + JvmOption minHeapSize = finalJvmOptions.get("MinHeapSize"); + return minHeapSize != null && minHeapSize.isCommandLineOrigin(); } public static long extractMaxDirectMemorySize(final Map finalJvmOptions) { From 75cb18b89e105b375dffd6e972fc31a4b76f5c34 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Mon, 14 Dec 2020 11:04:57 -0800 Subject: [PATCH 18/35] Add fix for earlier jdks which used InitialHeap instead of MinHeap --- .../java/org/elasticsearch/tools/launchers/JvmOption.java | 5 +++++ .../elasticsearch/tools/launchers/MachineDependentHeap.java | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java index 0e1039e868c8b..688309bb1517a 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOption.java @@ -73,6 +73,11 @@ public static boolean isMinHeapSpecified(final Map finalJvmOp return minHeapSize != null && minHeapSize.isCommandLineOrigin(); } + public static boolean isInitialHeapSpecified(final Map finalJvmOptions) { + JvmOption initialHeapSize = finalJvmOptions.get("InitialHeapSize"); + return initialHeapSize != null && initialHeapSize.isCommandLineOrigin(); + } + public static long extractMaxDirectMemorySize(final Map finalJvmOptions) { return Long.parseLong(finalJvmOptions.get("MaxDirectMemorySize").getMandatoryValue()); } diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java index 8ebf90e70e8cb..ce0febebb7807 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/MachineDependentHeap.java @@ -37,6 +37,9 @@ import static java.lang.Math.max; import static java.lang.Math.min; +import static org.elasticsearch.tools.launchers.JvmOption.isInitialHeapSpecified; +import static org.elasticsearch.tools.launchers.JvmOption.isMaxHeapSpecified; +import static org.elasticsearch.tools.launchers.JvmOption.isMinHeapSpecified; /** * Determines optimal default heap settings based on available system memory and assigned node roles. @@ -66,7 +69,7 @@ public MachineDependentHeap(SystemMemoryInfo systemMemoryInfo) { public List determineHeapSettings(Path configDir, List userDefinedJvmOptions) throws IOException, InterruptedException { // TODO: this could be more efficient, to only parse final options once final Map finalJvmOptions = JvmOption.findFinalOptions(userDefinedJvmOptions); - if (JvmOption.isMaxHeapSpecified(finalJvmOptions) || JvmOption.isMinHeapSpecified(finalJvmOptions)) { + if (isMaxHeapSpecified(finalJvmOptions) || isMinHeapSpecified(finalJvmOptions) || isInitialHeapSpecified(finalJvmOptions)) { // User has explicitly set memory settings so we use those return Collections.emptyList(); } From 88a81a11a80cc2805b74913c0f337bc28c4c4474 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 14 Dec 2020 15:27:40 -0800 Subject: [PATCH 19/35] Remove Xmx/Xms from jvm.options --- distribution/src/config/jvm.options | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/distribution/src/config/jvm.options b/distribution/src/config/jvm.options index 99580b65c5934..81f476f566c2c 100644 --- a/distribution/src/config/jvm.options +++ b/distribution/src/config/jvm.options @@ -20,10 +20,13 @@ ## IMPORTANT: JVM heap size ################################################################ ## -## You must always set the initial and maximum JVM heap size to -## the same value. For example, to set the heap to 4 GB, create -## a new file in the jvm.options.d directory containing these -## lines: +## The heap size is automatically configured by Elasticsearch +## based on the available memory in your system and the roles +## each node is configured to fulfill. If specifying heap is +## required, it should be done through a file in jvm.options.d, +## and the min and max should be set to the same value. For +## example, to set the heap to 4 GB, create a new file in the +## jvm.options.d directory containing these lines: ## ## -Xms4g ## -Xmx4g @@ -33,13 +36,6 @@ ## ################################################################ -# Xms represents the initial size of the JVM heap -# Xmx represents the maximum size of the JVM heap - --Xms${heap.min} --Xmx${heap.max} - - ################################################################ ## Expert settings From ca7e564d081a87aaef734ee1557d6a9d49c5f810 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 15 Dec 2020 12:34:27 -0800 Subject: [PATCH 20/35] Set 1g heap for packaging tests by default --- .../packaging/test/PackagingTestCase.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 9a48d4b6ce032..0339ab32e13d6 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -173,6 +173,7 @@ public void setup() throws Exception { Platforms.onLinux(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome)); Platforms.onWindows(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome)); } + setHeap("1g"); } @After @@ -224,6 +225,9 @@ protected static void install() throws Exception { default: throw new IllegalStateException("Unknown Elasticsearch packaging type."); } + + // the purpose of the packaging tests are not to all test auto heap, so we explicitly set heap size to 1g + setHeap("1g"); } protected static void cleanup() throws Exception { @@ -447,4 +451,16 @@ public void withCustomConfig(CheckedConsumer action) throws Exc } IOUtils.rm(tempDir); } + + /** + * Manually set the heap size with a jvm.options.d file. This will be reset before each test. + */ + public static void setHeap(String heapSize) throws IOException { + Path heapOptions = installation.config.resolve("jvm.options.d").resolve("heap.options"); + if (heapSize == null) { + FileUtils.rm(heapOptions); + } else { + Files.writeString(heapOptions, "-Xmx=" + heapSize + "\n-Xms=" + heapSize); + } + } } From 755cfac1b1a6264fc5f5670f6ff865b765fe5f99 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 15 Dec 2020 12:43:49 -0800 Subject: [PATCH 21/35] guard for no installation yet when setting heap to 1g --- .../org/elasticsearch/packaging/test/PackagingTestCase.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 0339ab32e13d6..8d8e62258be3b 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -173,7 +173,9 @@ public void setup() throws Exception { Platforms.onLinux(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome)); Platforms.onWindows(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome)); } - setHeap("1g"); + if (installation != null) { + setHeap("1g"); + } } @After From 1c3552e932da0d30917c079f3895af56677619ad Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 15 Dec 2020 12:54:19 -0800 Subject: [PATCH 22/35] fix format oops --- .../org/elasticsearch/packaging/test/PackagingTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 8d8e62258be3b..e8de2d5e01c83 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -462,7 +462,7 @@ public static void setHeap(String heapSize) throws IOException { if (heapSize == null) { FileUtils.rm(heapOptions); } else { - Files.writeString(heapOptions, "-Xmx=" + heapSize + "\n-Xms=" + heapSize); + Files.writeString(heapOptions, "-Xmx" + heapSize + "\n-Xms" + heapSize); } } } From 472df8f75c1727dde1e609070b7f43200312e985 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 15 Dec 2020 13:32:21 -0800 Subject: [PATCH 23/35] tweak tests overriding heap --- .../packaging/test/ArchiveTests.java | 3 ++- .../packaging/test/PackageTests.java | 15 +++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index 5362057415822..4a759723e401b 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -248,7 +248,8 @@ public void test54ForceBundledJdkEmptyJavaHome() throws Exception { public void test70CustomPathConfAndJvmOptions() throws Exception { withCustomConfig(tempConf -> { - final List jvmOptions = List.of("-Xms512m", "-Xmx512m", "-Dlog4j2.disable.jmx=true"); + setHeap("512m"); + final List jvmOptions = List.of("-Dlog4j2.disable.jmx=true"); Files.write(tempConf.resolve("jvm.options"), jvmOptions, CREATE, APPEND); sh.getEnv().put("ES_JAVA_OPTS", "-XX:-UseCompressedOops"); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java index fa70f8d1cb4f2..316e021ee542a 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java @@ -133,19 +133,14 @@ public void test33RunsIfJavaNotOnPath() throws Exception { } public void test34CustomJvmOptionsDirectoryFile() throws Exception { - final Path heapOptions = installation.config(Paths.get("jvm.options.d", "heap.options")); - try { - append(heapOptions, "-Xms512m\n-Xmx512m\n"); + setHeap("512m"); - startElasticsearch(); + startElasticsearch(); - final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes")); - assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":536870912")); + final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes")); + assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":536870912")); - stopElasticsearch(); - } finally { - rm(heapOptions); - } + stopElasticsearch(); } public void test42BundledJdkRemoved() throws Exception { From 759e76f28f8a9bb9a2b16f6d92b2b20b8f26e483 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 14:05:32 -0800 Subject: [PATCH 24/35] Use temp config directory when appropriate --- .../java/org/elasticsearch/packaging/test/ArchiveTests.java | 2 +- .../org/elasticsearch/packaging/test/PackagingTestCase.java | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index 4a759723e401b..2cdc8fea801ec 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -248,7 +248,7 @@ public void test54ForceBundledJdkEmptyJavaHome() throws Exception { public void test70CustomPathConfAndJvmOptions() throws Exception { withCustomConfig(tempConf -> { - setHeap("512m"); + setHeap("512m", tempConf); final List jvmOptions = List.of("-Dlog4j2.disable.jmx=true"); Files.write(tempConf.resolve("jvm.options"), jvmOptions, CREATE, APPEND); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index e8de2d5e01c83..b828582a571ac 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -458,7 +458,11 @@ public void withCustomConfig(CheckedConsumer action) throws Exc * Manually set the heap size with a jvm.options.d file. This will be reset before each test. */ public static void setHeap(String heapSize) throws IOException { - Path heapOptions = installation.config.resolve("jvm.options.d").resolve("heap.options"); + setHeap(heapSize, installation.config); + } + + public static void setHeap(String heapSize, Path config) throws IOException { + Path heapOptions = config.resolve("jvm.options.d").resolve("heap.options"); if (heapSize == null) { FileUtils.rm(heapOptions); } else { From f407562d4b85716e165e9ec03ef9dd5582017f7e Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 15:02:52 -0800 Subject: [PATCH 25/35] More packaging test fixes --- .../java/org/elasticsearch/packaging/test/ArchiveTests.java | 2 ++ .../org/elasticsearch/packaging/test/PackagingTestCase.java | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index 2cdc8fea801ec..0f3e2d0fb9eb5 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -267,6 +267,7 @@ public void test70CustomPathConfAndJvmOptions() throws Exception { public void test71CustomJvmOptionsDirectoryFile() throws Exception { final Path heapOptions = installation.config(Paths.get("jvm.options.d", "heap.options")); try { + setHeap(null); // delete default options append(heapOptions, "-Xms512m\n-Xmx512m\n"); startElasticsearch(); @@ -284,6 +285,7 @@ public void test72CustomJvmOptionsDirectoryFilesAreProcessedInSortedOrder() thro final Path firstOptions = installation.config(Paths.get("jvm.options.d", "first.options")); final Path secondOptions = installation.config(Paths.get("jvm.options.d", "second.options")); try { + setHeap(null); // delete default options /* * We override the heap in the first file, and disable compressed oops, and override the heap in the second file. By doing this, * we can test that both files are processed by the JVM options parser, and also that they are processed in lexicographic order. diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index b828582a571ac..fd0ea32ed6b3e 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -59,6 +59,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.PosixFilePermissions; import java.util.Collections; @@ -466,7 +467,7 @@ public static void setHeap(String heapSize, Path config) throws IOException { if (heapSize == null) { FileUtils.rm(heapOptions); } else { - Files.writeString(heapOptions, "-Xmx" + heapSize + "\n-Xms" + heapSize); + Files.writeString(heapOptions, "-Xmx" + heapSize + "\n-Xms" + heapSize, StandardOpenOption.CREATE); } } } From 8c45424c412778c490c2003b557515835a1eabcb Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 15:53:04 -0800 Subject: [PATCH 26/35] Even more packaging test fixes for platform-specific line separators --- .../org/elasticsearch/packaging/test/PackagingTestCase.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index fd0ea32ed6b3e..952a473cfd1e0 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -467,7 +467,8 @@ public static void setHeap(String heapSize, Path config) throws IOException { if (heapSize == null) { FileUtils.rm(heapOptions); } else { - Files.writeString(heapOptions, "-Xmx" + heapSize + "\n-Xms" + heapSize, StandardOpenOption.CREATE); + Files.createDirectory(config.resolve("jvm.options.d")); + Files.writeString(heapOptions, String.format("-Xmx%1$s%n-Xms%1$s", heapSize), StandardOpenOption.CREATE); } } } From 557af6ac10194906115e51d7c367a136dfd2a1f1 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 16:06:09 -0800 Subject: [PATCH 27/35] Fix forbidden apis --- .../org/elasticsearch/packaging/test/PackagingTestCase.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 952a473cfd1e0..7ecbdc524f645 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -64,6 +64,7 @@ import java.nio.file.attribute.PosixFilePermissions; import java.util.Collections; import java.util.List; +import java.util.Locale; import static org.elasticsearch.packaging.util.Cleanup.cleanEverything; import static org.elasticsearch.packaging.util.Docker.ensureImageIsLoaded; @@ -468,7 +469,7 @@ public static void setHeap(String heapSize, Path config) throws IOException { FileUtils.rm(heapOptions); } else { Files.createDirectory(config.resolve("jvm.options.d")); - Files.writeString(heapOptions, String.format("-Xmx%1$s%n-Xms%1$s", heapSize), StandardOpenOption.CREATE); + Files.writeString(heapOptions, String.format(Locale.ROOT, "-Xmx%1$s%n-Xms%1$s", heapSize), StandardOpenOption.CREATE); } } } From 47baa29d5749677223abf936b1343c2f4babae8b Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 16:08:28 -0800 Subject: [PATCH 28/35] Only create parent when required --- .../org/elasticsearch/packaging/test/PackagingTestCase.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 7ecbdc524f645..7db4644196122 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -468,7 +468,9 @@ public static void setHeap(String heapSize, Path config) throws IOException { if (heapSize == null) { FileUtils.rm(heapOptions); } else { - Files.createDirectory(config.resolve("jvm.options.d")); + if (Files.exists(heapOptions.getParent()) == false) { + Files.createDirectory(heapOptions.getParent()); + } Files.writeString(heapOptions, String.format(Locale.ROOT, "-Xmx%1$s%n-Xms%1$s", heapSize), StandardOpenOption.CREATE); } } From b6032e8b286b56b31a2221dc59ecf053dd60782a Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 16:32:49 -0800 Subject: [PATCH 29/35] No need to set default args since we do it in PackagingTestCase setup --- .../elasticsearch/packaging/test/WindowsServiceTests.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java index d163711d3bc00..05caad826fa3a 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java @@ -215,11 +215,6 @@ public void assertStartedAndStop() throws Exception { ); } - public void test22SetDefaultJvmArgs() { - append(installation.config.resolve("jvm.options.d/heap.options"), "-Xmx1g" + System.lineSeparator()); - append(installation.config.resolve("jvm.options.d/heap.options"), "-Xms1g" + System.lineSeparator()); - } - public void test30StartStop() throws Exception { sh.run(serviceScript + " install"); assertCommand(serviceScript + " start"); From bce1e4f665fdb76a09542eeed842ec1aeafb9012 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 17:13:24 -0800 Subject: [PATCH 30/35] Fix docker packaging tests --- .../elasticsearch/packaging/test/PackagingTestCase.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 7db4644196122..1697cc46e8f84 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -175,7 +175,7 @@ public void setup() throws Exception { Platforms.onLinux(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome)); Platforms.onWindows(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome)); } - if (installation != null) { + if (installation != null && distribution.isDocker() == false) { setHeap("1g"); } } @@ -231,7 +231,9 @@ protected static void install() throws Exception { } // the purpose of the packaging tests are not to all test auto heap, so we explicitly set heap size to 1g - setHeap("1g"); + if (distribution.isDocker() == false) { + setHeap("1g"); + } } protected static void cleanup() throws Exception { @@ -468,9 +470,6 @@ public static void setHeap(String heapSize, Path config) throws IOException { if (heapSize == null) { FileUtils.rm(heapOptions); } else { - if (Files.exists(heapOptions.getParent()) == false) { - Files.createDirectory(heapOptions.getParent()); - } Files.writeString(heapOptions, String.format(Locale.ROOT, "-Xmx%1$s%n-Xms%1$s", heapSize), StandardOpenOption.CREATE); } } From f6fed4b0c9b8654deec58e3fef1419f45cd63946 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 17:24:37 -0800 Subject: [PATCH 31/35] Ensure we add a newline so we can safely append to this file --- .../packaging/test/PackageTests.java | 24 +++++++++---------- .../packaging/test/PackagingTestCase.java | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java index 316e021ee542a..df1888095af45 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java @@ -143,18 +143,6 @@ public void test34CustomJvmOptionsDirectoryFile() throws Exception { stopElasticsearch(); } - public void test42BundledJdkRemoved() throws Exception { - assumeThat(distribution().hasJdk, is(true)); - - Path relocatedJdk = installation.bundledJdk.getParent().resolve("jdk.relocated"); - try { - mv(installation.bundledJdk, relocatedJdk); - assertRunsWithJavaHome(); - } finally { - mv(relocatedJdk, installation.bundledJdk); - } - } - public void test40StartServer() throws Exception { String start = sh.runIgnoreExitCode("date ").stdout.trim(); startElasticsearch(); @@ -172,6 +160,18 @@ public void test40StartServer() throws Exception { stopElasticsearch(); } + public void test42BundledJdkRemoved() throws Exception { + assumeThat(distribution().hasJdk, is(true)); + + Path relocatedJdk = installation.bundledJdk.getParent().resolve("jdk.relocated"); + try { + mv(installation.bundledJdk, relocatedJdk); + assertRunsWithJavaHome(); + } finally { + mv(relocatedJdk, installation.bundledJdk); + } + } + public void test50Remove() throws Exception { // add fake bin directory as if a plugin was installed Files.createDirectories(installation.bin.resolve("myplugin")); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 1697cc46e8f84..c119404de5588 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -470,7 +470,7 @@ public static void setHeap(String heapSize, Path config) throws IOException { if (heapSize == null) { FileUtils.rm(heapOptions); } else { - Files.writeString(heapOptions, String.format(Locale.ROOT, "-Xmx%1$s%n-Xms%1$s", heapSize), StandardOpenOption.CREATE); + Files.writeString(heapOptions, String.format(Locale.ROOT, "-Xmx%1$s%n-Xms%1$s%n", heapSize), StandardOpenOption.CREATE); } } } From 7068cd02844673e2ab8b7ce790ee3cd029c9a103 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 20:16:52 -0800 Subject: [PATCH 32/35] Fix packaging test when heap is overriden --- .../org/elasticsearch/packaging/test/PackagingTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index c119404de5588..d709c825be33f 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -470,7 +470,7 @@ public static void setHeap(String heapSize, Path config) throws IOException { if (heapSize == null) { FileUtils.rm(heapOptions); } else { - Files.writeString(heapOptions, String.format(Locale.ROOT, "-Xmx%1$s%n-Xms%1$s%n", heapSize), StandardOpenOption.CREATE); + Files.writeString(heapOptions, String.format(Locale.ROOT, "-Xmx%1$s%n-Xms%1$s%n", heapSize), StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); } } } From df4ba2e47887567ee1b4d6995b906cc5190cfab7 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 20:24:38 -0800 Subject: [PATCH 33/35] Fix checkstyle --- .../elasticsearch/packaging/test/PackagingTestCase.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index d709c825be33f..e1190ebb75620 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -470,7 +470,12 @@ public static void setHeap(String heapSize, Path config) throws IOException { if (heapSize == null) { FileUtils.rm(heapOptions); } else { - Files.writeString(heapOptions, String.format(Locale.ROOT, "-Xmx%1$s%n-Xms%1$s%n", heapSize), StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); + Files.writeString( + heapOptions, + String.format(Locale.ROOT, "-Xmx%1$s%n-Xms%1$s%n", heapSize), + StandardOpenOption.CREATE, + StandardOpenOption.TRUNCATE_EXISTING + ); } } } From 2a53668d694b0846ee888b694d529e6fd1a057d4 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 15 Dec 2020 21:30:16 -0800 Subject: [PATCH 34/35] remove heap options before removing rpm --- .../org/elasticsearch/packaging/test/RpmPreservationTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java index d934f0e62c275..01e6ddd05804f 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java @@ -57,6 +57,7 @@ public void test10Install() throws Exception { } public void test20Remove() throws Exception { + setHeap(null); // remove test heap options, so the config directory can be removed remove(distribution()); // config was removed From 9b1cf77d6d76c307d4f95cf4cad6996f9634204b Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 15 Dec 2020 22:22:22 -0800 Subject: [PATCH 35/35] Fix rpm tests --- .../org/elasticsearch/packaging/test/RpmPreservationTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java index 01e6ddd05804f..bc2968208713b 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java @@ -65,6 +65,9 @@ public void test20Remove() throws Exception { // defaults file was removed assertThat(installation.envFile, fileDoesNotExist()); + + // don't perform normal setup/teardown after this since we removed the install + installation = null; } public void test30PreserveConfig() throws Exception {