Merge pull request #16518 from vespa-engine/geirst/reserve-memory-for-content-node-processes-in-hwinfo-memory

Reserve memory for other processes on the content node the proper way…

3 files changed, 35 insertions, 41 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/NodeResourcesTuning.java b/config-model/src/main/java/com/yahoo/vespa/model/search/NodeResourcesTuning.java
index c36d3c1dabb..91ad413adb3 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/search/NodeResourcesTuning.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/search/NodeResourcesTuning.java
@@ -15,13 +15,16 @@ import static java.lang.Long.min;
 public class NodeResourcesTuning implements ProtonConfig.Producer {
 
     final static long MB = 1024 * 1024;
-    final static long GB = MB * 1024;
+    public final static long GB = MB * 1024;
     private final NodeResources resources;
     private final int redundancy;
     private final int searchableCopies;
     private final int threadsPerSearch;
     private final boolean combined;
 
+    // "Reserve" 1GB of memory for other processes running on the content node (config-proxy, cluster-controller, metrics-proxy).
+    public static final double reservedMemoryGb = 1;
+
     public NodeResourcesTuning(NodeResources resources,
                                int redundancy,
                                int searchableCopies,
@@ -45,7 +48,6 @@ public class NodeResourcesTuning implements ProtonConfig.Producer {
         tuneSummaryReadIo(builder.summary.read);
         tuneSummaryCache(builder.summary.cache);
         tuneSearchReadIo(builder.search.mmap);
-        tuneWriteFilter(builder.writefilter);
         for (ProtonConfig.Documentdb.Builder dbb : builder.documentdb) {
             getConfig(dbb);
         }
@@ -121,20 +123,15 @@ public class NodeResourcesTuning implements ProtonConfig.Producer {
         builder.numthreadspersearch(threadsPerSearch);
     }
 
-    private void tuneWriteFilter(ProtonConfig.Writefilter.Builder builder) {
-        // "Reserve" 1GB of memory for other processes running on the content node (config-proxy, cluster-controller, metrics-proxy)
-        double reservedMemoryGb = 1;
-        double defaultMemoryLimit = new ProtonConfig.Writefilter(new ProtonConfig.Writefilter.Builder()).memorylimit();
-        double scaledMemoryLimit = ((usableMemoryGb() - reservedMemoryGb) * defaultMemoryLimit) / usableMemoryGb();
-        builder.memorylimit(scaledMemoryLimit);
-    }
-
     /** Returns the memory we can expect will be available for the content node processes */
     private double usableMemoryGb() {
-        if ( ! combined ) return resources.memoryGb();
+        double usableMemoryGb = resources.memoryGb() - reservedMemoryGb;
+        if (!combined) {
+            return usableMemoryGb;
+        }
 
         double fractionTakenByContainer = (double)ApplicationContainerCluster.heapSizePercentageOfTotalNodeMemoryWhenCombinedCluster / 100;
-        return resources.memoryGb() * (1 - fractionTakenByContainer);
+        return usableMemoryGb * (1 - fractionTakenByContainer);
     }
 
 }
diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
index 5a71f83e468..3e6a3332f9a 100644
--- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
@@ -48,6 +48,8 @@ import java.util.stream.Collectors;
 
 import static com.yahoo.config.model.test.TestUtil.joinLines;
 import static com.yahoo.vespa.defaults.Defaults.getDefaults;
+import static com.yahoo.vespa.model.search.NodeResourcesTuning.GB;
+import static com.yahoo.vespa.model.search.NodeResourcesTuning.reservedMemoryGb;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -269,7 +271,7 @@ public class ModelProvisioningTest {
             assertEquals("Heap size is lowered with combined clusters",
                          17, physicalMemoryPercentage(model.getContainerClusters().get("container1")));
             assertEquals("Memory for proton is lowered to account for the jvm heap",
-                         (long)(3 * (Math.pow(1024, 3)) * (1 - 0.17)), protonMemorySize(model.getContentClusters().get("content1")));
+                         (long)((3 - reservedMemoryGb) * (Math.pow(1024, 3)) * (1 - 0.17)), protonMemorySize(model.getContentClusters().get("content1")));
             assertProvisioned(0, ClusterSpec.Id.from("container1"), ClusterSpec.Type.container, model);
             assertProvisioned(2, ClusterSpec.Id.from("content1"), ClusterSpec.Id.from("container1"), ClusterSpec.Type.combined, model);
         }
@@ -305,7 +307,7 @@ public class ModelProvisioningTest {
             assertEquals("Heap size is normal",
                          60, physicalMemoryPercentage(model.getContainerClusters().get("container1")));
             assertEquals("Memory for proton is normal",
-                         (long)(3 * (Math.pow(1024, 3))), protonMemorySize(model.getContentClusters().get("content1")));
+                         (long)((3 - reservedMemoryGb) * (Math.pow(1024, 3))), protonMemorySize(model.getContentClusters().get("content1")));
         }
     }
 
@@ -2002,12 +2004,10 @@ public class ModelProvisioningTest {
         ProtonConfig cfg = getProtonConfig(model, cluster.getSearchNodes().get(0).getConfigId());
         assertEquals(2000, cfg.flush().memory().maxtlssize()); // from config override
         assertEquals(1000, cfg.flush().memory().maxmemory()); // from explicit tuning
-        assertEquals((long) 16 * GB, cfg.flush().memory().each().maxmemory()); // from default node flavor tuning
+        assertEquals((long) (128 - reservedMemoryGb) * GB / 8, cfg.flush().memory().each().maxmemory()); // from default node flavor tuning
         assertEquals(0.92, cfg.writefilter().memorylimit(), 0.0001); // from explicit resource-limits
     }
 
-    private static long GB = 1024 * 1024 * 1024;
-
     private static ProtonConfig getProtonConfig(VespaModel model, String configId) {
         ProtonConfig.Builder builder = new ProtonConfig.Builder();
         model.getConfig(builder, configId);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/search/NodeResourcesTuningTest.java b/config-model/src/test/java/com/yahoo/vespa/model/search/NodeResourcesTuningTest.java
index 5df522e1b9d..1f9c9b1e07a 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/search/NodeResourcesTuningTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/search/NodeResourcesTuningTest.java
@@ -20,6 +20,8 @@ import static com.yahoo.vespa.model.search.NodeResourcesTuning.GB;
 public class NodeResourcesTuningTest {
 
     private static double delta = 0.00001;
+    private static double combinedFactor = 1 - 17.0/100;
+    private static int reservedMemoryGb = (int)NodeResourcesTuning.reservedMemoryGb;
 
     @Test
     public void require_that_hwinfo_disk_size_is_set() {
@@ -29,9 +31,13 @@ public class NodeResourcesTuningTest {
 
     @Test
     public void require_that_hwinfo_memory_size_is_set() {
-        double combinedFactor = 1 - 17.0/100;
-        assertEquals(24 * GB, configFromMemorySetting(24, false).hwinfo().memory().size());
-        assertEquals(combinedFactor * 24 * GB, configFromMemorySetting(24, true).hwinfo().memory().size(), 1000);
+        assertEquals(24 * GB, configFromMemorySetting(24 + reservedMemoryGb, false).hwinfo().memory().size());
+        assertEquals(combinedFactor * 24 * GB, configFromMemorySetting(24 + reservedMemoryGb, true).hwinfo().memory().size(), 1000);
+    }
+
+    @Test
+    public void reserved_memory_on_content_node_is_1_gb() {
+        assertEquals(1.0, NodeResourcesTuning.reservedMemoryGb, delta);
     }
 
     private ProtonConfig getProtonMemoryConfig(List<Pair<String, String>> sdAndMode, int gb, int redundancy, int searchableCopies) {
@@ -47,13 +53,13 @@ public class NodeResourcesTuningTest {
 
     private void verify_that_initial_numdocs_is_dependent_of_mode(int redundancy, int searchablecopies) {
         int divisor = Math.max(redundancy, searchablecopies);
-        ProtonConfig cfg = getProtonMemoryConfig(Arrays.asList(new Pair<>("a", "INDEX"), new Pair<>("b", "STREAMING"), new Pair<>("c", "STORE_ONLY")), 24, redundancy, searchablecopies);
+        ProtonConfig cfg = getProtonMemoryConfig(Arrays.asList(new Pair<>("a", "INDEX"), new Pair<>("b", "STREAMING"), new Pair<>("c", "STORE_ONLY")), 24 + reservedMemoryGb, redundancy, searchablecopies);
         assertEquals(3, cfg.documentdb().size());
         assertEquals(1024, cfg.documentdb(0).allocation().initialnumdocs());
         assertEquals("a", cfg.documentdb(0).inputdoctypename());
-        assertEquals(402653184/divisor, cfg.documentdb(1).allocation().initialnumdocs());
+        assertEquals(24 * GB / 64 / divisor, cfg.documentdb(1).allocation().initialnumdocs());
         assertEquals("b", cfg.documentdb(1).inputdoctypename());
-        assertEquals(402653184/divisor, cfg.documentdb(2).allocation().initialnumdocs());
+        assertEquals(24 * GB / 64 / divisor, cfg.documentdb(2).allocation().initialnumdocs());
         assertEquals("c", cfg.documentdb(2).inputdoctypename());
     }
 
@@ -148,15 +154,14 @@ public class NodeResourcesTuningTest {
 
     @Test
     public void require_that_summary_cache_max_bytes_is_set_based_on_memory() {
-        assertEquals(1*GB / 20, configFromMemorySetting(1, false).summary().cache().maxbytes());
-        assertEquals(256*GB / 20, configFromMemorySetting(256, false).summary().cache().maxbytes());
+        assertEquals(1*GB / 20, configFromMemorySetting(1 + reservedMemoryGb, false).summary().cache().maxbytes());
+        assertEquals(256*GB / 20, configFromMemorySetting(256 + reservedMemoryGb, false).summary().cache().maxbytes());
     }
 
     @Test
     public void require_that_summary_cache_memory_is_reduced_with_combined_cluster() {
-        double combinedFactor = 1 - 17.0/100;
-        assertEquals(combinedFactor * 1*GB / 20, configFromMemorySetting(1, true).summary().cache().maxbytes(), 1000);
-        assertEquals(combinedFactor * 256*GB / 20, configFromMemorySetting(256, true).summary().cache().maxbytes(), 1000);
+        assertEquals(combinedFactor * 1*GB / 20, configFromMemorySetting(1 + reservedMemoryGb, true).summary().cache().maxbytes(), 1000);
+        assertEquals(combinedFactor * 256*GB / 20, configFromMemorySetting(256 + reservedMemoryGb, true).summary().cache().maxbytes(), 1000);
     }
 
     @Test
@@ -164,21 +169,13 @@ public class NodeResourcesTuningTest {
         assertSharedDisk(true, true);
     }
 
-    @Test
-    public void require_that_write_filter_memory_limit_is_scaled() {
-        assertWriteFilter(0.7, 8);
-        assertWriteFilter(0.75, 16);
-        assertWriteFilter(0.775, 32);
-        assertWriteFilter(0.7875, 64);
-    }
-
-    private static void assertDocumentStoreMaxFileSize(long expFileSizeBytes, int memoryGb) {
-        assertEquals(expFileSizeBytes, configFromMemorySetting(memoryGb, false).summary().log().maxfilesize());
+    private static void assertDocumentStoreMaxFileSize(long expFileSizeBytes, int wantedMemoryGb) {
+        assertEquals(expFileSizeBytes, configFromMemorySetting(wantedMemoryGb + reservedMemoryGb, false).summary().log().maxfilesize());
     }
 
-    private static void assertFlushStrategyMemory(long expMemoryBytes, int memoryGb) {
-        assertEquals(expMemoryBytes, configFromMemorySetting(memoryGb, false).flush().memory().maxmemory());
-        assertEquals(expMemoryBytes, configFromMemorySetting(memoryGb, false).flush().memory().each().maxmemory());
+    private static void assertFlushStrategyMemory(long expMemoryBytes, int wantedMemoryGb) {
+        assertEquals(expMemoryBytes, configFromMemorySetting(wantedMemoryGb + reservedMemoryGb, false).flush().memory().maxmemory());
+        assertEquals(expMemoryBytes, configFromMemorySetting(wantedMemoryGb + reservedMemoryGb, false).flush().memory().each().maxmemory());
     }
 
     private static void assertFlushStrategyTlsSize(long expTlsSizeBytes, int diskGb) {