From 7fbe373ed95d7680ab75bf221de2d80bfa46e84e Mon Sep 17 00:00:00 2001
From: Martin Polden <mpolden@mpolden.no>
Date: Wed, 14 Jun 2023 10:25:25 +0200
Subject: Warn on low disk compared to memory

---
 .../model/builder/xml/dom/NodesSpecification.java  | 19 +++++++++++
 .../model/provision/ModelProvisioningTest.java     | 38 ++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
index e70c555a366..9621e8ab9eb 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
@@ -25,6 +25,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.function.Function;
+import java.util.logging.Level;
 
 /**
  * A common utility class to represent a requirement for nodes during model building.
@@ -289,9 +290,27 @@ public class NodesSpecification {
                                          .loadBalancerSettings(zoneEndpoint)
                                          .stateful(stateful)
                                          .build();
+        logInsufficientDiskResources(clusterId, clusterType, logger);
         return hostSystem.allocateHosts(cluster, Capacity.from(min, max, groupSize, required, canFail, cloudAccount, info), logger);
     }
 
+    /** Log a message if requested disk may not fit core/heap dumps */
+    private void logInsufficientDiskResources(ClusterSpec.Id clusterId, ClusterSpec.Type clusterType, DeployLogger deployLogger) {
+        NodeResources resources = min.nodeResources();
+        if (resources.diskGbIsUnspecified() || resources.memoryGbIsUnspecified()) return;
+        double minDiskGb = resources.memoryGb() * switch (clusterType) {
+            case combined, content -> 3;
+            case container -> 2;
+            default -> 0; // No constraint on other types
+        };
+        if (resources.diskGb() < minDiskGb) {
+            deployLogger.logApplicationPackage(Level.WARNING, "Requested disk (" + resources.diskGb() +
+                                                              "Gb) in " + clusterId + " is not large enough to fit " +
+                                                              "core/heap dumps. Minimum recommended disk resources " +
+                                                              "is " + minDiskGb + "Gb");
+        }
+    }
+
     private static Pair<NodeResources, NodeResources> nodeResources(ModelElement nodesElement) {
         ModelElement resources = nodesElement.child("resources");
         if (resources != null) {
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 8b8191ebbbb..84804bc48fa 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
@@ -2579,6 +2579,44 @@ public class ModelProvisioningTest {
         assertEquals((long) ((128 - memoryOverheadGb) * GB * 0.08), cfg.flush().memory().each().maxmemory()); // from default node flavor tuning
     }
 
+    @Test
+    public void warn_on_insufficient_disk_resources() {
+        String services = """
+                <?xml version='1.0' encoding='utf-8' ?>
+                <services>
+                  <container version='1.0' id='c1'>
+                    <nodes count='1'>
+                      <resources vcpu='1' memory='24Gb' disk='40Gb'/>
+                    </nodes>
+                  </container>
+                  <container version='1.0' id='c2'>
+                    <nodes count='1'>
+                      <resources vcpu='1' memory='24Gb' disk='50Gb'/>
+                    </nodes>
+                  </container>
+                  <content version='1.0' id='c3'>
+                    <redundancy>1</redundancy>
+                    <documents>
+                      <document type='type1' mode='index'/>
+                    </documents>
+                    <nodes count='1'>
+                      <resources vcpu='1' memory='24Gb' disk='50Gb'/>
+                    </nodes>
+                  </content>
+                </services>
+                """;
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts(new NodeResources(1, 24, 50, 1, DiskSpeed.fast), 10);
+        TestLogger testLogger = new TestLogger();
+        VespaModel model = tester.createModel(services, true, new DeployState.Builder().deployLogger(testLogger));
+        assertEquals(1, model.getContainerClusters().get("c1").getContainers().size());
+        assertEquals(1, model.getContainerClusters().get("c2").getContainers().size());
+        assertEquals(1, model.getContentClusters().get("c3").getSearch().getSearchNodes().size());
+        assertEquals(List.of(new TestLogger.LogMessage(Level.WARNING, "Requested disk (40.0Gb) in cluster 'c1' is not large enough to fit core/heap dumps. Minimum recommended disk resources is 48.0Gb"),
+                             new TestLogger.LogMessage(Level.WARNING, "Requested disk (50.0Gb) in cluster 'c3' is not large enough to fit core/heap dumps. Minimum recommended disk resources is 72.0Gb")),
+                     testLogger.msgs());
+    }
+
     private static ProtonConfig getProtonConfig(VespaModel model, String configId) {
         ProtonConfig.Builder builder = new ProtonConfig.Builder();
         model.getConfig(builder, configId);
-- 
cgit v1.2.3