From bed3abea6afed8459bbd0647ce3e5b5439c438aa Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Fri, 12 Mar 2021 20:28:20 +0100 Subject: Revert "GC unused DiskState and add the partition metrics to node level." --- .../vespa/clustercontroller/core/NodeInfo.java | 6 +- .../vespa/clustercontroller/core/restapiv2/Id.java | 1 + .../clustercontroller/core/restapiv2/Response.java | 5 + .../core/restapiv2/requests/NodeStateRequest.java | 28 ----- .../restapiv2/requests/PartitionStateRequest.java | 5 +- .../restapiv2/requests/ServiceStateRequest.java | 2 + .../clustercontroller/core/restapiv2/NodeTest.java | 10 -- .../java/com/yahoo/vdslib/state/DiskState.java | 135 +++++++++++++++++++++ .../com/yahoo/vdslib/state/DiskStateTestCase.java | 116 ++++++++++++++++++ 9 files changed, 263 insertions(+), 45 deletions(-) create mode 100644 vdslib/src/main/java/com/yahoo/vdslib/state/DiskState.java create mode 100644 vdslib/src/test/java/com/yahoo/vdslib/state/DiskStateTestCase.java 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 f014690947d..6d97d9b2ce2 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,11 +6,7 @@ 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.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.vdslib.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 d956afba48a..ed5af93d7fb 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,6 +60,7 @@ 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 82512262b4a..17949b82365 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,6 +1,7 @@ // 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; @@ -31,6 +32,10 @@ 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 1404508e2fe..e007e2cc243 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,7 +3,6 @@ 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; @@ -33,34 +32,7 @@ public class NodeStateRequest extends Request { 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 b565bf0b572..bb42af45ef3 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,6 +1,7 @@ // 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; @@ -12,7 +13,6 @@ 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 { private static final Logger log = Logger.getLogger(PartitionStateRequest.class.getName()); private final Id.Partition id; @@ -32,7 +32,8 @@ public class PartitionStateRequest extends Request { fillInMetrics(context.cluster.getNodeInfo(id.getNode()).getHostInfo().getMetrics(), result); } NodeState nodeState = context.currentConsolidatedState.getNodeState(id.getNode()); - result.addState("generated", new Response.UnitStateImpl(nodeState)); + DiskState diskState = new DiskState(); + result.addState("generated", new Response.UnitStateImpl(diskState)); 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 b5452af79cc..9dd5ed7b071 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,6 +7,8 @@ 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 { 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 b00f46a3d00..67ae7911f80 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,11 +59,6 @@ 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)); @@ -89,11 +84,6 @@ 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 new file mode 100644 index 00000000000..1a05f8ab565 --- /dev/null +++ b/vdslib/src/main/java/com/yahoo/vdslib/state/DiskState.java @@ -0,0 +1,135 @@ +// 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 new file mode 100644 index 00000000000..bdc3be3dc70 --- /dev/null +++ b/vdslib/src/test/java/com/yahoo/vdslib/state/DiskStateTestCase.java @@ -0,0 +1,116 @@ +// 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()); + } + } +} -- cgit v1.2.3