diff options
author | Håkon Hallingstad <hakon@yahoo-inc.com> | 2017-05-18 16:28:49 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@yahoo-inc.com> | 2017-05-18 16:28:49 +0200 |
commit | 414a52d969971841abdb4b93e1e955e3266414ec (patch) | |
tree | f5fc9668ace65131b212834be27a9786f503e53e /clustercontroller-core | |
parent | e098893cb83228997b4ebe71a734e3a916cdb92e (diff) |
Safely set storage node to DOWN
Setting a storage node to DOWN is considered safe if it can be permantenly set
down (e.g. removed from the application):
- The node is RETIRED
- There are no managed buckets
Diffstat (limited to 'clustercontroller-core')
5 files changed, 188 insertions, 33 deletions
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 df0d2f78c61..1f2897a1453 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 @@ -6,10 +6,12 @@ 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.Metrics; import com.yahoo.vespa.clustercontroller.core.hostinfo.StorageNode; import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest; import java.util.List; +import java.util.Optional; /** * Checks if a node can be upgraded. @@ -17,6 +19,7 @@ import java.util.List; * @author dybis */ public class NodeStateChangeChecker { + public static final String BUCKETS_METRIC_NAME = "vds.datastored.alldisks.buckets"; private final int minStorageNodesUp; private double minRatioOfStorageNodesUp; @@ -108,12 +111,46 @@ public class NodeStateChangeChecker { case UP: return canSetStateUp(node, oldState.getState()); case MAINTENANCE: - return canSetStateMaintenance(node, clusterState); + return canSetStateMaintenanceTemporarily(node, clusterState); + case DOWN: + return canSetStateDownPermanently(node, clusterState); default: return Result.createDisallowed("Safe only supports state UP and MAINTENANCE, you tried: " + newState); } } + private Result canSetStateDownPermanently(Node node, ClusterState clusterState) { + NodeInfo nodeInfo = clusterInfo.getNodeInfo(node); + if (nodeInfo == null) { + return Result.createDisallowed("Unknown node " + node); + } + + NodeState currentState = clusterState.getNodeState(node); + if (currentState.getState() != State.RETIRED) { + return Result.createDisallowed("Only retired nodes are allowed to be set to DOWN in safe mode - is " + + currentState.getState()); + } + + Result thresholdCheckResult = checkUpThresholds(clusterState); + if (!thresholdCheckResult.settingWantedStateIsAllowed()) { + return thresholdCheckResult; + } + + Optional<Metrics.Value> bucketsMetric = clusterInfo.getStorageNodeInfo(node.getIndex()) + .getHostInfo().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 " + node.getIndex()); + } + + long lastBuckets = bucketsMetric.get().getLast(); + if (lastBuckets > 0) { + return Result.createDisallowed("The storage node manages " + lastBuckets + " buckets"); + } + + return Result.allowSettingOfWantedState(); + } + private Result canSetStateUp(Node node, State oldState) { if (oldState != State.MAINTENANCE) { return Result.createDisallowed("Refusing to set wanted state to up when it is currently in " + oldState); @@ -128,7 +165,7 @@ public class NodeStateChangeChecker { return Result.allowSettingOfWantedState(); } - private Result canSetStateMaintenance(Node node, ClusterState clusterState) { + private Result canSetStateMaintenanceTemporarily(Node node, ClusterState clusterState) { NodeInfo nodeInfo = clusterInfo.getNodeInfo(node); if (nodeInfo == null) { return Result.createDisallowed("Unknown node " + node); @@ -148,13 +185,9 @@ public class NodeStateChangeChecker { return ongoingChanges; } - if (clusterInfo.getStorageNodeInfo().size() < minStorageNodesUp) { - return Result.createDisallowed("There are only " + clusterInfo.getStorageNodeInfo().size() + - " storage nodes up, while config requires at least " + minStorageNodesUp); - } - Result fractionCheck = isFractionHighEnough(clusterState); - if (!fractionCheck.settingWantedStateIsAllowed()) { - return fractionCheck; + Result thresholdCheckResult = checkUpThresholds(clusterState); + if (!thresholdCheckResult.settingWantedStateIsAllowed()) { + return thresholdCheckResult; } return Result.allowSettingOfWantedState(); @@ -182,17 +215,22 @@ public class NodeStateChangeChecker { return upNodesCount; } - private Result isFractionHighEnough(ClusterState clusterState) { + private Result checkUpThresholds(ClusterState clusterState) { + if (clusterInfo.getStorageNodeInfo().size() < minStorageNodesUp) { + return Result.createDisallowed("There are only " + clusterInfo.getStorageNodeInfo().size() + + " storage nodes up, while config requires at least " + minStorageNodesUp); + } + final int nodesCount = clusterInfo.getStorageNodeInfo().size(); final int upNodesCount = contentNodesWithAvailableNodeState(clusterState); if (nodesCount == 0) { - return Result.createDisallowed("No storage nodes in cluster state, not safe to restart."); + return Result.createDisallowed("No storage nodes in cluster state"); } if (((double)upNodesCount) / nodesCount < minRatioOfStorageNodesUp) { - return Result.createDisallowed("Not enough storage nodes running, running: " + upNodesCount - + " total storage nodes " + nodesCount + - " required fraction " + minRatioOfStorageNodesUp); + return Result.createDisallowed("Not enough storage nodes running: " + upNodesCount + + " of " + nodesCount + " storage nodes are up which is less that the required fraction of " + + minRatioOfStorageNodesUp); } return Result.allowSettingOfWantedState(); } 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 b2924516f26..4770bb40545 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.Optional; /** * Keeper for Metrics for HostInfo. @@ -13,7 +14,17 @@ import java.util.List; */ public class Metrics { - public List<Metric> getValues() { return Collections.unmodifiableList(metricsList); } + public Optional<Value> getValue(String name) { + for (Metric metric : metricsList) { + if (name.equals(metric.getName())) { + return Optional.ofNullable(metric.getValue()); + } + } + + return Optional.empty(); + } + + public List<Metric> getMetrics() { return Collections.unmodifiableList(metricsList); } public static class Metric { private final String name; 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 8958bce8ccd..308b189841d 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 @@ -39,7 +39,7 @@ public class PartitionStateRequest extends Request<Response.PartitionResponse> { } private static void fillInMetrics(Metrics metrics, Response.PartitionResponse result) { - for (Metrics.Metric metric: metrics.getValues()) { + for (Metrics.Metric metric: metrics.getMetrics()) { fillInMetricValue(metric.getName(), metric.getValue(), result); } } 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 5b53e524102..37c241e7bc6 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 @@ -36,8 +36,9 @@ public class NodeStateChangeCheckerTest { private static final Node nodeDistributor = new Node(NodeType.DISTRIBUTOR, 1); private static final Node nodeStorage = new Node(NodeType.STORAGE, 1); - private static final NodeState upNodeState = new NodeState(NodeType.STORAGE, State.UP); - public static final NodeState maintenanceNodeState = createNodeState(State.MAINTENANCE, "Orchestrator"); + private static final NodeState UP_NODE_STATE = new NodeState(NodeType.STORAGE, State.UP); + public static final NodeState MAINTENANCE_NODE_STATE = createNodeState(State.MAINTENANCE, "Orchestrator"); + public static final NodeState DOWN_NODE_STATE = createNodeState(State.DOWN, "RetireEarlyExpirer"); private static NodeState createNodeState(State state, String description) { return new NodeState(NodeType.STORAGE, state).setDescription(description); @@ -122,7 +123,7 @@ public class NodeStateChangeCheckerTest { NodeState newState = new NodeState(NodeType.STORAGE, State.INITIALIZING); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeDistributor, defaultAllUpClusterState(), SetUnitStateRequest.Condition.FORCE, - upNodeState, newState); + UP_NODE_STATE, newState); assertTrue(result.settingWantedStateIsAllowed()); assertTrue(!result.wantedStateAlreadySet()); } @@ -132,7 +133,7 @@ public class NodeStateChangeCheckerTest { NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(createCluster(createNodes(1))); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeDistributor, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, - upNodeState, maintenanceNodeState); + UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertThat(result.getReason(), containsString("Safe-set of node state is only supported for storage nodes")); @@ -146,7 +147,7 @@ public class NodeStateChangeCheckerTest { 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo()); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, - upNodeState, maintenanceNodeState); + UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertThat(result.getReason(), is("There are only 4 storage nodes up, while config requires at least 5")); @@ -167,7 +168,7 @@ public class NodeStateChangeCheckerTest { NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, - maintenanceNodeState, upNodeState); + MAINTENANCE_NODE_STATE, UP_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } @@ -187,7 +188,7 @@ public class NodeStateChangeCheckerTest { NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, stateWithNodeDown, SetUnitStateRequest.Condition.SAFE, - maintenanceNodeState, upNodeState); + MAINTENANCE_NODE_STATE, UP_NODE_STATE); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } @@ -200,7 +201,7 @@ public class NodeStateChangeCheckerTest { NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, - new NodeState(NodeType.STORAGE, State.DOWN), upNodeState); + new NodeState(NodeType.STORAGE, State.DOWN), UP_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertThat(result.getReason(), is("Refusing to set wanted state to up when it is currently in Down")); @@ -214,7 +215,7 @@ public class NodeStateChangeCheckerTest { NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, - upNodeState, maintenanceNodeState); + UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertThat(result.getReason(), is("Distributor 0 says storage node 1 " + @@ -229,7 +230,7 @@ public class NodeStateChangeCheckerTest { NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( new Node(NodeType.STORAGE, 3), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, - upNodeState, maintenanceNodeState); + UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } @@ -253,7 +254,7 @@ public class NodeStateChangeCheckerTest { NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( new Node(NodeType.STORAGE, 1), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, - upNodeState, maintenanceNodeState); + UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } @@ -265,7 +266,7 @@ public class NodeStateChangeCheckerTest { cluster.clusterInfo().getStorageNodeInfo(1).setReportedState(new NodeState(NodeType.STORAGE, State.UP), 0); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, upNodeState, maintenanceNodeState); + nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertThat(result.getReason(), is("Distributor node (0) has not reported any cluster state version yet.")); @@ -332,7 +333,7 @@ public class NodeStateChangeCheckerTest { } return nodeStateChangeChecker.evaluateTransition( - nodeStorage, clusterState, SetUnitStateRequest.Condition.SAFE, upNodeState, maintenanceNodeState); + nodeStorage, clusterState, SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); } private void setAllNodesUp(ContentCluster cluster, HostInfo distributorHostInfo) { @@ -402,11 +403,116 @@ public class NodeStateChangeCheckerTest { NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, stateWithNodeDown, SetUnitStateRequest.Condition.SAFE, - upNodeState, maintenanceNodeState); + UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } + @Test + public void testDisallowedByNonRetiredState() { + ContentCluster cluster = createCluster(createNodes(4)); + NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); + + cluster.clusterInfo().getStorageNodeInfo(nodeStorage.getIndex()) + .setHostInfo(createHostInfoWithMetrics(currentClusterStateVersion, 1)); + + NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( + nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + UP_NODE_STATE, DOWN_NODE_STATE); + assertFalse(result.settingWantedStateIsAllowed()); + assertFalse(result.wantedStateAlreadySet()); + assertEquals("Only retired nodes are allowed to be set to DOWN in safe mode - is Up", result.getReason()); + } + + @Test + public void testDisallowedByBuckets() { + ContentCluster cluster = createCluster(createNodes(4)); + NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); + + ClusterState stateWithRetiredNode = clusterState(String.format( + "version:%d distributor:4 storage:4 .%d.s:r", + currentClusterStateVersion, nodeStorage.getIndex())); + + cluster.clusterInfo().getStorageNodeInfo(nodeStorage.getIndex()) + .setHostInfo(createHostInfoWithMetrics(currentClusterStateVersion, 1)); + + NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( + nodeStorage, stateWithRetiredNode, SetUnitStateRequest.Condition.SAFE, + UP_NODE_STATE, DOWN_NODE_STATE); + assertFalse(result.settingWantedStateIsAllowed()); + assertFalse(result.wantedStateAlreadySet()); + assertEquals("The storage node manages 1 buckets", result.getReason()); + } + + @Test + public void testAllowedToSetDown() { + ContentCluster cluster = createCluster(createNodes(4)); + NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); + + ClusterState stateWithRetiredNode = clusterState(String.format( + "version:%d distributor:4 storage:4 .%d.s:r", + currentClusterStateVersion, nodeStorage.getIndex())); + + cluster.clusterInfo().getStorageNodeInfo(nodeStorage.getIndex()) + .setHostInfo(createHostInfoWithMetrics(currentClusterStateVersion, 0)); + + NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( + nodeStorage, stateWithRetiredNode, SetUnitStateRequest.Condition.SAFE, + UP_NODE_STATE, DOWN_NODE_STATE); + assertTrue(result.settingWantedStateIsAllowed()); + assertFalse(result.wantedStateAlreadySet()); + } + + private static HostInfo createHostInfoWithMetrics(int clusterStateVersion, int lastAlldisksBuckets) { + return HostInfo.createHostInfo(String.format("{\n" + + " \"metrics\":\n" + + " {\n" + + " \"snapshot\":\n" + + " {\n" + + " \"from\":1494940706,\n" + + " \"to\":1494940766\n" + + " },\n" + + " \"values\":\n" + + " [\n" + + " {\n" + + " \"name\":\"vds.datastored.alldisks.buckets\",\n" + + " \"description\":\"buckets managed\",\n" + + " \"values\":\n" + + " {\n" + + " \"average\":262144.0,\n" + + " \"count\":1,\n" + + " \"rate\":0.016666,\n" + + " \"min\":262144,\n" + + " \"max\":262144,\n" + + " \"last\":%d\n" + + " },\n" + + " \"dimensions\":\n" + + " {\n" + + " }\n" + + " },\n" + + " {\n" + + " \"name\":\"vds.datastored.alldisks.docs\",\n" + + " \"description\":\"documents stored\",\n" + + " \"values\":\n" + + " {\n" + + " \"average\":154689587.0,\n" + + " \"count\":1,\n" + + " \"rate\":0.016666,\n" + + " \"min\":154689587,\n" + + " \"max\":154689587,\n" + + " \"last\":154689587\n" + + " },\n" + + " \"dimensions\":\n" + + " {\n" + + " }\n" + + " }\n" + + " ]\n" + + " },\n" + + " \"cluster-state-version\":%d\n" + + "}", + lastAlldisksBuckets, clusterStateVersion)); + } + private List<ConfiguredNode> createNodes(int count) { List<ConfiguredNode> nodes = new ArrayList<>(); for (int i = 0; i < count; i++) 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 d24b45817e0..fd93df4b2dc 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 @@ -32,7 +32,7 @@ public class HostInfoTest { HostInfo hostInfo = HostInfo.createHostInfo("{}"); assertThat(hostInfo.getVtag().getVersionOrNull(), is(nullValue())); assertThat(hostInfo.getDistributor().getStorageNodes().size(), is(0)); - assertThat(hostInfo.getMetrics().getValues().size(), is(0)); + assertThat(hostInfo.getMetrics().getMetrics().size(), is(0)); assertThat(hostInfo.getClusterStateVersionOrNull(), is(nullValue())); } @@ -51,7 +51,7 @@ public class HostInfoTest { assertThat(storageNodeList.get(0).getOpsLatenciesOrNull().getPut().getCount(), is(16L)); assertThat(storageNodeList.get(1).getOpsLatenciesOrNull().getPut().getCount(), is(18L)); assertThat(storageNodeList.get(0).getOpsLatenciesOrNull().getPut().getLatencyMsSum(), is(15L)); - List<Metrics.Metric> metrics = hostInfo.getMetrics().getValues(); + List<Metrics.Metric> metrics = hostInfo.getMetrics().getMetrics(); assertThat(metrics.size(), is(2)); Metrics.Value value = metrics.get(0).getValue(); assertThat(value.getLast(), is(5095L)); @@ -70,7 +70,7 @@ public class HostInfoTest { } HostInfo hostInfo = HostInfo.createHostInfo(json); // Check a value so not all code is removed by optimizer. - if (hostInfo.getMetrics().getValues().size() == -1) return; + if (hostInfo.getMetrics().getMetrics().size() == -1) return; } long end = System.currentTimeMillis(); System.out.println("Should take about 1.5 ms on fast machine, actually " + (end - start) / 10. + " ms."); |