Reserve memory for other processes on the content node the proper way by including it in hwinfo memory.

This removes the need to scale the write filter limit used for memory.

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 37619d3afc6..be272e05bf5 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
@@ -47,6 +47,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;
@@ -268,7 +270,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);
         }
@@ -304,7 +306,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")));
         }
     }
 
@@ -2000,12 +2002,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) {