Merge pull request #16935 from vespa-engine/revert-16934-revert-16932-balder/move-metrics-from-partition-to-node-level

Revert "Revert "GC unused DiskState and add the partition metrics to node level.""

7 files changed, 45 insertions, 39 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeInfo.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeInfo.java
index 6d97d9b2ce2..f014690947d 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeInfo.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeInfo.java
@@ -6,7 +6,11 @@ import com.yahoo.jrt.Target;
 import java.util.logging.Level;
 import com.yahoo.vdslib.distribution.Distribution;
 import com.yahoo.vdslib.distribution.Group;
-import com.yahoo.vdslib.state.*;
+import com.yahoo.vdslib.state.ClusterState;
+import com.yahoo.vdslib.state.Node;
+import com.yahoo.vdslib.state.NodeState;
+import com.yahoo.vdslib.state.NodeType;
+import com.yahoo.vdslib.state.State;
 import com.yahoo.vespa.clustercontroller.core.hostinfo.HostInfo;
 import com.yahoo.vespa.clustercontroller.core.rpc.RPCCommunicator;
 
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/Id.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/Id.java
index ed5af93d7fb..d956afba48a 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/Id.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/Id.java
@@ -60,7 +60,6 @@ public class Id {
             this.id = partition;
         }
 
-        public final int getPartitionIndex() { return id; }
         public String toString() { return super.toString() + "/" + id; }
     }
 
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/Response.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/Response.java
index 17949b82365..82512262b4a 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/Response.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/Response.java
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.clustercontroller.core.restapiv2;
 
-import com.yahoo.vdslib.state.DiskState;
 import com.yahoo.vdslib.state.NodeState;
 import com.yahoo.vdslib.state.State;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.InternalFailure;
@@ -32,10 +31,6 @@ public class Response {
             this.id = parseId(ns.getState());
             this.reason = ns.getDescription();
         }
-        public UnitStateImpl(DiskState ds) throws StateRestApiException {
-            this.id = parseId(ds.getState());
-            this.reason = ds.getDescription();
-        }
 
         public String parseId(State id) throws StateRestApiException {
             switch (id) {
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/NodeStateRequest.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/NodeStateRequest.java
index e007e2cc243..1404508e2fe 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/NodeStateRequest.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/NodeStateRequest.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.clustercontroller.core.restapiv2.requests;
 
 import com.yahoo.vespa.clustercontroller.core.NodeInfo;
 import com.yahoo.vespa.clustercontroller.core.RemoteClusterControllerTask;
+import com.yahoo.vespa.clustercontroller.core.hostinfo.Metrics;
 import com.yahoo.vespa.clustercontroller.core.restapiv2.Id;
 import com.yahoo.vespa.clustercontroller.core.restapiv2.Request;
 import com.yahoo.vespa.clustercontroller.core.restapiv2.Response;
@@ -32,7 +33,34 @@ public class NodeStateRequest extends Request<Response.NodeResponse> {
         result.addState("generated", new Response.UnitStateImpl(context.currentConsolidatedState.getNodeState(id.getNode())));
         result.addState("unit", new Response.UnitStateImpl(info.getReportedState()));
         result.addState("user", new Response.UnitStateImpl(info.getWantedState()));
+        if (info.isStorage()) {
+            fillInMetrics(context.cluster.getNodeInfo(id.getNode()).getHostInfo().getMetrics(), result);
+        }
 
         return result;
     }
+    private static void fillInMetrics(Metrics metrics, Response.NodeResponse result) {
+        for (Metrics.Metric metric: metrics.getMetrics()) {
+            fillInMetricValue(metric.getName(), metric.getValue(), result);
+        }
+    }
+
+    private static void fillInMetricValue(String name, Metrics.Value value, Response.NodeResponse result) {
+        if (name.equals("vds.datastored.alldisks.docs")) {
+            if (value.getLast() == null) {
+                return;
+            }
+            result.addMetric("unique-document-count", value.getLast());
+        } else if (name.equals("vds.datastored.alldisks.bytes")) {
+            if (value.getLast() == null) {
+                return;
+            }
+            result.addMetric("unique-document-total-size", value.getLast());
+        } else if (name.equals("vds.datastored.alldisks.buckets")) {
+            if (value.getLast() == null) {
+                return;
+            }
+            result.addMetric("bucket-count", value.getLast());
+        }
+    }
 }
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/PartitionStateRequest.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/PartitionStateRequest.java
index bb42af45ef3..b565bf0b572 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/PartitionStateRequest.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/PartitionStateRequest.java
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.clustercontroller.core.restapiv2.requests;
 
-import com.yahoo.vdslib.state.DiskState;
 import com.yahoo.vdslib.state.NodeState;
 import com.yahoo.vespa.clustercontroller.core.RemoteClusterControllerTask;
 import com.yahoo.vespa.clustercontroller.core.hostinfo.Metrics;
@@ -13,6 +12,7 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.StateRestApiE
 import java.util.Set;
 import java.util.logging.Logger;
 
+//TODO Remove once we have ensured that partition level is no longer used (it has never been)
 public class PartitionStateRequest extends Request<Response.PartitionResponse> {
     private static final Logger log = Logger.getLogger(PartitionStateRequest.class.getName());
     private final Id.Partition id;
@@ -32,8 +32,7 @@ public class PartitionStateRequest extends Request<Response.PartitionResponse> {
             fillInMetrics(context.cluster.getNodeInfo(id.getNode()).getHostInfo().getMetrics(), result);
         }
         NodeState nodeState = context.currentConsolidatedState.getNodeState(id.getNode());
-        DiskState diskState = new DiskState();
-        result.addState("generated", new Response.UnitStateImpl(diskState));
+        result.addState("generated", new Response.UnitStateImpl(nodeState));
 
         return result;
     }
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/ServiceStateRequest.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/ServiceStateRequest.java
index 9dd5ed7b071..b5452af79cc 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/ServiceStateRequest.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/ServiceStateRequest.java
@@ -7,8 +7,6 @@ import com.yahoo.vespa.clustercontroller.core.restapiv2.Request;
 import com.yahoo.vespa.clustercontroller.core.restapiv2.Response;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.StateRestApiException;
 
-import java.util.EnumSet;
-
 public class ServiceStateRequest extends Request<Response.ServiceResponse> {
     private final Id.Service id;
     private final int recursive;
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NodeTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NodeTest.java
index 67ae7911f80..d9091942120 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NodeTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NodeTest.java
@@ -59,6 +59,11 @@ public class NodeTest extends StateRestApiTest {
                 "      \"state\": \"up\",\n" +
                 "      \"reason\": \"\"\n" +
                 "    }\n" +
+                "  },\n" +
+                "  \"metrics\": {\n" +
+                "    \"bucket-count\": 1,\n" +
+                "    \"unique-document-count\": 2,\n" +
+                "    \"unique-document-total-size\": 3\n" +
                 "  }\n" +
                 "}";
         assertEquals(expected, jsonWriter.createJson(response).toString(2));
@@ -84,6 +89,11 @@ public class NodeTest extends StateRestApiTest {
                 "      \"state\": \"up\",\n" +
                 "      \"reason\": \"\"\n" +
                 "    }\n" +
+                "  },\n" +
+                "  \"metrics\": {\n" +
+                "    \"bucket-count\": 1,\n" +
+                "    \"unique-document-count\": 2,\n" +
+                "    \"unique-document-total-size\": 3\n" +
                 "  }\n" +
                 "}";
         assertEquals(expected, jsonWriter.createJson(response).toString(2));
@@ -118,31 +128,4 @@ public class NodeTest extends StateRestApiTest {
                 "}";
         assertEquals(expected, jsonWriter.createJson(response).toString(2));
     }
-
-    @Test
-    public void testRecursiveStorageClusterDoesNotIncludePerNodeStatsOrMetrics() throws Exception {
-        setUp(true);
-        UnitResponse response = restAPI.getState(new StateRequest("music/storage", 1));
-        String expected =
-                "{\n" +
-                "  \"attributes\": {\"hierarchical-group\": \"east.g2\"},\n" +
-                "  \"state\": {\n" +
-                "    \"generated\": {\n" +
-                "      \"state\": \"up\",\n" +
-                "      \"reason\": \"\"\n" +
-                "    },\n" +
-                "    \"unit\": {\n" +
-                "      \"state\": \"up\",\n" +
-                "      \"reason\": \"\"\n" +
-                "    },\n" +
-                "    \"user\": {\n" +
-                "      \"state\": \"up\",\n" +
-                "      \"reason\": \"\"\n" +
-                "    }\n" +
-                "  }\n" +
-                "}";
-        JSONObject json = jsonWriter.createJson(response);
-        assertEquals(expected, json.getJSONObject("node").getJSONObject("1").toString(2));
-    }
-
 }