Using always present adjusted_total instead of optional total_override

Author: David Roberts

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`::
 (<<byte-units,byte value>>)
 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`::
 (<<byte-units,byte value>>)

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`::
 (<<byte-units,byte value>>)
-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`::
 (<<byte-units, byte units>>)

server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodes.java

@@ -266,7 +266,7 @@ private OsStats(List<NodeInfo> nodeInfos, List<NodeStats> 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,24 +275,17 @@ private OsStats(List<NodeInfo> nodeInfos, List<NodeStats> 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) {
                         freeMemory += free;
                     }
                 }
             }
-            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() {

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);

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,37 +240,36 @@ 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;
         }
 
         @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(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());

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),

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));
     }
 

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
         );

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<NamedXContentRegistry.Entry> getNamedXContent() {
         List<NamedXContentRegistry.Entry> namedXContent = new ArrayList<>();

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);