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

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

9 files changed, 45 insertions, 263 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..b00f46a3d00 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));
diff --git a/vdslib/src/main/java/com/yahoo/vdslib/state/DiskState.java b/vdslib/src/main/java/com/yahoo/vdslib/state/DiskState.java
deleted file mode 100644
index 1a05f8ab565..00000000000
--- a/vdslib/src/main/java/com/yahoo/vdslib/state/DiskState.java
+++ /dev/null
@@ -1,135 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vdslib.state;
-
-import com.yahoo.text.StringUtilities;
-
-import java.util.StringTokenizer;
-import java.text.ParseException;
-
-/**
- *
- */
-public class DiskState implements Cloneable {
-    private State state = State.UP;
-    private String description = "";
-    private double capacity = 1.0;
-
-    public DiskState() {}
-    public DiskState(State s) {
-        setState(s);
-    }
-    public DiskState(State s, String description, double capacity) {
-        setState(s); // Set via set methods, so we can have illegal argument checks only one place
-        setCapacity(capacity);
-        setDescription(description);
-    }
-    public DiskState clone() {
-        try{
-            return (DiskState) super.clone();
-        } catch (CloneNotSupportedException e) {
-            assert(false); // Should not happen
-            return null;
-        }
-    }
-
-    public DiskState(String serialized) throws ParseException {
-        StringTokenizer st = new StringTokenizer(serialized, " \t\f\r\n");
-        while (st.hasMoreTokens()) {
-            String token = st.nextToken();
-            int index = token.indexOf(':');
-            if (index < 0) {
-                throw new ParseException("Token " + token + " does not contain ':': " + serialized, 0);
-            }
-            String key = token.substring(0, index);
-            String value = token.substring(index + 1);
-            if (key.length() > 0) switch (key.charAt(0)) {
-                case 's':
-                    if (key.length() > 1) break;
-                    setState(State.get(value));
-                    continue;
-                case 'c':
-                    if (key.length() > 1) break;
-                    try{
-                        setCapacity(Double.valueOf(value));
-                    } catch (Exception e) {
-                        throw new ParseException("Illegal disk capacity '" + value + "'. Capacity must be a positive floating point number", 0);
-                    }
-                    continue;
-                case 'm':
-                    if (key.length() > 1) break;
-                    description = StringUtilities.unescape(value);
-                    continue;
-                default:
-                    break;
-            }
-            // Ignore unknown tokens
-        }
-    }
-
-    public String serialize(String prefix, boolean includeDescription) {
-        boolean empty = true;
-        StringBuilder sb = new StringBuilder();
-        if (!state.equals(State.UP) || prefix.length() < 2) {
-            sb.append(prefix).append("s:").append(state.serialize());
-            empty = false;
-        }
-        if (Math.abs(capacity - 1.0) > 0.000000001) {
-            if (empty) { empty = false; } else { sb.append(' '); }
-            sb.append(prefix).append("c:").append(capacity);
-        }
-        if (includeDescription && description.length() > 0) {
-            if (!empty) { sb.append(' '); }
-            sb.append(prefix).append("m:").append(StringUtilities.escape(description, ' '));
-        }
-        return sb.toString();
-    }
-
-    public State getState() { return state; }
-    public double getCapacity() { return capacity; }
-    public String getDescription() { return description; }
-
-    public void setState(State s) {
-        if (!s.validDiskState()) {
-            throw new IllegalArgumentException("State " + s + " is not a valid disk state.");
-        }
-        state = s;
-    }
-    public void setCapacity(double capacity) {
-        if (capacity < 0) {
-            throw new IllegalArgumentException("Negative capacity makes no sense.");
-        }
-        this.capacity = capacity;
-    }
-    public void setDescription(String desc) { description = desc; }
-
-    public String toString() {
-        StringBuilder sb = new StringBuilder();
-        sb.append("DiskState(").append(state.serialize());
-        if (Math.abs(capacity - 1.0) > 0.00000001) {
-            sb.append(", capacity ").append(capacity);
-        }
-        if (description.length() > 0) {
-            sb.append(": ").append(description);
-        }
-        sb.append(")");
-        return sb.toString();
-    }
-
-    @Override
-    public boolean equals(Object o) {
-        if (!(o instanceof DiskState)) { return false; }
-        DiskState other = (DiskState) o;
-        if (state.equals(other.state)
-            && Math.abs(capacity - other.capacity) < 0.00000001)
-        {
-            return true;
-        }
-        return false;
-    }
-
-    @Override
-    public int hashCode() {
-        // NOTE: capacity cannot be part of the hashCode
-        return state.hashCode();
-    }
-}
diff --git a/vdslib/src/test/java/com/yahoo/vdslib/state/DiskStateTestCase.java b/vdslib/src/test/java/com/yahoo/vdslib/state/DiskStateTestCase.java
deleted file mode 100644
index bdc3be3dc70..00000000000
--- a/vdslib/src/test/java/com/yahoo/vdslib/state/DiskStateTestCase.java
+++ /dev/null
@@ -1,116 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vdslib.state;
-
-import org.junit.Test;
-
-import java.text.ParseException;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
-public class DiskStateTestCase {
-
-    private static final double delta = 0.0000000001;
-
-    @Test
-    public void testEquals() {
-        DiskState d1 = new DiskState(State.UP, "", 1);
-        DiskState d2 = new DiskState(State.UP, "", 2);
-        DiskState d3 = new DiskState(State.DOWN, "Failed disk", 1);
-        DiskState d4 = new DiskState(State.DOWN, "IO error", 1);
-        DiskState d5 = new DiskState(State.UP, "", 1);
-        DiskState d6 = new DiskState(State.UP, "", 2);
-        DiskState d7 = new DiskState(State.DOWN, "Failed disk", 1);
-        DiskState d8 = new DiskState(State.DOWN, "IO error", 1);
-
-        assertEquals(d1, d5);
-        assertEquals(d2, d6);
-        assertEquals(d3, d7);
-        assertEquals(d4, d8);
-
-        assertFalse(d1.equals(d2));
-        assertFalse(d1.equals(d3));
-        assertFalse(d1.equals(d4));
-
-        assertFalse(d2.equals(d1));
-        assertFalse(d2.equals(d3));
-        assertFalse(d2.equals(d4));
-
-        assertFalse(d3.equals(d1));
-        assertFalse(d3.equals(d2));
-        assertEquals(d3, d4);
-
-        assertFalse(d4.equals(d1));
-        assertFalse(d4.equals(d2));
-        assertEquals(d4, d3);
-
-        assertFalse(d1.equals("class not instance of Node"));
-    }
-
-    @Test
-    public void testSerialization() throws ParseException {
-        DiskState d = new DiskState();
-        DiskState other = new DiskState(d.serialize("", true));
-        assertEquals(d, other);
-        assertEquals(d.toString(), other.toString());
-        assertEquals(State.UP, other.getState());
-        assertEquals(1.0, other.getCapacity(), delta);
-        assertEquals("", other.getDescription());
-        assertEquals("s:u", d.serialize("", false));
-        assertEquals("s:u", d.serialize("", true));
-        assertEquals("", d.serialize(".0.", false));
-        assertEquals("", d.serialize(".0.", true));
-
-        assertEquals(d, new DiskState(": s:u sbadkey:somevalue cbadkey:somevalue mbadkey:somevalue unknwonkey:somevalue"));
-
-        d = new DiskState(State.UP, "Slow disk", 1.0);
-        other = new DiskState(d.serialize("", true));
-        assertEquals(d, other);
-        assertEquals(d.toString(), other.toString());
-        assertEquals(State.UP, other.getState());
-        assertEquals(1.0, other.getCapacity(), delta);
-        assertEquals("Slow disk", other.getDescription());
-        assertEquals("s:u", d.serialize("", false));
-        assertEquals("s:u m:Slow\\x20disk", d.serialize("", true));
-        assertEquals("", d.serialize(".0.", false));
-        assertEquals(".0.m:Slow\\x20disk", d.serialize(".0.", true));
-
-        d = new DiskState(State.DOWN, "Failed disk", 2.0);
-        other = new DiskState(d.serialize("", true));
-        assertEquals(d, other);
-        assertEquals(d.toString(), other.toString());
-        assertEquals(State.DOWN, other.getState());
-        assertEquals(2.0, other.getCapacity(), delta);
-        assertEquals("Failed disk", other.getDescription());
-        assertEquals("s:d c:2.0", d.serialize("", false));
-        assertEquals("s:d c:2.0 m:Failed\\x20disk", d.serialize("", true));
-        assertEquals(".0.s:d .0.c:2.0", d.serialize(".0.", false));
-        assertEquals(".0.s:d .0.c:2.0 .0.m:Failed\\x20disk", d.serialize(".0.", true));
-
-        try {
-            new DiskState(State.MAINTENANCE);
-            assertTrue("Method expected to throw IllegalArgumentException", false);
-        } catch (IllegalArgumentException e) {
-            assertEquals("State " + State.MAINTENANCE + " is not a valid disk state.", e.getMessage());
-        }
-        try {
-            new DiskState(State.UP, "", -1);
-            assertTrue("Method expected to throw IllegalArgumentException", false);
-        } catch (IllegalArgumentException e) {
-            assertEquals("Negative capacity makes no sense.", e.getMessage());
-        }
-        try {
-            new DiskState("nocolon");
-            assertTrue("Method expected to throw ParseException", false);
-        } catch (ParseException e) {
-            assertEquals("Token nocolon does not contain ':': nocolon", e.getMessage());
-        }
-        try {
-            new DiskState("s:d c:badvalue");
-            assertTrue("Method expected to throw ParseException", false);
-        } catch (ParseException e) {
-            assertEquals("Illegal disk capacity 'badvalue'. Capacity must be a positive floating point number", e.getMessage());
-        }
-    }
-}