From cd22163f9c299d1aad3c21a5e58867181d48a5aa Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 6 Oct 2021 12:28:11 +0100 Subject: [PATCH 01/23] Allow total memory to be overridden Since #65905 Elasticsearch has determined the Java heap settings from node roles and total system memory. This change allows the total system memory used in that calculation to be overridden with a user-specified value. This is intended to be used when Elasticsearch is running on a machine where some other software that consumes a non-negligible amount of memory is running. For example, a user could tell Elasticsearch to assume it was running on a machine with 3GB of RAM when actually it was running on a machine with 4GB of RAM. The system property is `es.total_memory_bytes`, so, for example, could be specified using `-Des.total_memory_bytes=3221225472`. (It is specified in bytes rather than using a unit, because it needs to be parsed by startup code that does not have access to the utility classes that interpret byte size units.) --- .../tools/launchers/MachineDependentHeap.java | 21 +++++- .../launchers/MachineDependentHeapTests.java | 69 +++++++++++++++---- docs/reference/cluster/nodes-stats.asciidoc | 11 +++ docs/reference/cluster/stats.asciidoc | 12 ++++ .../cluster/stats/ClusterStatsNodes.java | 17 ++++- .../org/elasticsearch/monitor/os/OsProbe.java | 26 ++++++- .../org/elasticsearch/monitor/os/OsStats.java | 29 +++++++- .../cluster/node/stats/NodeStatsTests.java | 2 +- .../monitor/os/OsProbeTests.java | 12 ++++ .../monitor/os/OsStatsTests.java | 4 +- .../xpack/ml/MachineLearning.java | 13 +++- .../xpack/ml/MachineLearningTests.java | 28 ++++---- 12 files changed, 205 insertions(+), 39 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 df025de60fb50..caa11822819dc 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 @@ -66,15 +66,30 @@ public List determineHeapSettings(Path configDir, List userDefin Path config = configDir.resolve(ELASTICSEARCH_YML); try (InputStream in = Files.newInputStream(config)) { - return determineHeapSettings(in); + return determineHeapSettings(in, parseForcedMemoryInBytes(userDefinedJvmOptions)); } } - List determineHeapSettings(InputStream config) { + static Long parseForcedMemoryInBytes(List userDefinedJvmOptions) { + String totalMemoryBytesOption = userDefinedJvmOptions.stream().filter(option -> option.startsWith("-Des.total_memory_bytes=")) + .findFirst().orElse(null); + if (totalMemoryBytesOption == null) { + return null; + } + try { + return Long.parseLong(totalMemoryBytesOption.split("=", 2)[1]); + } catch (NumberFormatException e) { + throw new RuntimeException("Unable to parse number of bytes from [" + totalMemoryBytesOption + "]"); + } + } + + List determineHeapSettings(InputStream config, Long availableSystemMemory) { MachineNodeRole nodeRole = NodeRoleParser.parse(config); try { - long availableSystemMemory = systemMemoryInfo.availableSystemMemory(); + if (availableSystemMemory == null) { + 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 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 2649c1e5aa11a..59175a19562fa 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 @@ -22,10 +22,14 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; public class MachineDependentHeapTests extends LaunchersTestCase { + private static final long BYTES_IN_GB = 1024L * 1024L * 1024L; + public void testDefaultHeapSize() throws Exception { MachineDependentHeap heap = new MachineDependentHeap(systemMemoryInGigabytes(8)); List options = heap.determineHeapSettings(configPath(), Collections.emptyList()); @@ -42,38 +46,68 @@ public void testUserPassedHeapArgs() throws Exception { } public void testMasterOnlyOptions() { - List options = calculateHeap(16, "master"); + List options = calculateHeap(16, null, "master"); + assertThat(options, containsInAnyOrder("-Xmx9830m", "-Xms9830m")); + + calculateHeap(20, 16 * BYTES_IN_GB, "master"); assertThat(options, containsInAnyOrder("-Xmx9830m", "-Xms9830m")); - options = calculateHeap(64, "master"); + options = calculateHeap(64, null, "master"); + assertThat(options, containsInAnyOrder("-Xmx31744m", "-Xms31744m")); + + options = calculateHeap(77, 64 * BYTES_IN_GB, "master"); assertThat(options, containsInAnyOrder("-Xmx31744m", "-Xms31744m")); } public void testMlOnlyOptions() { - List options = calculateHeap(1, "ml"); + List options = calculateHeap(1, null, "ml"); + assertThat(options, containsInAnyOrder("-Xmx409m", "-Xms409m")); + + options = calculateHeap(4, BYTES_IN_GB, "ml"); assertThat(options, containsInAnyOrder("-Xmx409m", "-Xms409m")); - options = calculateHeap(4, "ml"); + options = calculateHeap(4, null, "ml"); + assertThat(options, containsInAnyOrder("-Xmx1024m", "-Xms1024m")); + + options = calculateHeap(6, 4 * BYTES_IN_GB, "ml"); assertThat(options, containsInAnyOrder("-Xmx1024m", "-Xms1024m")); - options = calculateHeap(32, "ml"); + options = calculateHeap(32, null, "ml"); + assertThat(options, containsInAnyOrder("-Xmx2048m", "-Xms2048m")); + + options = calculateHeap(100, 32 * BYTES_IN_GB, "ml"); assertThat(options, containsInAnyOrder("-Xmx2048m", "-Xms2048m")); } public void testDataNodeOptions() { - List options = calculateHeap(1, "data"); + List options = calculateHeap(1, null, "data"); assertThat(options, containsInAnyOrder("-Xmx512m", "-Xms512m")); - options = calculateHeap(8, "data"); + options = calculateHeap(5, BYTES_IN_GB, "data"); + assertThat(options, containsInAnyOrder("-Xmx512m", "-Xms512m")); + + options = calculateHeap(8, null, "data"); + assertThat(options, containsInAnyOrder("-Xmx4096m", "-Xms4096m")); + + options = calculateHeap(42, 8 * BYTES_IN_GB, "data"); assertThat(options, containsInAnyOrder("-Xmx4096m", "-Xms4096m")); - options = calculateHeap(64, "data"); + options = calculateHeap(64, null, "data"); assertThat(options, containsInAnyOrder("-Xmx31744m", "-Xms31744m")); - options = calculateHeap(0.5, "data"); + options = calculateHeap(65, 64 * BYTES_IN_GB, "data"); + assertThat(options, containsInAnyOrder("-Xmx31744m", "-Xms31744m")); + + options = calculateHeap(0.5, null, "data"); assertThat(options, containsInAnyOrder("-Xmx204m", "-Xms204m")); - options = calculateHeap(0.2, "data"); + options = calculateHeap(3, BYTES_IN_GB / 2, "data"); + assertThat(options, containsInAnyOrder("-Xmx204m", "-Xms204m")); + + options = calculateHeap(0.2, null, "data"); + assertThat(options, containsInAnyOrder("-Xmx128m", "-Xms128m")); + + options = calculateHeap(1, BYTES_IN_GB / 5, "data"); assertThat(options, containsInAnyOrder("-Xmx128m", "-Xms128m")); } @@ -83,15 +117,22 @@ public void testFallbackOptions() throws Exception { assertThat(options, containsInAnyOrder("-Xmx1024m", "-Xms1024m")); } - private static List calculateHeap(double memoryInGigabytes, String... roles) { + public void testParseForcedMemoryInBytes() { + assertThat(MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Da=b", "-Dx=y")), nullValue()); + assertThat(MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Da=b", "-Des.total_memory_bytes=123456789", "-Dx=y")), + is(123456789L)); + assertThat(MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Des.total_memory_bytes=987654321")), is(987654321L)); + } + + private static List calculateHeap(double memoryInGigabytes, Long forcedMemoryInBytes, String... roles) { MachineDependentHeap machineDependentHeap = new MachineDependentHeap(systemMemoryInGigabytes(memoryInGigabytes)); String configYaml = "node.roles: [" + String.join(",", roles) + "]"; - return calculateHeap(machineDependentHeap, configYaml); + return calculateHeap(machineDependentHeap, configYaml, forcedMemoryInBytes); } - private static List calculateHeap(MachineDependentHeap machineDependentHeap, String configYaml) { + private static List calculateHeap(MachineDependentHeap machineDependentHeap, String configYaml, Long forcedMemoryInBytes) { try (InputStream in = new ByteArrayInputStream(configYaml.getBytes(StandardCharsets.UTF_8))) { - return machineDependentHeap.determineHeapSettings(in); + return machineDependentHeap.determineHeapSettings(in, forcedMemoryInBytes); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/docs/reference/cluster/nodes-stats.asciidoc b/docs/reference/cluster/nodes-stats.asciidoc index b92f3403d8aa5..1cf0b827112fb 100644 --- a/docs/reference/cluster/nodes-stats.asciidoc +++ b/docs/reference/cluster/nodes-stats.asciidoc @@ -1036,6 +1036,17 @@ Total amount of physical memory. (integer) Total amount of physical memory in bytes. +`total_override`:: +(<>) +If the amount of physical memory has been overridden using the `es.total_memory_bytes` +system property then this reports the overridden value. Otherwise it is not present. + +`total_override_in_bytes`:: +(integer) +If the amount of physical memory has been overridden using the `es.total_memory_bytes` +system property then this reports the overridden value in bytes. Otherwise it is not +present. + `free`:: (<>) Amount of free physical memory. diff --git a/docs/reference/cluster/stats.asciidoc b/docs/reference/cluster/stats.asciidoc index e9066542b0157..67d605dddd1eb 100644 --- a/docs/reference/cluster/stats.asciidoc +++ b/docs/reference/cluster/stats.asciidoc @@ -916,6 +916,18 @@ Total amount of physical memory across all selected nodes. (integer) Total amount, in bytes, of physical memory across all selected nodes. +`total_override`:: +(<>) +If the amount of physical memory has been overridden using the `es.total_memory_bytes` +system property on all selected nodes then this reports the sum of the overridden +values. Otherwise it is not present. + +`total_override_in_bytes`:: +(integer) +If the amount of physical memory has been overridden using the `es.total_memory_bytes` +system property on all selected nodes then this reports the sum of the overridden +values in bytes. Otherwise it is not present. + `free`:: (<>) Amount of free physical memory across all selected nodes. diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java index 435e2856a9c51..05008d51381a8 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java @@ -266,20 +266,33 @@ private OsStats(List nodeInfos, List nodeStatsList) { this.allocatedProcessors = allocatedProcessors; long totalMemory = 0; + Long totalMemoryOverride = 0L; long freeMemory = 0; for (NodeStats nodeStats : nodeStatsList) { if (nodeStats.getOs() != null) { - long total = nodeStats.getOs().getMem().getTotal().getBytes(); + org.elasticsearch.monitor.os.OsStats.Mem mem = nodeStats.getOs().getMem(); + long total = mem.getTotal().getBytes(); if (total > 0) { totalMemory += total; } + // Only report a total memory override for the whole cluster if every node has overridden total memory + if (totalMemoryOverride != null) { + if (mem.getTotalOverride() != null) { + long totalOverride = mem.getTotalOverride().getBytes(); + if (totalOverride > 0) { + totalMemoryOverride += totalOverride; + } + } else { + totalMemoryOverride = null; + } + } long free = nodeStats.getOs().getMem().getFree().getBytes(); if (free > 0) { freeMemory += free; } } } - this.mem = new org.elasticsearch.monitor.os.OsStats.Mem(totalMemory, freeMemory); + this.mem = new org.elasticsearch.monitor.os.OsStats.Mem(totalMemory, totalMemoryOverride, freeMemory); } public int getAvailableProcessors() { diff --git a/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java b/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java index 8ee5a945d490d..1aadc0d800dbc 100644 --- a/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java +++ b/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java @@ -60,6 +60,10 @@ public class OsProbe { private static final OperatingSystemMXBean osMxBean = ManagementFactory.getOperatingSystemMXBean(); + // This property is specified without units because it also needs to be parsed by the launcher + // code, which does not have access to all the utility classes of the Elasticsearch server. + private static final String memoryOverrideProperty = System.getProperty("es.total_memory_bytes", "-1"); + private static final Method getFreePhysicalMemorySize; private static final Method getTotalPhysicalMemorySize; private static final Method getFreeSwapSpaceSize; @@ -123,6 +127,26 @@ public long getTotalPhysicalMemorySize() { } } + /** + * Returns the overridden total amount of physical memory in bytes. + * Total memory may be overridden when some other process is running + * that is known to consume a non-negligible amount of memory. This + * is read from the "es.total_memory_bytes" system property. Negative + * values or not set at all mean no override. + */ + public Long getTotalMemoryOverride() { + return getTotalMemoryOverride(memoryOverrideProperty); + } + + static Long getTotalMemoryOverride(String memoryOverrideProperty) { + try { + long memoryOverride = Long.parseLong(memoryOverrideProperty); + return (memoryOverride < 0) ? null : memoryOverride; + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid value for [es.total_memory_bytes]: [" + memoryOverrideProperty + "]", e); + } + } + /** * Returns the amount of free swap space in bytes. */ @@ -859,7 +883,7 @@ OsStats.Cgroup getCgroup(boolean isLinux) { public OsStats osStats() { final OsStats.Cpu cpu = new OsStats.Cpu(getSystemCpuPercent(), getSystemLoadAverage()); - final OsStats.Mem mem = new OsStats.Mem(getTotalPhysicalMemorySize(), getFreePhysicalMemorySize()); + final OsStats.Mem mem = new OsStats.Mem(getTotalPhysicalMemorySize(), getTotalMemoryOverride(), getFreePhysicalMemorySize()); final OsStats.Swap swap = new OsStats.Swap(getTotalSwapSpaceSize(), getFreeSwapSpaceSize()); final OsStats.Cgroup cgroup = getCgroup(Constants.LINUX); return new OsStats(System.currentTimeMillis(), cpu, mem, swap, cgroup); diff --git a/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java b/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java index fdc48605a3eec..b0d3c5427afd0 100644 --- a/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java +++ b/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -90,6 +91,8 @@ static final class Fields { static final String USED_IN_BYTES = "used_in_bytes"; static final String TOTAL = "total"; static final String TOTAL_IN_BYTES = "total_in_bytes"; + static final String TOTAL_OVERRIDE = "total_override"; + static final String TOTAL_OVERRIDE_IN_BYTES = "total_override_in_bytes"; static final String FREE_PERCENT = "free_percent"; static final String USED_PERCENT = "used_percent"; @@ -237,25 +240,38 @@ public static class Mem implements Writeable, ToXContentFragment { private static final Logger logger = LogManager.getLogger(Mem.class); private final long total; + private final Long totalOverride; private final long free; - public Mem(long total, long free) { + public Mem(long total, Long totalOverride, long free) { assert total >= 0 : "expected total memory to be positive, got: " + total; - assert free >= 0 : "expected free memory to be positive, got: " + total; + assert totalOverride == null || totalOverride >= 0 : "expected total overridden memory to be positive, got: " + totalOverride; + assert free >= 0 : "expected free memory to be positive, got: " + free; this.total = total; + this.totalOverride = totalOverride; this.free = free; } public Mem(StreamInput in) throws IOException { this.total = in.readLong(); assert total >= 0 : "expected total memory to be positive, got: " + total; + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + this.totalOverride = in.readOptionalLong(); + assert totalOverride == null || totalOverride >= 0 + : "expected total overridden memory to be positive, got: " + totalOverride; + } else { + this.totalOverride = null; + } this.free = in.readLong(); - assert free >= 0 : "expected free memory to be positive, got: " + total; + assert free >= 0 : "expected free memory to be positive, got: " + free; } @Override public void writeTo(StreamOutput out) throws IOException { out.writeLong(total); + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + out.writeOptionalLong(totalOverride); + } out.writeLong(free); } @@ -263,6 +279,10 @@ public ByteSizeValue getTotal() { return new ByteSizeValue(total); } + public ByteSizeValue getTotalOverride() { + return totalOverride == null ? null : new ByteSizeValue(totalOverride); + } + public ByteSizeValue getUsed() { if (total == 0) { // The work in https://github.com/elastic/elasticsearch/pull/42725 established that total memory @@ -295,6 +315,9 @@ public short getFreePercent() { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(Fields.MEM); builder.humanReadableField(Fields.TOTAL_IN_BYTES, Fields.TOTAL, getTotal()); + if (getTotalOverride() != null) { + builder.humanReadableField(Fields.TOTAL_OVERRIDE_IN_BYTES, Fields.TOTAL_OVERRIDE, getTotalOverride()); + } builder.humanReadableField(Fields.FREE_IN_BYTES, Fields.FREE, getFree()); builder.humanReadableField(Fields.USED_IN_BYTES, Fields.USED, getUsed()); builder.field(Fields.FREE_PERCENT, getFreePercent()); diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java index b6d90e9e15ed3..7929322bdaa26 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -430,7 +430,7 @@ public static NodeStats createNodeStats() { long memTotal = randomNonNegativeLong(); long swapTotal = randomNonNegativeLong(); osStats = new OsStats(System.currentTimeMillis(), new OsStats.Cpu(randomShort(), loadAverages), - new OsStats.Mem(memTotal, randomLongBetween(0, memTotal)), + new OsStats.Mem(memTotal, randomFrom(randomLongBetween(1, memTotal), null), randomLongBetween(0, memTotal)), new OsStats.Swap(swapTotal, randomLongBetween(0, swapTotal)), new OsStats.Cgroup( randomAlphaOfLength(8), diff --git a/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java b/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java index 43c94b6154069..852400b402b97 100644 --- a/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java +++ b/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java @@ -28,6 +28,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; public class OsProbeTests extends ESTestCase { @@ -307,6 +308,17 @@ public void testGetTotalMemFromProcMeminfo() throws Exception { assertThat(probe.getTotalMemFromProcMeminfo(), equalTo(memTotalInKb * 1024L)); } + public void testTotalMemoryOverride() { + assertThat(OsProbe.getTotalMemoryOverride("123456789"), is(123456789L)); + assertThat(OsProbe.getTotalMemoryOverride("-1"), nullValue()); + assertThat(OsProbe.getTotalMemoryOverride("123456789123456789"), is(123456789123456789L)); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> OsProbe.getTotalMemoryOverride("abc")); + assertThat(e.getMessage(), is("Invalid value for [es.total_memory_bytes]: [abc]")); + // Although numeric, this value overflows long. This won't be a problem if the override is set appropriately on a 64 bit machine. + e = expectThrows(IllegalArgumentException.class, () -> OsProbe.getTotalMemoryOverride("123456789123456789123456789")); + assertThat(e.getMessage(), is("Invalid value for [es.total_memory_bytes]: [123456789123456789123456789]")); + } + public void testGetTotalMemoryOnDebian8() throws Exception { // tests the workaround for JDK bug on debian8: https://github.com/elastic/elasticsearch/issues/67089#issuecomment-756114654 final OsProbe osProbe = new OsProbe(); diff --git a/server/src/test/java/org/elasticsearch/monitor/os/OsStatsTests.java b/server/src/test/java/org/elasticsearch/monitor/os/OsStatsTests.java index e9b0705c69f64..9e1ee31c9fb27 100644 --- a/server/src/test/java/org/elasticsearch/monitor/os/OsStatsTests.java +++ b/server/src/test/java/org/elasticsearch/monitor/os/OsStatsTests.java @@ -26,7 +26,7 @@ public void testSerialization() throws IOException { } OsStats.Cpu cpu = new OsStats.Cpu(randomShort(), loadAverages); long memTotal = randomNonNegativeLong(); - OsStats.Mem mem = new OsStats.Mem(memTotal, randomLongBetween(0, memTotal)); + OsStats.Mem mem = new OsStats.Mem(memTotal, randomFrom(randomLongBetween(1, memTotal), null), randomLongBetween(0, memTotal)); long swapTotal = randomNonNegativeLong(); OsStats.Swap swap = new OsStats.Swap(swapTotal, randomLongBetween(0, swapTotal)); OsStats.Cgroup cgroup = new OsStats.Cgroup( @@ -73,7 +73,7 @@ public void testSerialization() throws IOException { } public void testGetUsedMemoryWithZeroTotal() { - OsStats.Mem mem = new OsStats.Mem(0, randomNonNegativeLong()); + OsStats.Mem mem = new OsStats.Mem(0, null, randomNonNegativeLong()); assertThat(mem.getUsed().getBytes(), equalTo(0L)); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java index 970f856ce5142..2d9d526d2489f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java @@ -1286,7 +1286,8 @@ public static boolean allTemplatesInstalled(ClusterState clusterState) { /** * Find the memory size (in bytes) of the machine this node is running on. - * Takes container limits (as used by Docker for example) into account. + * Takes container limits (as used by Docker for example) and forced memory + * size overrides into account. */ static long machineMemoryFromStats(OsStats stats) { long mem = stats.getMem().getTotal().getBytes(); @@ -1302,6 +1303,16 @@ static long machineMemoryFromStats(OsStats stats) { } } } + ByteSizeValue memOverride = stats.getMem().getTotalOverride(); + if (memOverride != null && memOverride.getBytes() != mem) { + long diff = memOverride.getBytes() - mem; + // Only log if the difference is more than 1% + if (Math.abs((double) diff) / (double) Math.max(memOverride.getBytes(), mem) > 0.01) { + logger.info("ML memory calculations will be based on total memory override [{}] rather than measured total memory [{}]", + memOverride, ByteSizeValue.ofBytes(mem)); + } + mem = memOverride.getBytes(); + } return mem; } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java index 48f97538ef498..48bec94d8f5ec 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java @@ -11,8 +11,6 @@ import org.elasticsearch.monitor.os.OsStats; import org.elasticsearch.test.ESTestCase; -import java.io.IOException; - import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.startsWith; import static org.mockito.Mockito.mock; @@ -96,37 +94,43 @@ public void testNoAttributes_givenClash() { "it is reserved for machine learning. If your intention was to customize machine learning, set the [xpack.ml.")); } - public void testMachineMemory_givenStatsFailure() throws IOException { + public void testMachineMemory_givenStatsFailure() { OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(0, 0)); + when(stats.getMem()).thenReturn(new OsStats.Mem(0, null, 0)); assertEquals(0L, MachineLearning.machineMemoryFromStats(stats)); } - public void testMachineMemory_givenNoCgroup() throws IOException { + public void testMachineMemory_givenOverride() { + OsStats stats = mock(OsStats.class); + when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, 1_234_567_890L, 5_368_709_120L)); + assertEquals(1_234_567_890L, MachineLearning.machineMemoryFromStats(stats)); + } + + public void testMachineMemory_givenNoCgroup() { OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, 5_368_709_120L)); + when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, null, 5_368_709_120L)); assertEquals(10_737_418_240L, MachineLearning.machineMemoryFromStats(stats)); } - public void testMachineMemory_givenCgroupNullLimit() throws IOException { + public void testMachineMemory_givenCgroupNullLimit() { OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, 5_368_709_120L)); + when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, null, 5_368_709_120L)); when(stats.getCgroup()).thenReturn(new OsStats.Cgroup("a", 1, "b", 2, 3, new OsStats.Cgroup.CpuStat(4, 5, 6), null, null, null)); assertEquals(10_737_418_240L, MachineLearning.machineMemoryFromStats(stats)); } - public void testMachineMemory_givenCgroupNoLimit() throws IOException { + public void testMachineMemory_givenCgroupNoLimit() { OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, 5_368_709_120L)); + when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, null, 5_368_709_120L)); when(stats.getCgroup()).thenReturn(new OsStats.Cgroup("a", 1, "b", 2, 3, new OsStats.Cgroup.CpuStat(4, 5, 6), "c", "18446744073709551615", "4796416")); assertEquals(10_737_418_240L, MachineLearning.machineMemoryFromStats(stats)); } - public void testMachineMemory_givenCgroupLowLimit() throws IOException { + public void testMachineMemory_givenCgroupLowLimit() { OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, 5_368_709_120L)); + when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, null, 5_368_709_120L)); when(stats.getCgroup()).thenReturn(new OsStats.Cgroup("a", 1, "b", 2, 3, new OsStats.Cgroup.CpuStat(4, 5, 6), "c", "7516192768", "4796416")); assertEquals(7_516_192_768L, MachineLearning.machineMemoryFromStats(stats)); From 0f1dd57b5353ce1d4844f2eb21a2c23e883701fd Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 6 Oct 2021 14:36:32 +0100 Subject: [PATCH 02/23] Fix formatting --- .../elasticsearch/tools/launchers/MachineDependentHeap.java | 6 ++++-- .../tools/launchers/MachineDependentHeapTests.java | 6 ++++-- 2 files changed, 8 insertions(+), 4 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 caa11822819dc..3e4b836bc1d4c 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 @@ -71,8 +71,10 @@ public List determineHeapSettings(Path configDir, List userDefin } static Long parseForcedMemoryInBytes(List userDefinedJvmOptions) { - String totalMemoryBytesOption = userDefinedJvmOptions.stream().filter(option -> option.startsWith("-Des.total_memory_bytes=")) - .findFirst().orElse(null); + String totalMemoryBytesOption = userDefinedJvmOptions.stream() + .filter(option -> option.startsWith("-Des.total_memory_bytes=")) + .findFirst() + .orElse(null); if (totalMemoryBytesOption == null) { return null; } 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 59175a19562fa..d50a9257a331a 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 @@ -119,8 +119,10 @@ public void testFallbackOptions() throws Exception { public void testParseForcedMemoryInBytes() { assertThat(MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Da=b", "-Dx=y")), nullValue()); - assertThat(MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Da=b", "-Des.total_memory_bytes=123456789", "-Dx=y")), - is(123456789L)); + assertThat( + MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Da=b", "-Des.total_memory_bytes=123456789", "-Dx=y")), + is(123456789L) + ); assertThat(MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Des.total_memory_bytes=987654321")), is(987654321L)); } From 26c557949e72d0365a20fcc5b72cdc6b738024b9 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 6 Oct 2021 15:33:03 +0100 Subject: [PATCH 03/23] Fix test --- .../capacity/memory/AutoscalingMemoryInfoServiceTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java b/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java index ce6ba2557f6f7..55295701a16d7 100644 --- a/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java +++ b/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java @@ -347,7 +347,7 @@ private static NodeStats statsForNode(DiscoveryNode node, long memory) { OsStats osStats = new OsStats( randomNonNegativeLong(), new OsStats.Cpu(randomShort(), null), - new OsStats.Mem(memory, randomLongBetween(0, memory)), + new OsStats.Mem(memory, null, randomLongBetween(0, memory)), new OsStats.Swap(randomNonNegativeLong(), randomNonNegativeLong()), null ); From 431c8bd1c95a130d714764ae2f13ee20317bae38 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 6 Oct 2021 15:49:53 +0100 Subject: [PATCH 04/23] Fix more tests --- .../collector/cluster/ClusterStatsMonitoringDocTests.java | 3 ++- .../monitoring/collector/node/NodeStatsMonitoringDocTests.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java index 8a89ea4d77136..5c473ba6628db 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java @@ -297,7 +297,7 @@ public void testToXContent() throws IOException { final OsStats mockOsStats = mock(OsStats.class); when(mockNodeStats.getOs()).thenReturn(mockOsStats); - when(mockOsStats.getMem()).thenReturn(new OsStats.Mem(100, 79)); + when(mockOsStats.getMem()).thenReturn(new OsStats.Mem(100, 99L, 79)); final ProcessStats mockProcessStats = mock(ProcessStats.class); when(mockNodeStats.getProcess()).thenReturn(mockProcessStats); @@ -517,6 +517,7 @@ public void testToXContent() throws IOException { + " ]," + " \"mem\": {" + " \"total_in_bytes\": 100," + + " \"total_override_in_bytes\": 99," + " \"free_in_bytes\": 79," + " \"used_in_bytes\": 21," + " \"free_percent\": 79," diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java index 2f78e7dc9514a..ff2c19ac4fbea 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java @@ -341,7 +341,7 @@ private static NodeStats mockNodeStats() { final OsStats.Cgroup osCgroup = new OsStats.Cgroup("_cpu_acct_ctrl_group", ++iota, "_cpu_ctrl_group", ++iota, ++iota, osCpuStat, "_memory_ctrl_group", "2000000000", "1000000000"); - final OsStats.Mem osMem = new OsStats.Mem(0, 0); + final OsStats.Mem osMem = new OsStats.Mem(0, null, 0); final OsStats.Swap osSwap = new OsStats.Swap(0, 0); final OsStats os = new OsStats(no, osCpu, osMem, osSwap, osCgroup); From 6acad57aea76d786786bba176823a54e7b755324 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 6 Oct 2021 16:51:52 +0100 Subject: [PATCH 05/23] Adding packaging test --- .../packaging/test/DockerTests.java | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 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 45689f0fed691..d163b127add3a 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 @@ -868,22 +868,45 @@ public void test140CgroupOsStatsAreAvailable() throws Exception { * logic sets the correct heap size, based on the container limits. */ public void test150MachineDependentHeap() throws Exception { + final List xArgs = machineDependentHeapTest("942m", List.of()); + + // This is roughly 0.4 * 942 + assertThat(xArgs, hasItems("-Xms376m", "-Xmx376m")); + } + + /** + * Check that when available system memory is constrained by a total memory override as well as Docker, + * the machine-dependant heap sizing logic sets the correct heap size, preferring the override to the + * container limits. + */ + public void test151MachineDependentHeapWithSizeOverride() throws Exception { + final List xArgs = machineDependentHeapTest( + "942m", + // 799014912 = 762m + List.of("-Des.total_memory_override=799014912") + ); + + // This is roughly 0.4 * 762, in particular it's NOT 0.4 * 942 + assertThat(xArgs, hasItems("-Xms304m", "-Xmx304m")); + } + + private List machineDependentHeapTest(final String containerMemory, final List extraJvmOptions) throws Exception { // 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()); + final List jvmOptions = Stream.concat( + Files.readAllLines(jvmOptionsPath).stream().filter(line -> (line.startsWith("-Xms") || line.startsWith("-Xmx")) == false), + extraJvmOptions.stream() + ).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") + builder().memory(containerMemory) .volumes(Map.of(jvmOptionsPath, containerJvmOptionsPath)) .envVars(Map.of("ingest.geoip.downloader.enabled", "false", "ELASTIC_PASSWORD", PASSWORD)) ); @@ -899,12 +922,9 @@ public void test150MachineDependentHeap() throws Exception { 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*")) + return 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("-Xms376m", "-Xmx376m")); } /** From 0f27a97d222a8286939f8c9b8a53baedfeb5d4c2 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 6 Oct 2021 16:59:45 +0100 Subject: [PATCH 06/23] Update docs/changelog/78750.yaml --- docs/changelog/78750.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/78750.yaml diff --git a/docs/changelog/78750.yaml b/docs/changelog/78750.yaml new file mode 100644 index 0000000000000..7754b491481e2 --- /dev/null +++ b/docs/changelog/78750.yaml @@ -0,0 +1,6 @@ +pr: 78750 +summary: Allow total memory to be overridden +area: "Infra/Core, Machine Learning, Packaging" +type: enhancement +issues: + - 65905 From 824019a98a035ade24ecdef9842de3605c964fdb Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 6 Oct 2021 17:10:39 +0100 Subject: [PATCH 07/23] Fix changelog --- docs/changelog/78750.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/changelog/78750.yaml b/docs/changelog/78750.yaml index 7754b491481e2..a12f0b1003fb4 100644 --- a/docs/changelog/78750.yaml +++ b/docs/changelog/78750.yaml @@ -1,6 +1,4 @@ pr: 78750 summary: Allow total memory to be overridden -area: "Infra/Core, Machine Learning, Packaging" +area: "Packaging" type: enhancement -issues: - - 65905 From 898fec6ecc196ec01295fd736e7118a33564b6ac Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 6 Oct 2021 17:11:25 +0100 Subject: [PATCH 08/23] Update docs/changelog/78750.yaml --- docs/changelog/78750.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/changelog/78750.yaml b/docs/changelog/78750.yaml index a12f0b1003fb4..c5f06c8783460 100644 --- a/docs/changelog/78750.yaml +++ b/docs/changelog/78750.yaml @@ -1,4 +1,6 @@ pr: 78750 summary: Allow total memory to be overridden -area: "Packaging" +area: Packaging type: enhancement +issues: + - 65905 From 82c79375a26cb367017681dc282a04f28ae5d972 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 6 Oct 2021 17:21:04 +0100 Subject: [PATCH 09/23] Fix test --- .../test/java/org/elasticsearch/packaging/test/DockerTests.java | 2 +- 1 file changed, 1 insertion(+), 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 d163b127add3a..1e727b9680144 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 @@ -883,7 +883,7 @@ public void test151MachineDependentHeapWithSizeOverride() throws Exception { final List xArgs = machineDependentHeapTest( "942m", // 799014912 = 762m - List.of("-Des.total_memory_override=799014912") + List.of("-Des.total_memory_bytes=799014912") ); // This is roughly 0.4 * 762, in particular it's NOT 0.4 * 942 From d1d58e67af6a60b5735145eb825a5f08d6c053d4 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 6 Oct 2021 17:34:31 +0100 Subject: [PATCH 10/23] Adding a launcher test for option parse failure --- .../tools/launchers/MachineDependentHeap.java | 2 +- .../tools/launchers/MachineDependentHeapTests.java | 7 +++++++ 2 files changed, 8 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 3e4b836bc1d4c..7c7526ec7c171 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 @@ -81,7 +81,7 @@ static Long parseForcedMemoryInBytes(List userDefinedJvmOptions) { try { return Long.parseLong(totalMemoryBytesOption.split("=", 2)[1]); } catch (NumberFormatException e) { - throw new RuntimeException("Unable to parse number of bytes from [" + totalMemoryBytesOption + "]"); + throw new IllegalArgumentException("Unable to parse number of bytes from [" + totalMemoryBytesOption + "]"); } } 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 d50a9257a331a..a5fa42d5b0378 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 @@ -25,6 +25,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; public class MachineDependentHeapTests extends LaunchersTestCase { @@ -124,6 +125,12 @@ public void testParseForcedMemoryInBytes() { is(123456789L) ); assertThat(MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Des.total_memory_bytes=987654321")), is(987654321L)); + try { + MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Des.total_memory_bytes=invalid")); + fail("expected parse to fail"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), is("Unable to parse number of bytes from [-Des.total_memory_bytes=invalid]")); + } } private static List calculateHeap(double memoryInGigabytes, Long forcedMemoryInBytes, String... roles) { From 017263b6fbed1c50e98c428875d7e7bf52c46783 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Thu, 7 Oct 2021 12:10:29 +0100 Subject: [PATCH 11/23] Addressing comments related to the launcher code --- .../tools/launchers/JvmOptionsParser.java | 4 +- .../tools/launchers/MachineDependentHeap.java | 23 +---- .../OverridableSystemMemoryInfo.java | 48 +++++++++++ .../launchers/MachineDependentHeapTests.java | 78 +++-------------- .../OverridableSystemMemoryInfoTests.java | 86 +++++++++++++++++++ 5 files changed, 154 insertions(+), 85 deletions(-) create mode 100644 distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfo.java create mode 100644 distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfoTests.java 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 bb6da0c6a4eb5..4f33a1ba5b02c 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 @@ -123,7 +123,6 @@ 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( @@ -132,6 +131,9 @@ private List jvmOptions(final Path config, Path plugins, final String es } final List substitutedJvmOptions = substitutePlaceholders(jvmOptions, Collections.unmodifiableMap(substitutions)); + final MachineDependentHeap machineDependentHeap = new MachineDependentHeap( + new OverridableSystemMemoryInfo(substitutedJvmOptions, new DefaultSystemMemoryInfo()) + ); substitutedJvmOptions.addAll(machineDependentHeap.determineHeapSettings(config, substitutedJvmOptions)); final List ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions); final List systemJvmOptions = SystemJvmOptions.systemJvmOptions(); 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 7c7526ec7c171..df025de60fb50 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 @@ -66,32 +66,15 @@ public List determineHeapSettings(Path configDir, List userDefin Path config = configDir.resolve(ELASTICSEARCH_YML); try (InputStream in = Files.newInputStream(config)) { - return determineHeapSettings(in, parseForcedMemoryInBytes(userDefinedJvmOptions)); + return determineHeapSettings(in); } } - static Long parseForcedMemoryInBytes(List userDefinedJvmOptions) { - String totalMemoryBytesOption = userDefinedJvmOptions.stream() - .filter(option -> option.startsWith("-Des.total_memory_bytes=")) - .findFirst() - .orElse(null); - if (totalMemoryBytesOption == null) { - return null; - } - try { - return Long.parseLong(totalMemoryBytesOption.split("=", 2)[1]); - } catch (NumberFormatException e) { - throw new IllegalArgumentException("Unable to parse number of bytes from [" + totalMemoryBytesOption + "]"); - } - } - - List determineHeapSettings(InputStream config, Long availableSystemMemory) { + List determineHeapSettings(InputStream config) { MachineNodeRole nodeRole = NodeRoleParser.parse(config); try { - if (availableSystemMemory == null) { - availableSystemMemory = systemMemoryInfo.availableSystemMemory(); - } + 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 diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfo.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfo.java new file mode 100644 index 0000000000000..1226831bc2f51 --- /dev/null +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfo.java @@ -0,0 +1,48 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.tools.launchers; + +import java.util.List; +import java.util.Objects; + +/** + * A {@link SystemMemoryInfo} which returns a user-overridden memory size if + * has been specified using the {@code es.total_memory_bytes} system property, or + * else returns the value provided by a fallback provider. + */ +public final class OverridableSystemMemoryInfo implements SystemMemoryInfo { + + private final List userDefinedJvmOptions; + private final SystemMemoryInfo fallbackSystemMemoryInfo; + + public OverridableSystemMemoryInfo(final List userDefinedJvmOptions, SystemMemoryInfo fallbackSystemMemoryInfo) { + this.userDefinedJvmOptions = Objects.requireNonNull(userDefinedJvmOptions); + this.fallbackSystemMemoryInfo = Objects.requireNonNull(fallbackSystemMemoryInfo); + } + + @Override + public long availableSystemMemory() throws SystemMemoryInfoException { + + return userDefinedJvmOptions.stream() + .filter(option -> option.startsWith("-Des.total_memory_bytes=")) + .map(totalMemoryBytesOption -> { + try { + long bytes = Long.parseLong(totalMemoryBytesOption.split("=", 2)[1]); + if (bytes < 0) { + throw new IllegalArgumentException("Negative memory size specified in [" + totalMemoryBytesOption + "]"); + } + return bytes; + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Unable to parse number of bytes from [" + totalMemoryBytesOption + "]", e); + } + }) + .reduce((previous, current) -> current) // this is effectively findLast(), so that ES_JAVA_OPTS overrides jvm.options + .orElse(fallbackSystemMemoryInfo.availableSystemMemory()); + } +} 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 a5fa42d5b0378..2649c1e5aa11a 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 @@ -22,15 +22,10 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; public class MachineDependentHeapTests extends LaunchersTestCase { - private static final long BYTES_IN_GB = 1024L * 1024L * 1024L; - public void testDefaultHeapSize() throws Exception { MachineDependentHeap heap = new MachineDependentHeap(systemMemoryInGigabytes(8)); List options = heap.determineHeapSettings(configPath(), Collections.emptyList()); @@ -47,68 +42,38 @@ public void testUserPassedHeapArgs() throws Exception { } public void testMasterOnlyOptions() { - List options = calculateHeap(16, null, "master"); - assertThat(options, containsInAnyOrder("-Xmx9830m", "-Xms9830m")); - - calculateHeap(20, 16 * BYTES_IN_GB, "master"); + List options = calculateHeap(16, "master"); assertThat(options, containsInAnyOrder("-Xmx9830m", "-Xms9830m")); - options = calculateHeap(64, null, "master"); - assertThat(options, containsInAnyOrder("-Xmx31744m", "-Xms31744m")); - - options = calculateHeap(77, 64 * BYTES_IN_GB, "master"); + options = calculateHeap(64, "master"); assertThat(options, containsInAnyOrder("-Xmx31744m", "-Xms31744m")); } public void testMlOnlyOptions() { - List options = calculateHeap(1, null, "ml"); - assertThat(options, containsInAnyOrder("-Xmx409m", "-Xms409m")); - - options = calculateHeap(4, BYTES_IN_GB, "ml"); + List options = calculateHeap(1, "ml"); assertThat(options, containsInAnyOrder("-Xmx409m", "-Xms409m")); - options = calculateHeap(4, null, "ml"); + options = calculateHeap(4, "ml"); assertThat(options, containsInAnyOrder("-Xmx1024m", "-Xms1024m")); - options = calculateHeap(6, 4 * BYTES_IN_GB, "ml"); - assertThat(options, containsInAnyOrder("-Xmx1024m", "-Xms1024m")); - - options = calculateHeap(32, null, "ml"); - assertThat(options, containsInAnyOrder("-Xmx2048m", "-Xms2048m")); - - options = calculateHeap(100, 32 * BYTES_IN_GB, "ml"); + options = calculateHeap(32, "ml"); assertThat(options, containsInAnyOrder("-Xmx2048m", "-Xms2048m")); } public void testDataNodeOptions() { - List options = calculateHeap(1, null, "data"); - assertThat(options, containsInAnyOrder("-Xmx512m", "-Xms512m")); - - options = calculateHeap(5, BYTES_IN_GB, "data"); + List options = calculateHeap(1, "data"); assertThat(options, containsInAnyOrder("-Xmx512m", "-Xms512m")); - options = calculateHeap(8, null, "data"); + options = calculateHeap(8, "data"); assertThat(options, containsInAnyOrder("-Xmx4096m", "-Xms4096m")); - options = calculateHeap(42, 8 * BYTES_IN_GB, "data"); - assertThat(options, containsInAnyOrder("-Xmx4096m", "-Xms4096m")); - - options = calculateHeap(64, null, "data"); - assertThat(options, containsInAnyOrder("-Xmx31744m", "-Xms31744m")); - - options = calculateHeap(65, 64 * BYTES_IN_GB, "data"); + options = calculateHeap(64, "data"); assertThat(options, containsInAnyOrder("-Xmx31744m", "-Xms31744m")); - options = calculateHeap(0.5, null, "data"); - assertThat(options, containsInAnyOrder("-Xmx204m", "-Xms204m")); - - options = calculateHeap(3, BYTES_IN_GB / 2, "data"); + options = calculateHeap(0.5, "data"); assertThat(options, containsInAnyOrder("-Xmx204m", "-Xms204m")); - options = calculateHeap(0.2, null, "data"); - assertThat(options, containsInAnyOrder("-Xmx128m", "-Xms128m")); - - options = calculateHeap(1, BYTES_IN_GB / 5, "data"); + options = calculateHeap(0.2, "data"); assertThat(options, containsInAnyOrder("-Xmx128m", "-Xms128m")); } @@ -118,30 +83,15 @@ public void testFallbackOptions() throws Exception { assertThat(options, containsInAnyOrder("-Xmx1024m", "-Xms1024m")); } - public void testParseForcedMemoryInBytes() { - assertThat(MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Da=b", "-Dx=y")), nullValue()); - assertThat( - MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Da=b", "-Des.total_memory_bytes=123456789", "-Dx=y")), - is(123456789L) - ); - assertThat(MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Des.total_memory_bytes=987654321")), is(987654321L)); - try { - MachineDependentHeap.parseForcedMemoryInBytes(List.of("-Des.total_memory_bytes=invalid")); - fail("expected parse to fail"); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), is("Unable to parse number of bytes from [-Des.total_memory_bytes=invalid]")); - } - } - - private static List calculateHeap(double memoryInGigabytes, Long forcedMemoryInBytes, String... roles) { + 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, forcedMemoryInBytes); + return calculateHeap(machineDependentHeap, configYaml); } - private static List calculateHeap(MachineDependentHeap machineDependentHeap, String configYaml, Long forcedMemoryInBytes) { + private static List calculateHeap(MachineDependentHeap machineDependentHeap, String configYaml) { try (InputStream in = new ByteArrayInputStream(configYaml.getBytes(StandardCharsets.UTF_8))) { - return machineDependentHeap.determineHeapSettings(in, forcedMemoryInBytes); + return machineDependentHeap.determineHeapSettings(in); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfoTests.java b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfoTests.java new file mode 100644 index 0000000000000..f56db17422578 --- /dev/null +++ b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfoTests.java @@ -0,0 +1,86 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.tools.launchers; + +import org.elasticsearch.tools.launchers.SystemMemoryInfo.SystemMemoryInfoException; + +import java.util.List; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +public class OverridableSystemMemoryInfoTests extends LaunchersTestCase { + + private static final long FALLBACK = -1L; + + public void testNoOptions() throws SystemMemoryInfoException { + final SystemMemoryInfo memoryInfo = new OverridableSystemMemoryInfo(List.of(), fallbackSystemMemoryInfo()); + assertThat(memoryInfo.availableSystemMemory(), is(FALLBACK)); + } + + public void testNoOverrides() throws SystemMemoryInfoException { + final SystemMemoryInfo memoryInfo = new OverridableSystemMemoryInfo(List.of("-Da=b", "-Dx=y"), fallbackSystemMemoryInfo()); + assertThat(memoryInfo.availableSystemMemory(), is(FALLBACK)); + } + + public void testValidSingleOverride() throws SystemMemoryInfoException { + final SystemMemoryInfo memoryInfo = new OverridableSystemMemoryInfo( + List.of("-Des.total_memory_bytes=123456789"), + fallbackSystemMemoryInfo() + ); + assertThat(memoryInfo.availableSystemMemory(), is(123456789L)); + } + + public void testValidOverrideInList() throws SystemMemoryInfoException { + final SystemMemoryInfo memoryInfo = new OverridableSystemMemoryInfo( + List.of("-Da=b", "-Des.total_memory_bytes=987654321", "-Dx=y"), + fallbackSystemMemoryInfo() + ); + assertThat(memoryInfo.availableSystemMemory(), is(987654321L)); + } + + public void testMultipleValidOverridesInList() throws SystemMemoryInfoException { + final SystemMemoryInfo memoryInfo = new OverridableSystemMemoryInfo( + List.of("-Des.total_memory_bytes=123456789", "-Da=b", "-Des.total_memory_bytes=987654321", "-Dx=y"), + fallbackSystemMemoryInfo() + ); + assertThat(memoryInfo.availableSystemMemory(), is(987654321L)); + } + + public void testNegativeOverride() throws SystemMemoryInfoException { + final SystemMemoryInfo memoryInfo = new OverridableSystemMemoryInfo( + List.of("-Da=b", "-Des.total_memory_bytes=-123", "-Dx=y"), + fallbackSystemMemoryInfo() + ); + try { + memoryInfo.availableSystemMemory(); + fail("expected to fail"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), is("Negative memory size specified in [-Des.total_memory_bytes=-123]")); + } + } + + public void testUnparsableOverride() throws SystemMemoryInfoException { + final SystemMemoryInfo memoryInfo = new OverridableSystemMemoryInfo( + List.of("-Da=b", "-Des.total_memory_bytes=invalid", "-Dx=y"), + fallbackSystemMemoryInfo() + ); + try { + memoryInfo.availableSystemMemory(); + fail("expected to fail"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), is("Unable to parse number of bytes from [-Des.total_memory_bytes=invalid]")); + } + } + + private static SystemMemoryInfo fallbackSystemMemoryInfo() { + return () -> FALLBACK; + } +} From 2a744744e254bcd2b3efc7fa32826407074814b3 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Thu, 7 Oct 2021 12:21:19 +0100 Subject: [PATCH 12/23] Add missing word --- .../tools/launchers/OverridableSystemMemoryInfo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfo.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfo.java index 1226831bc2f51..118c68b2111b6 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfo.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/OverridableSystemMemoryInfo.java @@ -12,7 +12,7 @@ import java.util.Objects; /** - * A {@link SystemMemoryInfo} which returns a user-overridden memory size if + * A {@link SystemMemoryInfo} which returns a user-overridden memory size if one * has been specified using the {@code es.total_memory_bytes} system property, or * else returns the value provided by a fallback provider. */ From a4bc21a821ec4b38035717854f758f223bf2db9a Mon Sep 17 00:00:00 2001 From: David Roberts Date: Thu, 7 Oct 2021 13:30:00 +0100 Subject: [PATCH 13/23] Adjust comment --- .../test/java/org/elasticsearch/monitor/os/OsProbeTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java b/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java index 852400b402b97..4e719f4a2da0f 100644 --- a/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java +++ b/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java @@ -314,7 +314,8 @@ public void testTotalMemoryOverride() { assertThat(OsProbe.getTotalMemoryOverride("123456789123456789"), is(123456789123456789L)); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> OsProbe.getTotalMemoryOverride("abc")); assertThat(e.getMessage(), is("Invalid value for [es.total_memory_bytes]: [abc]")); - // Although numeric, this value overflows long. This won't be a problem if the override is set appropriately on a 64 bit machine. + // Although numeric, this value overflows long. This won't be a problem in practice for sensible + // overrides, as it will be a very long time before machines have more than 8 exabytes of RAM. e = expectThrows(IllegalArgumentException.class, () -> OsProbe.getTotalMemoryOverride("123456789123456789123456789")); assertThat(e.getMessage(), is("Invalid value for [es.total_memory_bytes]: [123456789123456789123456789]")); } From 795f1553cd452164dbd9abc3777b696044bdfe50 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Thu, 7 Oct 2021 15:42:52 +0100 Subject: [PATCH 14/23] Adding a packaging test --- .../packaging/test/PackageTests.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) 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 998e3bf6d53c5..0b09e1eac0420 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 @@ -298,6 +298,30 @@ public void test81CustomPathConfAndJvmOptions() throws Exception { cleanup(); } + public void test82JvmOptionsTotalMemoryOverride() throws Exception { + assumeTrue(isSystemd()); + + assertPathsExist(installation.envFile); + stopElasticsearch(); + + withCustomConfig(tempConf -> { + // Work as though total system memory is 850MB + append(installation.envFile, "ES_JAVA_OPTS=\"-Des.total_memory_bytes=891289600\""); + + startElasticsearch(); + + final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes/stats")); + assertThat(nodesResponse, containsString("\"total_override_in_bytes\":891289600")); + + // 40% of 850MB + assertThat(sh.run("ps auwwx").stdout, containsString("-Xmx340m -Xms340m")); + + stopElasticsearch(); + }); + + cleanup(); + } + public void test83SystemdMask() throws Exception { try { assumeTrue(isSystemd()); From 42a000a4ba15d712223731bc493ecb8b0ac8c3ac Mon Sep 17 00:00:00 2001 From: David Roberts Date: Thu, 7 Oct 2021 16:43:06 +0100 Subject: [PATCH 15/23] Adding an archive test and fixing package test --- .../packaging/test/ArchiveTests.java | 21 +++++++++++++++++++ .../packaging/test/PackageTests.java | 6 ++++-- 2 files changed, 25 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 b098b5f7403d4..f49c5c91f03e3 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 @@ -346,6 +346,27 @@ public void test73CustomJvmOptionsDirectoryFilesWithoutOptionsExtensionIgnored() } } + public void test74CustomJvmOptionsTotalMemoryOverride() throws Exception { + final Path heapOptions = installation.config(Paths.get("jvm.options.d", "total_memory.options")); + try { + setHeap(null); // delete default options + // Work as though total system memory is 850MB + append(heapOptions, "-Des.total_memory_bytes=891289600\n"); + + startElasticsearch(); + + final String nodesStatsResponse = makeRequest(Request.Get("http://localhost:9200/_nodes/stats")); + assertThat(nodesStatsResponse, containsString("\"total_override_in_bytes\":891289600")); + final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes")); + // 40% of 850MB + assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":356515840")); + + stopElasticsearch(); + } finally { + rm(heapOptions); + } + } + public void test80RelativePathConf() throws Exception { withCustomConfig(tempConf -> { 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 0b09e1eac0420..ed746fe8480eb 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 @@ -301,6 +301,8 @@ public void test81CustomPathConfAndJvmOptions() throws Exception { public void test82JvmOptionsTotalMemoryOverride() throws Exception { assumeTrue(isSystemd()); + install(); + assertPathsExist(installation.envFile); stopElasticsearch(); @@ -310,8 +312,8 @@ public void test82JvmOptionsTotalMemoryOverride() throws Exception { startElasticsearch(); - final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes/stats")); - assertThat(nodesResponse, containsString("\"total_override_in_bytes\":891289600")); + final String nodesStatsResponse = makeRequest(Request.Get("http://localhost:9200/_nodes/stats")); + assertThat(nodesStatsResponse, containsString("\"total_override_in_bytes\":891289600")); // 40% of 850MB assertThat(sh.run("ps auwwx").stdout, containsString("-Xmx340m -Xms340m")); From 3c1eabdb7dab4a104ca897c1b29d99213d426d40 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Mon, 11 Oct 2021 11:31:48 +0100 Subject: [PATCH 16/23] Move packaging test out of systemd section --- .../packaging/test/PackageTests.java | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 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 ed746fe8480eb..2f1794a6cac1a 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 @@ -233,6 +233,31 @@ public void test70RestartServer() throws Exception { } } + public void test71JvmOptionsTotalMemoryOverride() throws Exception { + try { + install(); + assertPathsExist(installation.envFile); + setHeap(null); + + withCustomConfig(tempConf -> { + // Work as though total system memory is 850MB + append(installation.envFile, "ES_JAVA_OPTS=\"-Des.total_memory_bytes=891289600\""); + + startElasticsearch(); + + final String nodesStatsResponse = makeRequest(Request.Get("http://localhost:9200/_nodes/stats")); + assertThat(nodesStatsResponse, containsString("\"total_override_in_bytes\":891289600")); + + // 40% of 850MB + assertThat(sh.run("ps auwwx").stdout, containsString("-Xmx340m -Xms340m")); + + stopElasticsearch(); + }); + } finally { + cleanup(); + } + } + public void test72TestRuntimeDirectory() throws Exception { try { install(); @@ -298,32 +323,6 @@ public void test81CustomPathConfAndJvmOptions() throws Exception { cleanup(); } - public void test82JvmOptionsTotalMemoryOverride() throws Exception { - assumeTrue(isSystemd()); - - install(); - - assertPathsExist(installation.envFile); - stopElasticsearch(); - - withCustomConfig(tempConf -> { - // Work as though total system memory is 850MB - append(installation.envFile, "ES_JAVA_OPTS=\"-Des.total_memory_bytes=891289600\""); - - startElasticsearch(); - - final String nodesStatsResponse = makeRequest(Request.Get("http://localhost:9200/_nodes/stats")); - assertThat(nodesStatsResponse, containsString("\"total_override_in_bytes\":891289600")); - - // 40% of 850MB - assertThat(sh.run("ps auwwx").stdout, containsString("-Xmx340m -Xms340m")); - - stopElasticsearch(); - }); - - cleanup(); - } - public void test83SystemdMask() throws Exception { try { assumeTrue(isSystemd()); From b4c646cbb43d15f1bb315a983f12261ea1582a3d Mon Sep 17 00:00:00 2001 From: David Roberts Date: Mon, 11 Oct 2021 14:23:06 +0100 Subject: [PATCH 17/23] Fix test --- .../java/org/elasticsearch/packaging/test/PackageTests.java | 2 +- 1 file changed, 1 insertion(+), 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 2f1794a6cac1a..e4d2c61ed302f 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 @@ -249,7 +249,7 @@ public void test71JvmOptionsTotalMemoryOverride() throws Exception { assertThat(nodesStatsResponse, containsString("\"total_override_in_bytes\":891289600")); // 40% of 850MB - assertThat(sh.run("ps auwwx").stdout, containsString("-Xmx340m -Xms340m")); + assertThat(sh.run("ps auwwx").stdout, containsString("-Xms340m -Xmx340m")); stopElasticsearch(); }); From 449cf2a9ef2f1eccd9f82e8413cc1f67fd89c9bd Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 13 Oct 2021 17:11:10 +0100 Subject: [PATCH 18/23] Using always present adjusted_total instead of optional total_override --- docs/reference/cluster/nodes-stats.asciidoc | 11 ++--- docs/reference/cluster/stats.asciidoc | 16 +++---- .../cluster/stats/ClusterStatsNodes.java | 17 +++---- .../org/elasticsearch/monitor/os/OsProbe.java | 15 ++++--- .../org/elasticsearch/monitor/os/OsStats.java | 33 +++++++------- .../cluster/node/stats/NodeStatsTests.java | 2 +- .../monitor/os/OsStatsTests.java | 4 +- .../AutoscalingMemoryInfoServiceTests.java | 2 +- .../xpack/ml/MachineLearning.java | 36 +-------------- .../xpack/ml/MachineLearningTests.java | 44 ------------------- .../ClusterStatsMonitoringDocTests.java | 2 +- .../node/NodeStatsMonitoringDocTests.java | 2 +- 12 files changed, 49 insertions(+), 135 deletions(-) diff --git a/docs/reference/cluster/nodes-stats.asciidoc b/docs/reference/cluster/nodes-stats.asciidoc index 82aff37dd548b..7e15acb99d4bd 100644 --- a/docs/reference/cluster/nodes-stats.asciidoc +++ b/docs/reference/cluster/nodes-stats.asciidoc @@ -1036,16 +1036,17 @@ Total amount of physical memory. (integer) Total amount of physical memory in bytes. -`total_override`:: +`adjusted_total`:: (<>) If the amount of physical memory has been overridden using the `es.total_memory_bytes` -system property then this reports the overridden value. Otherwise it is not present. +system property then this reports the overridden value. Otherwise it reports the same +value as `total`. -`total_override_in_bytes`:: +`adjusted_total_in_bytes`:: (integer) If the amount of physical memory has been overridden using the `es.total_memory_bytes` -system property then this reports the overridden value in bytes. Otherwise it is not -present. +system property then this reports the overridden value in bytes. Otherwise it reports +the same value as `total_in_bytes`. `free`:: (<>) diff --git a/docs/reference/cluster/stats.asciidoc b/docs/reference/cluster/stats.asciidoc index 67d605dddd1eb..e4d3936babb8c 100644 --- a/docs/reference/cluster/stats.asciidoc +++ b/docs/reference/cluster/stats.asciidoc @@ -916,17 +916,17 @@ Total amount of physical memory across all selected nodes. (integer) Total amount, in bytes, of physical memory across all selected nodes. -`total_override`:: +`adjusted_total`:: (<>) -If the amount of physical memory has been overridden using the `es.total_memory_bytes` -system property on all selected nodes then this reports the sum of the overridden -values. Otherwise it is not present. +Total amount of memory across all selected nodes, but using the value specified +using the `es.total_memory_bytes` system property instead of measured total +memory for those nodes where that system property was set. -`total_override_in_bytes`:: +`adjusted_total_in_bytes`:: (integer) -If the amount of physical memory has been overridden using the `es.total_memory_bytes` -system property on all selected nodes then this reports the sum of the overridden -values in bytes. Otherwise it is not present. +Total amount, in bytes, of memory across all selected nodes, but using the +value specified using the `es.total_memory_bytes` system property instead +of measured total memory for those nodes where that system property was set. `free`:: (<>) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java index a2042f797b39e..b3fc80d74bc3e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java @@ -266,7 +266,7 @@ private OsStats(List nodeInfos, List nodeStatsList) { this.allocatedProcessors = allocatedProcessors; long totalMemory = 0; - Long totalMemoryOverride = 0L; + long adjustedTotalMemory = 0; long freeMemory = 0; for (NodeStats nodeStats : nodeStatsList) { if (nodeStats.getOs() != null) { @@ -275,16 +275,9 @@ private OsStats(List nodeInfos, List nodeStatsList) { if (total > 0) { totalMemory += total; } - // Only report a total memory override for the whole cluster if every node has overridden total memory - if (totalMemoryOverride != null) { - if (mem.getTotalOverride() != null) { - long totalOverride = mem.getTotalOverride().getBytes(); - if (totalOverride > 0) { - totalMemoryOverride += totalOverride; - } - } else { - totalMemoryOverride = null; - } + long adjustedTotal = mem.getAdjustedTotal().getBytes(); + if (total > 0) { + adjustedTotalMemory += adjustedTotal; } long free = nodeStats.getOs().getMem().getFree().getBytes(); if (free > 0) { @@ -292,7 +285,7 @@ private OsStats(List nodeInfos, List nodeStatsList) { } } } - this.mem = new org.elasticsearch.monitor.os.OsStats.Mem(totalMemory, totalMemoryOverride, freeMemory); + this.mem = new org.elasticsearch.monitor.os.OsStats.Mem(totalMemory, adjustedTotalMemory, freeMemory); } public int getAvailableProcessors() { diff --git a/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java b/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java index 1aadc0d800dbc..ea90894be37a3 100644 --- a/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java +++ b/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java @@ -128,14 +128,15 @@ public long getTotalPhysicalMemorySize() { } /** - * Returns the overridden total amount of physical memory in bytes. + * Returns the adjusted total amount of physical memory in bytes. * Total memory may be overridden when some other process is running - * that is known to consume a non-negligible amount of memory. This - * is read from the "es.total_memory_bytes" system property. Negative - * values or not set at all mean no override. + * that is known to consume a non-negligible amount of memory. This is + * read from the "es.total_memory_bytes" system property. Negative values + * or not set at all mean no override. When there is no override this + * method returns the same value as {@link #getTotalPhysicalMemorySize}. */ - public Long getTotalMemoryOverride() { - return getTotalMemoryOverride(memoryOverrideProperty); + public long getAdjustedTotalMemorySize() { + return Optional.ofNullable(getTotalMemoryOverride(memoryOverrideProperty)).orElse(getTotalPhysicalMemorySize()); } static Long getTotalMemoryOverride(String memoryOverrideProperty) { @@ -883,7 +884,7 @@ OsStats.Cgroup getCgroup(boolean isLinux) { public OsStats osStats() { final OsStats.Cpu cpu = new OsStats.Cpu(getSystemCpuPercent(), getSystemLoadAverage()); - final OsStats.Mem mem = new OsStats.Mem(getTotalPhysicalMemorySize(), getTotalMemoryOverride(), getFreePhysicalMemorySize()); + final OsStats.Mem mem = new OsStats.Mem(getTotalPhysicalMemorySize(), getAdjustedTotalMemorySize(), getFreePhysicalMemorySize()); final OsStats.Swap swap = new OsStats.Swap(getTotalSwapSpaceSize(), getFreeSwapSpaceSize()); final OsStats.Cgroup cgroup = getCgroup(Constants.LINUX); return new OsStats(System.currentTimeMillis(), cpu, mem, swap, cgroup); diff --git a/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java b/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java index d42bc3804c709..76aaf55f33aaa 100644 --- a/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java +++ b/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java @@ -91,8 +91,8 @@ static final class Fields { static final String USED_IN_BYTES = "used_in_bytes"; static final String TOTAL = "total"; static final String TOTAL_IN_BYTES = "total_in_bytes"; - static final String TOTAL_OVERRIDE = "total_override"; - static final String TOTAL_OVERRIDE_IN_BYTES = "total_override_in_bytes"; + static final String ADJUSTED_TOTAL = "adjusted_total"; + static final String ADJUSTED_TOTAL_IN_BYTES = "adjusted_total_in_bytes"; static final String FREE_PERCENT = "free_percent"; static final String USED_PERCENT = "used_percent"; @@ -240,29 +240,28 @@ public static class Mem implements Writeable, ToXContentFragment { private static final Logger logger = LogManager.getLogger(Mem.class); private final long total; - private final Long totalOverride; + private final long adjustedTotal; private final long free; - public Mem(long total, Long totalOverride, long free) { + public Mem(long total, long adjustedTotal, long free) { assert total >= 0 : "expected total memory to be positive, got: " + total; - assert totalOverride == null || totalOverride >= 0 : "expected total overridden memory to be positive, got: " + totalOverride; + assert adjustedTotal >= 0 : "expected adjusted total memory to be positive, got: " + adjustedTotal; assert free >= 0 : "expected free memory to be positive, got: " + free; this.total = total; - this.totalOverride = totalOverride; + this.adjustedTotal = adjustedTotal; this.free = free; } public Mem(StreamInput in) throws IOException { - this.total = in.readLong(); + total = in.readLong(); assert total >= 0 : "expected total memory to be positive, got: " + total; if (in.getVersion().onOrAfter(Version.V_8_0_0)) { - this.totalOverride = in.readOptionalLong(); - assert totalOverride == null || totalOverride >= 0 - : "expected total overridden memory to be positive, got: " + totalOverride; + adjustedTotal = in.readLong(); + assert adjustedTotal >= 0 : "expected adjusted total memory to be positive, got: " + adjustedTotal; } else { - this.totalOverride = null; + adjustedTotal = total; } - this.free = in.readLong(); + free = in.readLong(); assert free >= 0 : "expected free memory to be positive, got: " + free; } @@ -270,7 +269,7 @@ public Mem(StreamInput in) throws IOException { public void writeTo(StreamOutput out) throws IOException { out.writeLong(total); if (out.getVersion().onOrAfter(Version.V_8_0_0)) { - out.writeOptionalLong(totalOverride); + out.writeLong(adjustedTotal); } out.writeLong(free); } @@ -279,8 +278,8 @@ public ByteSizeValue getTotal() { return new ByteSizeValue(total); } - public ByteSizeValue getTotalOverride() { - return totalOverride == null ? null : new ByteSizeValue(totalOverride); + public ByteSizeValue getAdjustedTotal() { + return new ByteSizeValue(adjustedTotal); } public ByteSizeValue getUsed() { @@ -315,9 +314,7 @@ public short getFreePercent() { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(Fields.MEM); builder.humanReadableField(Fields.TOTAL_IN_BYTES, Fields.TOTAL, getTotal()); - if (getTotalOverride() != null) { - builder.humanReadableField(Fields.TOTAL_OVERRIDE_IN_BYTES, Fields.TOTAL_OVERRIDE, getTotalOverride()); - } + builder.humanReadableField(Fields.ADJUSTED_TOTAL_IN_BYTES, Fields.ADJUSTED_TOTAL, getAdjustedTotal()); builder.humanReadableField(Fields.FREE_IN_BYTES, Fields.FREE, getFree()); builder.humanReadableField(Fields.USED_IN_BYTES, Fields.USED, getUsed()); builder.field(Fields.FREE_PERCENT, getFreePercent()); diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java index 87ef0e3f82bf3..31167225bf89a 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -433,7 +433,7 @@ public static NodeStats createNodeStats() { long memTotal = randomNonNegativeLong(); long swapTotal = randomNonNegativeLong(); osStats = new OsStats(System.currentTimeMillis(), new OsStats.Cpu(randomShort(), loadAverages), - new OsStats.Mem(memTotal, randomFrom(randomLongBetween(1, memTotal), null), randomLongBetween(0, memTotal)), + new OsStats.Mem(memTotal, randomLongBetween(0, memTotal), randomLongBetween(0, memTotal)), new OsStats.Swap(swapTotal, randomLongBetween(0, swapTotal)), new OsStats.Cgroup( randomAlphaOfLength(8), diff --git a/server/src/test/java/org/elasticsearch/monitor/os/OsStatsTests.java b/server/src/test/java/org/elasticsearch/monitor/os/OsStatsTests.java index 9e1ee31c9fb27..a2ee845d4e69c 100644 --- a/server/src/test/java/org/elasticsearch/monitor/os/OsStatsTests.java +++ b/server/src/test/java/org/elasticsearch/monitor/os/OsStatsTests.java @@ -26,7 +26,7 @@ public void testSerialization() throws IOException { } OsStats.Cpu cpu = new OsStats.Cpu(randomShort(), loadAverages); long memTotal = randomNonNegativeLong(); - OsStats.Mem mem = new OsStats.Mem(memTotal, randomFrom(randomLongBetween(1, memTotal), null), randomLongBetween(0, memTotal)); + OsStats.Mem mem = new OsStats.Mem(memTotal, randomLongBetween(0, memTotal), randomLongBetween(0, memTotal)); long swapTotal = randomNonNegativeLong(); OsStats.Swap swap = new OsStats.Swap(swapTotal, randomLongBetween(0, swapTotal)); OsStats.Cgroup cgroup = new OsStats.Cgroup( @@ -73,7 +73,7 @@ public void testSerialization() throws IOException { } public void testGetUsedMemoryWithZeroTotal() { - OsStats.Mem mem = new OsStats.Mem(0, null, randomNonNegativeLong()); + OsStats.Mem mem = new OsStats.Mem(0, 0, randomNonNegativeLong()); assertThat(mem.getUsed().getBytes(), equalTo(0L)); } diff --git a/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java b/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java index 55295701a16d7..fb0a073c02e86 100644 --- a/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java +++ b/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java @@ -347,7 +347,7 @@ private static NodeStats statsForNode(DiscoveryNode node, long memory) { OsStats osStats = new OsStats( randomNonNegativeLong(), new OsStats.Cpu(randomShort(), null), - new OsStats.Mem(memory, null, randomLongBetween(0, memory)), + new OsStats.Mem(memory, memory, randomLongBetween(0, memory)), new OsStats.Swap(randomNonNegativeLong(), randomNonNegativeLong()), null ); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java index 7e0fa797101e2..e1ccddd681c24 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java @@ -59,7 +59,6 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.monitor.jvm.JvmInfo; import org.elasticsearch.monitor.os.OsProbe; -import org.elasticsearch.monitor.os.OsStats; import org.elasticsearch.persistent.PersistentTaskParams; import org.elasticsearch.persistent.PersistentTaskState; import org.elasticsearch.persistent.PersistentTasksExecutor; @@ -415,7 +414,6 @@ import org.elasticsearch.xpack.ml.utils.persistence.ResultsPersisterService; import java.io.IOException; -import java.math.BigInteger; import java.nio.file.Path; import java.time.Clock; import java.util.ArrayList; @@ -674,7 +672,7 @@ public Settings additionalSettings() { addMlNodeAttribute(additionalSettings, maxOpenJobsPerNodeNodeAttrName, String.valueOf(MAX_OPEN_JOBS_PER_NODE.get(settings))); addMlNodeAttribute(additionalSettings, machineMemoryAttrName, - Long.toString(machineMemoryFromStats(OsProbe.getInstance().osStats()))); + Long.toString(OsProbe.getInstance().osStats().getMem().getAdjustedTotal().getBytes())); addMlNodeAttribute(additionalSettings, jvmSizeAttrName, Long.toString(Runtime.getRuntime().maxMemory())); // This is not used in v7 and higher, but users are still prevented from setting it directly to avoid confusion disallowMlNodeAttributes(mlEnabledNodeAttrName); @@ -1284,38 +1282,6 @@ public static boolean allTemplatesInstalled(ClusterState clusterState) { return allPresent; } - /** - * Find the memory size (in bytes) of the machine this node is running on. - * Takes container limits (as used by Docker for example) and forced memory - * size overrides into account. - */ - static long machineMemoryFromStats(OsStats stats) { - long mem = stats.getMem().getTotal().getBytes(); - OsStats.Cgroup cgroup = stats.getCgroup(); - if (cgroup != null) { - String containerLimitStr = cgroup.getMemoryLimitInBytes(); - if (containerLimitStr != null && containerLimitStr.equals("max") == false) { - BigInteger containerLimit = new BigInteger(containerLimitStr); - if ((containerLimit.compareTo(BigInteger.valueOf(mem)) < 0 && containerLimit.compareTo(BigInteger.ZERO) > 0) - // mem <= 0 means the value couldn't be obtained for some reason - || (mem <= 0 && containerLimit.compareTo(BigInteger.valueOf(Long.MAX_VALUE)) < 0)) { - mem = containerLimit.longValue(); - } - } - } - ByteSizeValue memOverride = stats.getMem().getTotalOverride(); - if (memOverride != null && memOverride.getBytes() != mem) { - long diff = memOverride.getBytes() - mem; - // Only log if the difference is more than 1% - if (Math.abs((double) diff) / (double) Math.max(memOverride.getBytes(), mem) > 0.01) { - logger.info("ML memory calculations will be based on total memory override [{}] rather than measured total memory [{}]", - memOverride, ByteSizeValue.ofBytes(mem)); - } - mem = memOverride.getBytes(); - } - return mem; - } - @Override public List getNamedXContent() { List namedXContent = new ArrayList<>(); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java index 48bec94d8f5ec..8dbce77bf47c6 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java @@ -8,13 +8,11 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.license.XPackLicenseState; -import org.elasticsearch.monitor.os.OsStats; import org.elasticsearch.test.ESTestCase; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.startsWith; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class MachineLearningTests extends ESTestCase { @@ -94,48 +92,6 @@ public void testNoAttributes_givenClash() { "it is reserved for machine learning. If your intention was to customize machine learning, set the [xpack.ml.")); } - public void testMachineMemory_givenStatsFailure() { - OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(0, null, 0)); - assertEquals(0L, MachineLearning.machineMemoryFromStats(stats)); - } - - public void testMachineMemory_givenOverride() { - OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, 1_234_567_890L, 5_368_709_120L)); - assertEquals(1_234_567_890L, MachineLearning.machineMemoryFromStats(stats)); - } - - public void testMachineMemory_givenNoCgroup() { - OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, null, 5_368_709_120L)); - assertEquals(10_737_418_240L, MachineLearning.machineMemoryFromStats(stats)); - } - - public void testMachineMemory_givenCgroupNullLimit() { - OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, null, 5_368_709_120L)); - when(stats.getCgroup()).thenReturn(new OsStats.Cgroup("a", 1, "b", 2, 3, - new OsStats.Cgroup.CpuStat(4, 5, 6), null, null, null)); - assertEquals(10_737_418_240L, MachineLearning.machineMemoryFromStats(stats)); - } - - public void testMachineMemory_givenCgroupNoLimit() { - OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, null, 5_368_709_120L)); - when(stats.getCgroup()).thenReturn(new OsStats.Cgroup("a", 1, "b", 2, 3, - new OsStats.Cgroup.CpuStat(4, 5, 6), "c", "18446744073709551615", "4796416")); - assertEquals(10_737_418_240L, MachineLearning.machineMemoryFromStats(stats)); - } - - public void testMachineMemory_givenCgroupLowLimit() { - OsStats stats = mock(OsStats.class); - when(stats.getMem()).thenReturn(new OsStats.Mem(10_737_418_240L, null, 5_368_709_120L)); - when(stats.getCgroup()).thenReturn(new OsStats.Cgroup("a", 1, "b", 2, 3, - new OsStats.Cgroup.CpuStat(4, 5, 6), "c", "7516192768", "4796416")); - assertEquals(7_516_192_768L, MachineLearning.machineMemoryFromStats(stats)); - } - private MachineLearning createMachineLearning(Settings settings) { XPackLicenseState licenseState = mock(XPackLicenseState.class); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java index 8fcb42bbc2057..2f78b79a44341 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java @@ -297,7 +297,7 @@ public void testToXContent() throws IOException { final OsStats mockOsStats = mock(OsStats.class); when(mockNodeStats.getOs()).thenReturn(mockOsStats); - when(mockOsStats.getMem()).thenReturn(new OsStats.Mem(100, 99L, 79)); + when(mockOsStats.getMem()).thenReturn(new OsStats.Mem(100, 99, 79)); final ProcessStats mockProcessStats = mock(ProcessStats.class); when(mockNodeStats.getProcess()).thenReturn(mockProcessStats); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java index 6fa9b1710dcda..ad4d99347c069 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java @@ -341,7 +341,7 @@ private static NodeStats mockNodeStats() { final OsStats.Cgroup osCgroup = new OsStats.Cgroup("_cpu_acct_ctrl_group", ++iota, "_cpu_ctrl_group", ++iota, ++iota, osCpuStat, "_memory_ctrl_group", "2000000000", "1000000000"); - final OsStats.Mem osMem = new OsStats.Mem(0, null, 0); + final OsStats.Mem osMem = new OsStats.Mem(0, 0, 0); final OsStats.Swap osSwap = new OsStats.Swap(0, 0); final OsStats os = new OsStats(no, osCpu, osMem, osSwap, osCgroup); From ee59b9f931aa185c633a2ee8cb55513516a283c8 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 13 Oct 2021 18:56:33 +0100 Subject: [PATCH 19/23] Fixing tests --- docs/reference/cluster/stats.asciidoc | 2 ++ .../java/org/elasticsearch/packaging/test/ArchiveTests.java | 2 +- .../java/org/elasticsearch/packaging/test/PackageTests.java | 2 +- .../collector/cluster/ClusterStatsMonitoringDocTests.java | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/reference/cluster/stats.asciidoc b/docs/reference/cluster/stats.asciidoc index e4d3936babb8c..209c40a3e4070 100644 --- a/docs/reference/cluster/stats.asciidoc +++ b/docs/reference/cluster/stats.asciidoc @@ -1411,6 +1411,8 @@ The API returns the following response: "mem" : { "total" : "16gb", "total_in_bytes" : 17179869184, + "adjusted_total" : "16gb", + "adjusted_total_in_bytes" : 17179869184, "free" : "78.1mb", "free_in_bytes" : 81960960, "used" : "15.9gb", 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 f49c5c91f03e3..56155c4101e8d 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 @@ -356,7 +356,7 @@ public void test74CustomJvmOptionsTotalMemoryOverride() throws Exception { startElasticsearch(); final String nodesStatsResponse = makeRequest(Request.Get("http://localhost:9200/_nodes/stats")); - assertThat(nodesStatsResponse, containsString("\"total_override_in_bytes\":891289600")); + assertThat(nodesStatsResponse, containsString("\"adjusted_total_in_bytes\":891289600")); final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes")); // 40% of 850MB assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":356515840")); 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 e4d2c61ed302f..a4c539b241ba6 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 @@ -246,7 +246,7 @@ public void test71JvmOptionsTotalMemoryOverride() throws Exception { startElasticsearch(); final String nodesStatsResponse = makeRequest(Request.Get("http://localhost:9200/_nodes/stats")); - assertThat(nodesStatsResponse, containsString("\"total_override_in_bytes\":891289600")); + assertThat(nodesStatsResponse, containsString("\"adjusted_total_in_bytes\":891289600")); // 40% of 850MB assertThat(sh.run("ps auwwx").stdout, containsString("-Xms340m -Xmx340m")); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java index 2f78b79a44341..d166d37bbf043 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java @@ -517,7 +517,7 @@ public void testToXContent() throws IOException { + " ]," + " \"mem\": {" + " \"total_in_bytes\": 100," - + " \"total_override_in_bytes\": 99," + + " \"adjusted_total_in_bytes\": 99," + " \"free_in_bytes\": 79," + " \"used_in_bytes\": 21," + " \"free_percent\": 79," From be14364d073b2b646b3cd16e86c98dd4edd1ee64 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Fri, 15 Oct 2021 10:50:30 +0100 Subject: [PATCH 20/23] Address code review comments --- .../cluster/stats/ClusterStatsNodes.java | 4 +- .../org/elasticsearch/monitor/os/OsProbe.java | 19 +++++++--- .../org/elasticsearch/monitor/os/OsStats.java | 37 +++++++++++++++++-- .../monitor/os/OsProbeTests.java | 6 +-- 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java index b3fc80d74bc3e..31b009a08b37b 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java @@ -276,10 +276,10 @@ private OsStats(List nodeInfos, List nodeStatsList) { totalMemory += total; } long adjustedTotal = mem.getAdjustedTotal().getBytes(); - if (total > 0) { + if (adjustedTotal > 0) { adjustedTotalMemory += adjustedTotal; } - long free = nodeStats.getOs().getMem().getFree().getBytes(); + long free = mem.getFree().getBytes(); if (free > 0) { freeMemory += free; } diff --git a/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java b/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java index ea90894be37a3..4f5e5e419f182 100644 --- a/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java +++ b/server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java @@ -62,7 +62,7 @@ public class OsProbe { // This property is specified without units because it also needs to be parsed by the launcher // code, which does not have access to all the utility classes of the Elasticsearch server. - private static final String memoryOverrideProperty = System.getProperty("es.total_memory_bytes", "-1"); + private static final String memoryOverrideProperty = System.getProperty("es.total_memory_bytes"); private static final Method getFreePhysicalMemorySize; private static final Method getTotalPhysicalMemorySize; @@ -130,19 +130,26 @@ public long getTotalPhysicalMemorySize() { /** * Returns the adjusted total amount of physical memory in bytes. * Total memory may be overridden when some other process is running - * that is known to consume a non-negligible amount of memory. This is - * read from the "es.total_memory_bytes" system property. Negative values - * or not set at all mean no override. When there is no override this - * method returns the same value as {@link #getTotalPhysicalMemorySize}. + * that is known to consume a non-negligible amount of memory. This + * is read from the "es.total_memory_bytes" system property. When + * there is no override this method returns the same value as + * {@link #getTotalPhysicalMemorySize}. */ public long getAdjustedTotalMemorySize() { return Optional.ofNullable(getTotalMemoryOverride(memoryOverrideProperty)).orElse(getTotalPhysicalMemorySize()); } static Long getTotalMemoryOverride(String memoryOverrideProperty) { + if (memoryOverrideProperty == null) { + return null; + } try { long memoryOverride = Long.parseLong(memoryOverrideProperty); - return (memoryOverride < 0) ? null : memoryOverride; + if (memoryOverride < 0) { + throw new IllegalArgumentException("Negative memory size specified in [es.total_memory_bytes]: [" + + memoryOverrideProperty + "]"); + } + return memoryOverride; } catch (NumberFormatException e) { throw new IllegalArgumentException("Invalid value for [es.total_memory_bytes]: [" + memoryOverrideProperty + "]", e); } diff --git a/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java b/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java index 76aaf55f33aaa..d5913c89fd165 100644 --- a/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java +++ b/server/src/main/java/org/elasticsearch/monitor/os/OsStats.java @@ -247,22 +247,51 @@ public Mem(long total, long adjustedTotal, long free) { assert total >= 0 : "expected total memory to be positive, got: " + total; assert adjustedTotal >= 0 : "expected adjusted total memory to be positive, got: " + adjustedTotal; assert free >= 0 : "expected free memory to be positive, got: " + free; + // Extra layer of protection for when assertions are disabled + if (total < 0) { + logger.error("negative total memory [{}] found in memory stats", total); + total = 0; + } + if (adjustedTotal < 0) { + logger.error("negative adjusted total memory [{}] found in memory stats", total); + adjustedTotal = 0; + } + if (free < 0) { + logger.error("negative free memory [{}] found in memory stats", total); + free = 0; + } this.total = total; this.adjustedTotal = adjustedTotal; this.free = free; } public Mem(StreamInput in) throws IOException { - total = in.readLong(); + long total = in.readLong(); assert total >= 0 : "expected total memory to be positive, got: " + total; + // Extra layer of protection for when assertions are disabled + if (total < 0) { + logger.error("negative total memory [{}] deserialized in memory stats", total); + total = 0; + } + this.total = total; if (in.getVersion().onOrAfter(Version.V_8_0_0)) { - adjustedTotal = in.readLong(); + long adjustedTotal = in.readLong(); assert adjustedTotal >= 0 : "expected adjusted total memory to be positive, got: " + adjustedTotal; + if (adjustedTotal < 0) { + logger.error("negative adjusted total memory [{}] deserialized in memory stats", adjustedTotal); + adjustedTotal = 0; + } + this.adjustedTotal = adjustedTotal; } else { - adjustedTotal = total; + this.adjustedTotal = total; } - free = in.readLong(); + long free = in.readLong(); assert free >= 0 : "expected free memory to be positive, got: " + free; + if (free < 0) { + logger.error("negative free memory [{}] deserialized in memory stats", free); + free = 0; + } + this.free = free; } @Override diff --git a/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java b/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java index 4e719f4a2da0f..23091169f94e1 100644 --- a/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java +++ b/server/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java @@ -28,7 +28,6 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; public class OsProbeTests extends ESTestCase { @@ -310,9 +309,10 @@ public void testGetTotalMemFromProcMeminfo() throws Exception { public void testTotalMemoryOverride() { assertThat(OsProbe.getTotalMemoryOverride("123456789"), is(123456789L)); - assertThat(OsProbe.getTotalMemoryOverride("-1"), nullValue()); assertThat(OsProbe.getTotalMemoryOverride("123456789123456789"), is(123456789123456789L)); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> OsProbe.getTotalMemoryOverride("abc")); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> OsProbe.getTotalMemoryOverride("-1")); + assertThat(e.getMessage(), is("Negative memory size specified in [es.total_memory_bytes]: [-1]")); + e = expectThrows(IllegalArgumentException.class, () -> OsProbe.getTotalMemoryOverride("abc")); assertThat(e.getMessage(), is("Invalid value for [es.total_memory_bytes]: [abc]")); // Although numeric, this value overflows long. This won't be a problem in practice for sensible // overrides, as it will be a very long time before machines have more than 8 exabytes of RAM. From 8741c98fe6fd5281f199f9d5285a0be941f83a10 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Fri, 15 Oct 2021 11:18:58 +0100 Subject: [PATCH 21/23] Adapt to packaging test framework changes --- .../java/org/elasticsearch/packaging/test/ArchiveTests.java | 4 ++-- .../java/org/elasticsearch/packaging/test/PackageTests.java | 2 +- 2 files changed, 3 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 764c4710baaee..30ce6690721dd 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 @@ -520,9 +520,9 @@ public void test74CustomJvmOptionsTotalMemoryOverride() throws Exception { startElasticsearch(); - final String nodesStatsResponse = makeRequest(Request.Get("http://localhost:9200/_nodes/stats")); + final String nodesStatsResponse = makeRequest("http://localhost:9200/_nodes/stats"); assertThat(nodesStatsResponse, containsString("\"adjusted_total_in_bytes\":891289600")); - final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes")); + final String nodesResponse = makeRequest("http://localhost:9200/_nodes"); // 40% of 850MB assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":356515840")); 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 6b33d30629276..177dae1c52686 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 @@ -249,7 +249,7 @@ public void test71JvmOptionsTotalMemoryOverride() throws Exception { startElasticsearch(); - final String nodesStatsResponse = makeRequest(Request.Get("http://localhost:9200/_nodes/stats")); + final String nodesStatsResponse = makeRequest("http://localhost:9200/_nodes/stats"); assertThat(nodesStatsResponse, containsString("\"adjusted_total_in_bytes\":891289600")); // 40% of 850MB From 71f7a2a95f16612f829e09dff60bbb035bd529dd Mon Sep 17 00:00:00 2001 From: David Roberts Date: Fri, 15 Oct 2021 12:23:22 +0100 Subject: [PATCH 22/23] Packaging tests need https now --- .../java/org/elasticsearch/packaging/test/ArchiveTests.java | 4 ++-- .../java/org/elasticsearch/packaging/test/PackageTests.java | 2 +- 2 files changed, 3 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 30ce6690721dd..72f4a37ce1853 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 @@ -520,9 +520,9 @@ public void test74CustomJvmOptionsTotalMemoryOverride() throws Exception { startElasticsearch(); - final String nodesStatsResponse = makeRequest("http://localhost:9200/_nodes/stats"); + final String nodesStatsResponse = makeRequest("https://localhost:9200/_nodes/stats"); assertThat(nodesStatsResponse, containsString("\"adjusted_total_in_bytes\":891289600")); - final String nodesResponse = makeRequest("http://localhost:9200/_nodes"); + final String nodesResponse = makeRequest("https://localhost:9200/_nodes"); // 40% of 850MB assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":356515840")); 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 177dae1c52686..bd0fc92a1930a 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 @@ -249,7 +249,7 @@ public void test71JvmOptionsTotalMemoryOverride() throws Exception { startElasticsearch(); - final String nodesStatsResponse = makeRequest("http://localhost:9200/_nodes/stats"); + final String nodesStatsResponse = makeRequest("https://localhost:9200/_nodes/stats"); assertThat(nodesStatsResponse, containsString("\"adjusted_total_in_bytes\":891289600")); // 40% of 850MB From 1cd19bf4ea13f290f18219cf9e7f93ec12a30221 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Fri, 15 Oct 2021 13:35:15 +0100 Subject: [PATCH 23/23] Set up security after calling install() --- .../java/org/elasticsearch/packaging/test/PackageTests.java | 3 +++ 1 file changed, 3 insertions(+) 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 bd0fc92a1930a..4047277f0b1ca 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 @@ -243,6 +243,9 @@ public void test71JvmOptionsTotalMemoryOverride() throws Exception { assertPathsExist(installation.envFile); setHeap(null); + // Recreate file realm users that have been deleted in earlier tests + setFileSuperuser("test_superuser", "test_superuser_password"); + withCustomConfig(tempConf -> { // Work as though total system memory is 850MB append(installation.envFile, "ES_JAVA_OPTS=\"-Des.total_memory_bytes=891289600\"");