Merge pull request #16292 from vespa-engine/vekterli/support-optional-resource-usage-name-field

Support optional resource usage name field

7 files changed, 86 insertions, 25 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ResourceExhaustionCalculator.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ResourceExhaustionCalculator.java
index 80b8a6110f1..c91c5dbeb8d 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ResourceExhaustionCalculator.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ResourceExhaustionCalculator.java
@@ -37,9 +37,7 @@ public class ResourceExhaustionCalculator {
         int maxDescriptions = 3;
         String description = exhaustions.stream()
                 .limit(maxDescriptions)
-                .map(n -> String.format("%s on node %s (%.3g > %.3g)",
-                        n.resourceType, n.node.getIndex(),
-                        n.resourceUsage.getUsage(), n.limit))
+                .map(ResourceExhaustionCalculator::formatNodeResourceExhaustion)
                 .collect(Collectors.joining(", "));
         if (exhaustions.size() > maxDescriptions) {
             description += String.format(" (... and %d more)", exhaustions.size() - maxDescriptions);
@@ -47,6 +45,14 @@ public class ResourceExhaustionCalculator {
         return ClusterStateBundle.FeedBlock.blockedWithDescription(description);
     }
 
+    private static String formatNodeResourceExhaustion(NodeResourceExhaustion n) {
+        return String.format("%s%s on node %s (%.3g > %.3g)",
+                             n.resourceType,
+                             (n.resourceUsage.getName() != null ? ":" + n.resourceUsage.getName() : ""),
+                             n.node.getIndex(),
+                             n.resourceUsage.getUsage(), n.limit);
+    }
+
     public List<NodeResourceExhaustion> resourceExhaustionsFromHostInfo(Node node, HostInfo hostInfo) {
         List<NodeResourceExhaustion> exceedingLimit = null;
         for (var usage : hostInfo.getContentNode().getResourceUsage().entrySet()) {
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/ResourceUsage.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/ResourceUsage.java
index e47ec5452a4..876bf9480a6 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/ResourceUsage.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/ResourceUsage.java
@@ -11,9 +11,12 @@ import java.util.Objects;
  */
 public class ResourceUsage {
     private final Double usage;
+    private final String name;
 
-    public ResourceUsage(@JsonProperty("usage") Double usage) {
+    public ResourceUsage(@JsonProperty("usage") Double usage,
+                         @JsonProperty("name") String name) {
         this.usage = usage;
+        this.name = name;
     }
 
     /** Resource usage in [0, 1] */
@@ -21,16 +24,20 @@ public class ResourceUsage {
         return usage;
     }
 
+    public String getName() {
+        return name;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         ResourceUsage that = (ResourceUsage) o;
-        return Objects.equals(usage, that.usage);
+        return Objects.equals(usage, that.usage) && Objects.equals(name, that.name);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(usage);
+        return Objects.hash(usage, name);
     }
 }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFeedBlockTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFeedBlockTest.java
index 2ac7113741b..5c6fcd21701 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFeedBlockTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFeedBlockTest.java
@@ -17,8 +17,10 @@ import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static com.yahoo.vespa.clustercontroller.core.FeedBlockUtil.mapOf;
+import static com.yahoo.vespa.clustercontroller.core.FeedBlockUtil.setOf;
 import static com.yahoo.vespa.clustercontroller.core.FeedBlockUtil.usage;
 import static com.yahoo.vespa.clustercontroller.core.FeedBlockUtil.createResourceUsageJson;
 import static org.junit.Assert.assertFalse;
@@ -89,7 +91,7 @@ public class ClusterFeedBlockTest extends FleetControllerTest {
         return options;
     }
 
-    private void reportResourceUsageFromNode(int nodeIndex, Map<String, Double> resourceUsages) throws Exception {
+    private void reportResourceUsageFromNode(int nodeIndex, Set<FeedBlockUtil.UsageDetails> resourceUsages) throws Exception {
         String hostInfo = createResourceUsageJson(resourceUsages);
         communicator.setNodeState(new Node(NodeType.STORAGE, nodeIndex), new NodeState(NodeType.STORAGE, State.UP), hostInfo);
         ctrl.tick();
@@ -102,17 +104,17 @@ public class ClusterFeedBlockTest extends FleetControllerTest {
         assertFalse(ctrl.getClusterStateBundle().clusterFeedIsBlocked());
 
         // Too much cheese in use, must block feed!
-        reportResourceUsageFromNode(1, mapOf(usage("cheese", 0.8), usage("wine", 0.3)));
+        reportResourceUsageFromNode(1, setOf(usage("cheese", 0.8), usage("wine", 0.3)));
         assertTrue(ctrl.getClusterStateBundle().clusterFeedIsBlocked());
         // TODO check desc?
 
         // Wine usage has gone up too, we should remain blocked
-        reportResourceUsageFromNode(1, mapOf(usage("cheese", 0.8), usage("wine", 0.5)));
+        reportResourceUsageFromNode(1, setOf(usage("cheese", 0.8), usage("wine", 0.5)));
         assertTrue(ctrl.getClusterStateBundle().clusterFeedIsBlocked());
         // TODO check desc?
 
         // Back to normal wine and cheese levels
-        reportResourceUsageFromNode(1, mapOf(usage("cheese", 0.6), usage("wine", 0.3)));
+        reportResourceUsageFromNode(1, setOf(usage("cheese", 0.6), usage("wine", 0.3)));
         assertFalse(ctrl.getClusterStateBundle().clusterFeedIsBlocked());
     }
 
@@ -121,7 +123,7 @@ public class ClusterFeedBlockTest extends FleetControllerTest {
         initialize(createOptions(mapOf(usage("cheese", 0.7), usage("wine", 0.4))));
         assertFalse(ctrl.getClusterStateBundle().clusterFeedIsBlocked());
 
-        reportResourceUsageFromNode(1, mapOf(usage("cheese", 0.8), usage("wine", 0.3)));
+        reportResourceUsageFromNode(1, setOf(usage("cheese", 0.8), usage("wine", 0.3)));
         assertTrue(ctrl.getClusterStateBundle().clusterFeedIsBlocked());
 
         // Increase cheese allowance. Should now automatically unblock since reported usage is lower.
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FeedBlockUtil.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FeedBlockUtil.java
index e2894705352..a26f8791cda 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FeedBlockUtil.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FeedBlockUtil.java
@@ -2,46 +2,78 @@
 package com.yahoo.vespa.clustercontroller.core;
 
 import java.util.Arrays;
+import java.util.LinkedHashSet;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 public class FeedBlockUtil {
 
     static class NodeAndUsages {
         public final int index;
-        public final Map<String, Double> usages;
+        public final Set<UsageDetails> usages;
 
-        public NodeAndUsages(int index, Map<String, Double> usages) {
+        public NodeAndUsages(int index, Set<UsageDetails> usages) {
             this.index = index;
             this.usages = usages;
         }
     }
 
-    static class NameAndUsage {
+    static class UsageDetails {
+        public final String type;
         public final String name;
         public final double usage;
 
-        public NameAndUsage(String name, double usage) {
+        public UsageDetails(String type, String name, double usage) {
+            this.type = type;
             this.name = name;
             this.usage = usage;
         }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+            UsageDetails that = (UsageDetails) o;
+            return Double.compare(that.usage, usage) == 0 &&
+                    Objects.equals(type, that.type) &&
+                    Objects.equals(name, that.name);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(type, name, usage);
+        }
+    }
+
+    static UsageDetails usage(String type, double usage) {
+        return new UsageDetails(type, null, usage);
+    }
+
+    static UsageDetails usage(String type, String name, double usage) {
+        return new UsageDetails(type, name, usage);
     }
 
-    static NameAndUsage usage(String name, double usage) {
-        return new NameAndUsage(name, usage);
+    static Map<String, Double> mapOf(UsageDetails... usages) {
+        return Arrays.stream(usages).collect(Collectors.toMap(u -> u.type, u -> u.usage));
     }
 
-    static Map<String, Double> mapOf(NameAndUsage... usages) {
-        return Arrays.stream(usages).collect(Collectors.toMap(u -> u.name, u -> u.usage));
+    static Set<UsageDetails> setOf(UsageDetails... usages) {
+        // Preserve input order to make stringification tests deterministic
+        return Arrays.stream(usages).collect(Collectors.toCollection(LinkedHashSet::new));
     }
 
-    static NodeAndUsages forNode(int index, NameAndUsage... usages) {
-        return new NodeAndUsages(index, mapOf(usages));
+    static NodeAndUsages forNode(int index, UsageDetails... usages) {
+        return new NodeAndUsages(index, setOf(usages));
     }
 
-    static String createResourceUsageJson(Map<String, Double> usages) {
-        String usageInnerJson = usages.entrySet().stream()
-                .map(kv -> String.format("\"%s\":{\"usage\": %.3g}", kv.getKey(), kv.getValue()))
+    static String createResourceUsageJson(Set<UsageDetails> usages) {
+        // We deal only in the finest of manual JSON string building technologies(tm).
+        String usageInnerJson = usages.stream()
+                .map(u -> String.format("\"%s\":{\"usage\": %.3g%s}",
+                        u.type, u.usage,
+                        (u.name != null ? String.format(",\"name\":\"%s\"", u.name) : "")))
                 .collect(Collectors.joining(","));
         return String.format("{\"content-node\":{\"resource-usage\":{%s}}}", usageInnerJson);
     }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ResourceExhaustionCalculatorTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ResourceExhaustionCalculatorTest.java
index 5a5cda1f4ed..ed1826046a8 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ResourceExhaustionCalculatorTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ResourceExhaustionCalculatorTest.java
@@ -10,6 +10,7 @@ import static com.yahoo.vespa.clustercontroller.core.ClusterFixture.storageNode;
 import static com.yahoo.vespa.clustercontroller.core.FeedBlockUtil.NodeAndUsages;
 import static com.yahoo.vespa.clustercontroller.core.FeedBlockUtil.forNode;
 import static com.yahoo.vespa.clustercontroller.core.FeedBlockUtil.mapOf;
+import static com.yahoo.vespa.clustercontroller.core.FeedBlockUtil.setOf;
 import static com.yahoo.vespa.clustercontroller.core.FeedBlockUtil.usage;
 import static com.yahoo.vespa.clustercontroller.core.FeedBlockUtil.createResourceUsageJson;
 
@@ -54,6 +55,17 @@ public class ResourceExhaustionCalculatorTest {
     }
 
     @Test
+    public void feed_block_description_can_contain_optional_name_component() {
+        var calc = new ResourceExhaustionCalculator(true, mapOf(usage("disk", 0.5), usage("memory", 0.8)));
+        var cf = createFixtureWithReportedUsages(forNode(1, usage("disk", "a-fancy-disk", 0.51), usage("memory", 0.79)),
+                forNode(2, usage("disk", 0.4), usage("memory", 0.6)));
+        var feedBlock = calc.inferContentClusterFeedBlockOrNull(cf.cluster().getNodeInfo());
+        assertNotNull(feedBlock);
+        assertTrue(feedBlock.blockFeedInCluster());
+        assertEquals("disk:a-fancy-disk on node 1 (0.510 > 0.500)", feedBlock.getDescription());
+    }
+
+    @Test
     public void feed_block_returned_when_multiple_resources_beyond_limit() {
         var calc = new ResourceExhaustionCalculator(true, mapOf(usage("disk", 0.4), usage("memory", 0.8)));
         var cf = createFixtureWithReportedUsages(forNode(1, usage("disk", 0.51), usage("memory", 0.85)),
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java
index f9b0a4ca36f..b2d3eb54f78 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java
@@ -75,6 +75,7 @@ public class HostInfoTest {
         assertEquals(resourceUsage.size(), 2);
         assertEquals(Optional.ofNullable(resourceUsage.get("memory")).map(ResourceUsage::getUsage).orElse(0.0), 0.85, 0.00001);
         assertEquals(Optional.ofNullable(resourceUsage.get("disk")).map(ResourceUsage::getUsage).orElse(0.0), 0.6, 0.00001);
+        assertEquals(Optional.ofNullable(resourceUsage.get("disk")).map(ResourceUsage::getName).orElse("missing"), "a cool disk");
         assertNull(resourceUsage.get("flux-capacitor"));
     }
 
diff --git a/protocols/getnodestate/host_info.json b/protocols/getnodestate/host_info.json
index 7ae5b0043ff..7eddf506e63 100644
--- a/protocols/getnodestate/host_info.json
+++ b/protocols/getnodestate/host_info.json
@@ -110,7 +110,8 @@
                 "usage": 0.85
             },
             "disk": {
-                "usage": 0.6
+                "usage": 0.6,
+                "name": "a cool disk"
             }
         }
     }