From c28f13dac59167dee1257f5b23835e6441bc5f31 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Fri, 17 Jan 2020 12:39:12 +0100 Subject: Use bucket_space metric in retirement This makes the Cluster Controller use the vds.datastored.bucket_space.buckets_total, dimension bucketSpace=default, to determine whether a content node manages zero buckets, and if so, will allow the node to go permanently down. This is used when a node is retiring, and it is to be removed from the application. The change is guarded by the use-bucket-space-metric, default true. If the new metric doesn't work as expected, we can revert to using the current/old metric by flipping the flag. The flag can be controlled per application. --- .../clustercontroller/core/ContentCluster.java | 8 +++- .../clustercontroller/core/FleetController.java | 2 +- .../core/FleetControllerOptions.java | 3 ++ .../core/NodeStateChangeChecker.java | 27 +++++++++--- .../clustercontroller/core/hostinfo/Metrics.java | 26 +++++++++++- .../clustercontroller/core/ClusterFixture.java | 4 +- .../core/FleetControllerTest.java | 6 +-- .../core/NodeStateChangeCheckerTest.java | 48 +++++++++++++++++++--- .../core/StateChangeHandlerTest.java | 2 +- .../clustercontroller/core/StateChangeTest.java | 2 +- .../core/hostinfo/HostInfoTest.java | 16 ++++++-- .../clustercontroller/core/restapiv2/NodeTest.java | 2 +- .../core/restapiv2/StateRestApiTest.java | 4 +- 13 files changed, 120 insertions(+), 30 deletions(-) (limited to 'clustercontroller-core/src') diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java index ddb9357f11f..ae12a6dabb1 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java @@ -22,6 +22,7 @@ public class ContentCluster { private final ClusterInfo clusterInfo = new ClusterInfo(); private final Map nodeStartTimestamps = new TreeMap<>(); + private final boolean determineBucketsFromBucketSpaceMetric; private int slobrokGenerationCount = 0; @@ -32,7 +33,9 @@ public class ContentCluster { private double minRatioOfStorageNodesUp; public ContentCluster(String clusterName, Collection configuredNodes, Distribution distribution, - int minStorageNodesUp, double minRatioOfStorageNodesUp) { + int minStorageNodesUp, double minRatioOfStorageNodesUp, + boolean determineBucketsFromBucketSpaceMetric) { + this.determineBucketsFromBucketSpaceMetric = determineBucketsFromBucketSpaceMetric; if (configuredNodes == null) throw new IllegalArgumentException("Nodes must be set"); this.clusterName = clusterName; this.distribution = distribution; @@ -183,7 +186,8 @@ public class ContentCluster { minStorageNodesUp, minRatioOfStorageNodesUp, distribution.getRedundancy(), - clusterInfo); + clusterInfo, + determineBucketsFromBucketSpaceMetric); return nodeStateChangeChecker.evaluateTransition(node, clusterState, condition, oldState, newState); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java index d943cf27f9c..02f52d5f0c7 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java @@ -193,7 +193,7 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd options.nodes, options.storageDistribution, options.minStorageNodesUp, - options.minRatioOfStorageNodesUp); + options.minRatioOfStorageNodesUp, true); NodeStateGatherer stateGatherer = new NodeStateGatherer(timer, timer, log); Communicator communicator = new RPCCommunicator( RPCCommunicator.createRealSupervisor(), diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java index 553b3332ee8..7ad6765cc47 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java @@ -128,6 +128,9 @@ public class FleetControllerOptions implements Cloneable { public int maxDivergentNodesPrintedInTaskErrorMessages = 10; + // TODO: May be removed once rolled out everywhere. + public boolean determineBucketsFromBucketSpaceMetric = true; + // TODO: Replace usage of this by usage where the nodes are explicitly passed (below) public FleetControllerOptions(String clusterName) { this.clusterName = clusterName; diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java index c31a4976827..6bcb5b07f28 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java @@ -12,6 +12,7 @@ import com.yahoo.vespa.clustercontroller.core.hostinfo.StorageNode; import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest; import java.util.List; +import java.util.Map; import java.util.Optional; /** @@ -20,22 +21,27 @@ import java.util.Optional; * @author Haakon Dybdahl */ public class NodeStateChangeChecker { - public static final String BUCKETS_METRIC_NAME = "vds.datastored.alldisks.buckets"; + public static final String LEGACY_BUCKETS_METRIC_NAME = "vds.datastored.alldisks.buckets"; + public static final String BUCKETS_METRIC_NAME = "vds.datastored.bucket_space.buckets_total"; + public static final Map BUCKETS_METRIC_DIMENSIONS = Map.of("bucketSpace", "default"); private final int minStorageNodesUp; private double minRatioOfStorageNodesUp; private final int requiredRedundancy; private final ClusterInfo clusterInfo; + private final boolean determineBucketsFromBucketSpaceMetric; public NodeStateChangeChecker( int minStorageNodesUp, double minRatioOfStorageNodesUp, int requiredRedundancy, - ClusterInfo clusterInfo) { + ClusterInfo clusterInfo, + boolean determineBucketsFromBucketSpaceMetric) { this.minStorageNodesUp = minStorageNodesUp; this.minRatioOfStorageNodesUp = minRatioOfStorageNodesUp; this.requiredRedundancy = requiredRedundancy; this.clusterInfo = clusterInfo; + this.determineBucketsFromBucketSpaceMetric = determineBucketsFromBucketSpaceMetric; } public static class Result { @@ -152,10 +158,19 @@ public class NodeStateChangeChecker { + hostInfoNodeVersion); } - Optional bucketsMetric = hostInfo.getMetrics().getValue(BUCKETS_METRIC_NAME); - if (!bucketsMetric.isPresent() || bucketsMetric.get().getLast() == null) { - return Result.createDisallowed("Missing last value of the " + BUCKETS_METRIC_NAME + - " metric for storage node " + nodeInfo.getNodeIndex()); + Optional bucketsMetric; + if (determineBucketsFromBucketSpaceMetric) { + bucketsMetric = hostInfo.getMetrics().getValueAt(BUCKETS_METRIC_NAME, BUCKETS_METRIC_DIMENSIONS); + if (!bucketsMetric.isPresent() || bucketsMetric.get().getLast() == null) { + return Result.createDisallowed("Missing last value of the " + BUCKETS_METRIC_NAME + + " metric for storage node " + nodeInfo.getNodeIndex()); + } + } else { + bucketsMetric = hostInfo.getMetrics().getValue(LEGACY_BUCKETS_METRIC_NAME); + if (!bucketsMetric.isPresent() || bucketsMetric.get().getLast() == null) { + return Result.createDisallowed("Missing last value of the " + LEGACY_BUCKETS_METRIC_NAME + + " metric for storage node " + nodeInfo.getNodeIndex()); + } } long lastBuckets = bucketsMetric.get().getLast(); diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/Metrics.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/Metrics.java index cb2a1e92612..eef3fd2e217 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/Metrics.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/Metrics.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Optional; /** @@ -25,21 +26,44 @@ public class Metrics { return Optional.empty(); } + /** + * Get the metric value whose dimensions MUST MATCH the given dimensions map. + * To require the metric to NOT have a dimension key, set it's value to null. + */ + public Optional getValueAt(String name, Map dimensions) { + return metricsList.stream() + .filter(metric -> metric.name.equals(name)) + .filter(metric -> dimensions.entrySet().stream() + .allMatch(entry -> { + String dimensionName = entry.getKey(); + Optional requiredDimensionValue = Optional.ofNullable(entry.getValue()); + return metric.getDimensionValue(dimensionName).equals(requiredDimensionValue); + })) + .map(Metric::getValue) + .findFirst(); + } + public List getMetrics() { return Collections.unmodifiableList(metricsList); } public static class Metric { private final String name; private final Value value; + private final Map dimensions; public Metric( @JsonProperty("name") String name, - @JsonProperty("values") Value value) { + @JsonProperty("values") Value value, + @JsonProperty("dimensions") Map dimensions) { this.name = name; this.value = value; + this.dimensions = dimensions; } public String getName() { return name; } public Value getValue() { return value; } + public Optional getDimensionValue(String dimension) { + return Optional.ofNullable(dimensions.get(dimension)); + } } public static class Value { diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java index 2df9279e450..36c49fdf5e2 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java @@ -198,7 +198,7 @@ public class ClusterFixture { Collection nodes = DistributionBuilder.buildConfiguredNodes(nodeCount); Distribution distribution = DistributionBuilder.forFlatCluster(nodeCount); - ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0); + ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0, true); return new ClusterFixture(cluster, distribution); } @@ -206,7 +206,7 @@ public class ClusterFixture { static ClusterFixture forHierarchicCluster(DistributionBuilder.GroupBuilder root) { List nodes = DistributionBuilder.buildConfiguredNodes(root.totalNodeCount()); Distribution distribution = DistributionBuilder.forHierarchicCluster(root); - ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0); + ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0, true); return new ClusterFixture(cluster, distribution); } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java index ae64bee6bbf..d569feb6f14 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java @@ -34,8 +34,6 @@ import org.junit.rules.TestRule; import org.junit.rules.TestWatcher; import org.junit.runner.Description; -import static org.junit.Assert.fail; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -49,6 +47,8 @@ import java.util.logging.Logger; import java.util.regex.Pattern; import java.util.stream.Collectors; +import static org.junit.Assert.fail; + /** * @author Håkon Humberset */ @@ -157,7 +157,7 @@ public abstract class FleetControllerTest implements Waiter { options.nodes, options.storageDistribution, options.minStorageNodesUp, - options.minRatioOfStorageNodesUp); + options.minRatioOfStorageNodesUp, true); NodeStateGatherer stateGatherer = new NodeStateGatherer(timer, timer, log); Communicator communicator = new RPCCommunicator( RPCCommunicator.createRealSupervisor(), diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java index 303376e7a5e..153d570adaf 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java @@ -61,14 +61,14 @@ public class NodeStateChangeCheckerTest { } private NodeStateChangeChecker createChangeChecker(ContentCluster cluster) { - return new NodeStateChangeChecker(minStorageNodesUp, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo()); + return new NodeStateChangeChecker(minStorageNodesUp, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo(), true); } private ContentCluster createCluster(Collection nodes) { Distribution distribution = mock(Distribution.class); Group group = new Group(2, "to"); when(distribution.getRootGroup()).thenReturn(group); - return new ContentCluster("Clustername", nodes, distribution, minStorageNodesUp, 0.0); + return new ContentCluster("Clustername", nodes, distribution, minStorageNodesUp, 0.0, true); } private StorageNodeInfo createStorageNodeInfo(int index, State state) { @@ -78,7 +78,7 @@ public class NodeStateChangeCheckerTest { String clusterName = "Clustername"; Set configuredNodeIndexes = new HashSet<>(); - ContentCluster cluster = new ContentCluster(clusterName, configuredNodeIndexes, distribution, minStorageNodesUp, 0.0); + ContentCluster cluster = new ContentCluster(clusterName, configuredNodeIndexes, distribution, minStorageNodesUp, 0.0, true); String rpcAddress = ""; StorageNodeInfo storageNodeInfo = new StorageNodeInfo(cluster, index, false, rpcAddress, distribution); @@ -136,7 +136,7 @@ public class NodeStateChangeCheckerTest { public void testUnknownStorageNode() { ContentCluster cluster = createCluster(createNodes(4)); NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( - 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo()); + 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo(), true); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( new Node(NodeType.STORAGE, 10), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); @@ -161,7 +161,7 @@ public class NodeStateChangeCheckerTest { ContentCluster cluster = createCluster(createNodes(4)); setAllNodesUp(cluster, HostInfo.createHostInfo(createDistributorHostInfo(4, 5, 6))); NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( - 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo()); + 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo(), true); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); @@ -549,12 +549,48 @@ public class NodeStateChangeCheckerTest { " \"dimensions\":\n" + " {\n" + " }\n" + + " },\n" + + " {\n" + + " \"name\":\"vds.datastored.bucket_space.buckets_total\",\n" + + " \"description\":\"Total number buckets present in the bucket space (ready + not ready)\",\n" + + " \"values\":\n" + + " {\n" + + " \"average\":0.0,\n" + + " \"sum\":0.0,\n" + + " \"count\":1,\n" + + " \"rate\":0.016666,\n" + + " \"min\":0,\n" + + " \"max\":0,\n" + + " \"last\":0\n" + + " },\n" + + " \"dimensions\":\n" + + " {\n" + + " \"bucketSpace\":\"global\"\n" + + " }\n" + + " },\n" + + " {\n" + + " \"name\":\"vds.datastored.bucket_space.buckets_total\",\n" + + " \"description\":\"Total number buckets present in the bucket space (ready + not ready)\",\n" + + " \"values\":\n" + + " {\n" + + " \"average\":129.0,\n" + + " \"sum\":129.0,\n" + + " \"count\":1,\n" + + " \"rate\":0.016666,\n" + + " \"min\":129,\n" + + " \"max\":129,\n" + + " \"last\":%d\n" + + " },\n" + + " \"dimensions\":\n" + + " {\n" + + " \"bucketSpace\":\"default\"\n" + + " }\n" + " }\n" + " ]\n" + " },\n" + " \"cluster-state-version\":%d\n" + "}", - lastAlldisksBuckets, clusterStateVersion)); + lastAlldisksBuckets, lastAlldisksBuckets, clusterStateVersion)); } private List createNodes(int count) { diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java index 68926b4d10d..dc76b381f51 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java @@ -80,7 +80,7 @@ public class StateChangeHandlerTest { Distribution distribution = new Distribution(Distribution.getDefaultDistributionConfig(2, 100)); this.config = config; for (int i=0; i metrics = hostInfo.getMetrics().getMetrics(); - assertThat(metrics.size(), is(2)); - Metrics.Value value = metrics.get(0).getValue(); - assertThat(value.getLast(), is(5095L)); + assertThat(metrics.size(), is(4)); + assertThat(metrics.get(0).getValue().getLast(), is(5095L)); assertThat(metrics.get(0).getName(), equalTo("vds.datastored.alldisks.buckets")); + assertThat(metrics.get(3).getValue().getLast(), is(129L)); + assertThat(metrics.get(3).getName(), equalTo("vds.datastored.bucket_space.buckets_total")); assertThat(hostInfo.getClusterStateVersionOrNull(), is(123)); + + Optional value = hostInfo.getMetrics() + .getValueAt("vds.datastored.bucket_space.buckets_total", Map.of("bucketSpace", "default")); + assertThat(value.map(Metrics.Value::getLast), equalTo(Optional.of(129L))); } @Test 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 6bd7f086249..368f64352d1 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 @@ -121,7 +121,7 @@ public class NodeTest extends StateRestApiTest { public void testNodeNotSeenInSlobrok() throws Exception { setUp(true); ContentCluster old = music.context.cluster; - music.context.cluster = new ContentCluster(old.getName(), old.getConfiguredNodes().values(), old.getDistribution(), 0, 0.0); + music.context.cluster = new ContentCluster(old.getName(), old.getConfiguredNodes().values(), old.getDistribution(), 0, 0.0, true); NodeState currentState = new NodeState(NodeType.STORAGE, State.DOWN); currentState.setDescription("Not seen"); music.context.currentConsolidatedState.setNodeState(new Node(NodeType.STORAGE, 1), currentState); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java index faebbf8755d..44dcd50ae88 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java @@ -42,7 +42,7 @@ public abstract class StateRestApiTest { { Set nodes = FleetControllerTest.toNodes(0, 1, 2, 3); ContentCluster cluster = new ContentCluster( - "books", nodes, distribution, 6 /* minStorageNodesUp*/, 0.9 /* minRatioOfStorageNodesUp */); + "books", nodes, distribution, 6 /* minStorageNodesUp*/, 0.9, /* minRatioOfStorageNodesUp */true); initializeCluster(cluster, nodes); AnnotatedClusterState baselineState = AnnotatedClusterState.withoutAnnotations(ClusterState.stateFromString("distributor:4 storage:4")); Map bucketSpaceStates = new HashMap<>(); @@ -56,7 +56,7 @@ public abstract class StateRestApiTest { Set nodesInSlobrok = FleetControllerTest.toNodes(1, 3, 5, 7); ContentCluster cluster = new ContentCluster( - "music", nodes, distribution, 4 /* minStorageNodesUp*/, 0.0 /* minRatioOfStorageNodesUp */); + "music", nodes, distribution, 4 /* minStorageNodesUp*/, 0.0, /* minRatioOfStorageNodesUp */true); if (dontInitializeNode2) { // TODO: this skips initialization of node 2 to fake that it is not answering // which really leaves us in an illegal state -- cgit v1.2.3