diff options
author | Håkon Hallingstad <hakon@yahoo-inc.com> | 2017-05-21 02:08:02 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@yahoo-inc.com> | 2017-05-21 02:08:02 +0200 |
commit | c3726853545aa5669627e1a8978041f23e6ae00d (patch) | |
tree | 7be3d3261d569a80d4b4374a21e8324540daf454 /clustercontroller-core | |
parent | 3c12bfac3719341c6d2c78525e451ed321e884f3 (diff) |
Extract common check
Diffstat (limited to 'clustercontroller-core')
2 files changed, 31 insertions, 23 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 7f2d9ba9611..c9d9645735c 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 @@ -99,6 +99,11 @@ public class NodeStateChangeChecker { "Requested node type: " + node.getType().toString()); } + NodeInfo nodeInfo = clusterInfo.getStorageNodeInfo(node.getIndex()); + if (nodeInfo == null) { + return Result.createDisallowed("Unknown node " + node); + } + // If the new state and description equals the existing, we're done. This is done for 2 cases: // - We can short-circuit setting of a new wanted state, which e.g. hits ZooKeeper. // - We ensure that clients that have previously set the wanted state, continue @@ -110,32 +115,27 @@ public class NodeStateChangeChecker { switch (newState.getState()) { case UP: - return canSetStateUp(node, oldState.getState()); + return canSetStateUp(nodeInfo, oldState.getState()); case MAINTENANCE: return canSetStateMaintenanceTemporarily(node, clusterState); case DOWN: - return canSetStateDownPermanently(node, clusterState); + return canSetStateDownPermanently(nodeInfo, clusterState); default: - return Result.createDisallowed("Safe only supports state UP and MAINTENANCE, you tried: " + newState); + return Result.createDisallowed("Destination node state unsupported in safe mode: " + newState); } } - private Result canSetStateDownPermanently(Node node, ClusterState clusterState) { - StorageNodeInfo nodeInfo = clusterInfo.getStorageNodeInfo(node.getIndex()); - if (nodeInfo == null) { - return Result.createDisallowed("Unknown node " + node); - } - + private Result canSetStateDownPermanently(NodeInfo nodeInfo, ClusterState clusterState) { State reportedState = nodeInfo.getReportedState().getState(); if (reportedState != State.UP) { return Result.createDisallowed("Reported state (" + reportedState + ") is not UP, so no bucket data is available"); } - NodeState currentState = clusterState.getNodeState(node); - if (currentState.getState() != State.RETIRED) { + State currentState = clusterState.getNodeState(nodeInfo.getNode()).getState(); + if (currentState != State.RETIRED) { return Result.createDisallowed("Only retired nodes are allowed to be set to DOWN in safe mode - is " - + currentState.getState()); + + currentState); } Result thresholdCheckResult = checkUpThresholds(clusterState); @@ -148,14 +148,14 @@ public class NodeStateChangeChecker { int clusterControllerVersion = clusterState.getVersion(); if (hostInfoNodeVersion == null || hostInfoNodeVersion != clusterControllerVersion) { return Result.createDisallowed("Cluster controller at version " + clusterControllerVersion - + " got info for storage node " + node.getIndex() + " at a different version " + + " got info for storage node " + nodeInfo.getNodeIndex() + " at a different version " + hostInfoNodeVersion); } Optional<Metrics.Value> 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 " + node.getIndex()); + " metric for storage node " + nodeInfo.getNodeIndex()); } long lastBuckets = bucketsMetric.get().getLast(); @@ -166,27 +166,22 @@ public class NodeStateChangeChecker { return Result.allowSettingOfWantedState(); } - private Result canSetStateUp(Node node, State oldState) { + private Result canSetStateUp(NodeInfo nodeInfo, State oldState) { if (oldState != State.MAINTENANCE) { return Result.createDisallowed("Refusing to set wanted state to up when it is currently in " + oldState); } - if (clusterInfo.getNodeInfo(node).getReportedState().getState() != State.UP) { + if (nodeInfo.getReportedState().getState() != State.UP) { return Result.createDisallowed("Refuse to set wanted state to UP, " + "since the reported state is not UP (" + - clusterInfo.getNodeInfo(node).getReportedState().getState() + ")"); + nodeInfo.getReportedState().getState() + ")"); } return Result.allowSettingOfWantedState(); } private Result canSetStateMaintenanceTemporarily(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.DOWN) { + if (clusterState.getNodeState(node).getState() == State.DOWN) { return Result.allowSettingOfWantedState(); } 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 2f89a9d0c27..0d2f12dfdcd 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 @@ -133,6 +133,19 @@ public class NodeStateChangeCheckerTest { } @Test + public void testUnknownStorageNode() { + ContentCluster cluster = createCluster(createNodes(4)); + NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( + 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo()); + NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( + new Node(NodeType.STORAGE, 10), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + UP_NODE_STATE, MAINTENANCE_NODE_STATE); + assertFalse(result.settingWantedStateIsAllowed()); + assertFalse(result.wantedStateAlreadySet()); + assertThat(result.getReason(), is("Unknown node storage.10")); + } + + @Test public void testSafeSetStateDistributors() { NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(createCluster(createNodes(1))); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( |