diff options
author | Geir Storli <geirstorli@yahoo.no> | 2018-03-09 09:38:03 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-03-09 09:38:03 +0100 |
commit | 4ed263a1aef81aeefcd2a56433ba05dbaac1cc8c (patch) | |
tree | ed07f3d5d8789cdf1bd928d50f92fdb7a870bd9c /clustercontroller-core | |
parent | 7e3e0ba633bb086b87b63bc468fdc18dec748f1d (diff) | |
parent | 94c9b32a26dbf29025c91da2fcf30b0c2a76b823 (diff) |
Merge pull request #5263 from vespa-engine/vekterli/add-simple-maintenance-eligibility-checker
Add simple constraint implementation for implicit maintenance state transitions
Diffstat (limited to 'clustercontroller-core')
7 files changed, 148 insertions, 20 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterInfo.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterInfo.java index ff92fc6867f..b80004b02e7 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterInfo.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterInfo.java @@ -78,15 +78,17 @@ public class ClusterInfo { // Update node set nodes.clear(); - for (ConfiguredNode node : newNodes) + for (ConfiguredNode node : newNodes) { this.nodes.put(node.index(), node); + } } private void addNodeInfo(NodeInfo nodeInfo) { - if (nodeInfo instanceof DistributorNodeInfo) - distributorNodeInfo.put(nodeInfo.getNodeIndex(), (DistributorNodeInfo)nodeInfo); - else - storageNodeInfo.put(nodeInfo.getNodeIndex(), (StorageNodeInfo)nodeInfo); + if (nodeInfo instanceof DistributorNodeInfo) { + distributorNodeInfo.put(nodeInfo.getNodeIndex(), (DistributorNodeInfo) nodeInfo); + } else { + storageNodeInfo.put(nodeInfo.getNodeIndex(), (StorageNodeInfo) nodeInfo); + } allNodeInfo.put(nodeInfo.getNode(), nodeInfo); nodeInfo.setReportedState(nodeInfo.getReportedState().setDescription("Node not seen in slobrok."), 0); } @@ -108,8 +110,9 @@ public class ClusterInfo { */ public NodeInfo setRpcAddress(Node node, String rpcAddress) { NodeInfo nodeInfo = getInfo(node); - if (nodeInfo != null) + if (nodeInfo != null) { nodeInfo.setRpcAddress(rpcAddress); + } return nodeInfo; } // TODO: Do all mutation of node info through setters in this 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 e97690d1105..2edebd17700 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 @@ -824,7 +824,14 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd } private ClusterStateDeriver createBucketSpaceStateDeriver() { - return new MaintenanceWhenPendingGlobalMerges(stateVersionTracker.createMergePendingChecker()); + return new MaintenanceWhenPendingGlobalMerges(stateVersionTracker.createMergePendingChecker(), + createDefaultSpaceMaintenanceTransitionConstraint()); + } + + private MaintenanceTransitionConstraint createDefaultSpaceMaintenanceTransitionConstraint() { + AnnotatedClusterState currentDefaultSpaceState = stateVersionTracker.getVersionedClusterStateBundle() + .getDerivedBucketSpaceStates().getOrDefault(FixedBucketSpaces.defaultSpace(), AnnotatedClusterState.emptyState()); + return UpEdgeMaintenanceTransitionConstraint.forPreviouslyPublishedState(currentDefaultSpaceState.getClusterState()); } /** diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceTransitionConstraint.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceTransitionConstraint.java new file mode 100644 index 00000000000..4da6c272660 --- /dev/null +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceTransitionConstraint.java @@ -0,0 +1,15 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.clustercontroller.core; + +/** + * Implementations of this interface add constraints to when a node with + * pending global merges may be implicitly transitioned to Maintenance + * state in the default bucket space. This is to avoid flip-flopping nodes + * between being available and in maintenance when merge statistics + * change in a running system. + */ +public interface MaintenanceTransitionConstraint { + + boolean maintenanceTransitionAllowed(int contentNodeIndex); + +} diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMerges.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMerges.java index 3af789c41d4..22f9b7436cc 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMerges.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMerges.java @@ -8,6 +8,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; /** * Cluster state deriver which checks if nodes have merges pending for globally @@ -24,9 +25,12 @@ public class MaintenanceWhenPendingGlobalMerges implements ClusterStateDeriver { private static final String bucketSpaceToDerive = FixedBucketSpaces.defaultSpace(); private final MergePendingChecker mergePendingChecker; + private final MaintenanceTransitionConstraint maintenanceTransitionConstraint; - public MaintenanceWhenPendingGlobalMerges(MergePendingChecker mergePendingChecker) { + public MaintenanceWhenPendingGlobalMerges(MergePendingChecker mergePendingChecker, + MaintenanceTransitionConstraint maintenanceTransitionConstraint) { this.mergePendingChecker = mergePendingChecker; + this.maintenanceTransitionConstraint = maintenanceTransitionConstraint; } @Override @@ -34,25 +38,26 @@ public class MaintenanceWhenPendingGlobalMerges implements ClusterStateDeriver { if (!bucketSpace.equals(bucketSpaceToDerive)) { return baselineState.clone(); } - Set<Integer> incompleteNodeIndices = nodesWithMergesNotDone(baselineState.getClusterState()); + Set<Integer> incompleteNodeIndices = availableContentNodes(baselineState.getClusterState()).stream() + .filter(nodeIndex -> mayHaveMergesPending(bucketSpaceToCheck, nodeIndex)) + .filter(maintenanceTransitionConstraint::maintenanceTransitionAllowed) + .collect(Collectors.toSet()); + if (incompleteNodeIndices.isEmpty()) { return baselineState.clone(); } return setNodesInMaintenance(baselineState, incompleteNodeIndices); } - private Set<Integer> nodesWithMergesNotDone(ClusterState baselineState) { - final Set<Integer> incompleteNodes = new HashSet<>(); + private static Set<Integer> availableContentNodes(ClusterState baselineState) { + final Set<Integer> availableNodes = new HashSet<>(); final int nodeCount = baselineState.getNodeCount(NodeType.STORAGE); for (int nodeIndex = 0; nodeIndex < nodeCount; ++nodeIndex) { - // FIXME should only set nodes into maintenance if they've not yet been up in the cluster - // state since they came back as Reported state Up! - // Must be implemented before this state deriver is enabled in production. - if (contentNodeIsAvailable(baselineState, nodeIndex) && mayHaveMergesPending(bucketSpaceToCheck, nodeIndex)) { - incompleteNodes.add(nodeIndex); + if (contentNodeIsAvailable(baselineState, nodeIndex)) { + availableNodes.add(nodeIndex); } } - return incompleteNodes; + return availableNodes; } private AnnotatedClusterState setNodesInMaintenance(AnnotatedClusterState baselineState, @@ -69,7 +74,7 @@ public class MaintenanceWhenPendingGlobalMerges implements ClusterStateDeriver { nodeStateReasons); } - private boolean contentNodeIsAvailable(ClusterState state, int nodeIndex) { + private static boolean contentNodeIsAvailable(ClusterState state, int nodeIndex) { return state.getNodeState(Node.ofStorage(nodeIndex)).getState().oneOf("uir"); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/UpEdgeMaintenanceTransitionConstraint.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/UpEdgeMaintenanceTransitionConstraint.java new file mode 100644 index 00000000000..35cfb76f0ce --- /dev/null +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/UpEdgeMaintenanceTransitionConstraint.java @@ -0,0 +1,37 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.clustercontroller.core; + +import com.yahoo.vdslib.state.ClusterState; +import com.yahoo.vdslib.state.Node; +import com.yahoo.vdslib.state.NodeState; + +/** + * Simple implementation which considers a node eligible for being set to maintenance iff + * it was down or in maintenance in the cluster state previously published. This avoids + * taking down nodes with pending global merges if these have already passed through an + * up-edge, but runs the risk of taking down a large set of nodes when the cluster + * controller is restarted (due to not knowing the previously published state). + * + * Output from this checker must always be combined with other constraints to decide whether + * a node should be in maintenance; used alone it would most certainly cause havoc. + * + * Considered sufficient for beta testing use cases. + */ +public class UpEdgeMaintenanceTransitionConstraint implements MaintenanceTransitionConstraint { + + private final ClusterState previouslyPublishedDerivedState; + + private UpEdgeMaintenanceTransitionConstraint(ClusterState previouslyPublishedDerivedState) { + this.previouslyPublishedDerivedState = previouslyPublishedDerivedState; + } + + public static UpEdgeMaintenanceTransitionConstraint forPreviouslyPublishedState(ClusterState state) { + return new UpEdgeMaintenanceTransitionConstraint(state); + } + + @Override + public boolean maintenanceTransitionAllowed(int contentNodeIndex) { + NodeState prevState = previouslyPublishedDerivedState.getNodeState(Node.ofStorage(contentNodeIndex)); + return prevState.getState().oneOf("dm"); + } +} diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java index a2e661aa161..294c57d520c 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java @@ -19,8 +19,13 @@ import static org.mockito.Mockito.when; public class MaintenanceWhenPendingGlobalMergesTest { private static class Fixture { - public MergePendingChecker mockPendingChecker = mock(MergePendingChecker.class); - public MaintenanceWhenPendingGlobalMerges deriver = new MaintenanceWhenPendingGlobalMerges(mockPendingChecker); + MergePendingChecker mockPendingChecker = mock(MergePendingChecker.class); + MaintenanceTransitionConstraint mockTransitionConstraint = mock(MaintenanceTransitionConstraint.class); + MaintenanceWhenPendingGlobalMerges deriver = new MaintenanceWhenPendingGlobalMerges(mockPendingChecker, mockTransitionConstraint); + + Fixture() { + when(mockTransitionConstraint.maintenanceTransitionAllowed(anyInt())).thenReturn(true); + } } private static String defaultSpace() { @@ -102,4 +107,14 @@ public class MaintenanceWhenPendingGlobalMergesTest { .reason(NODE_TOO_UNSTABLE, 2).build())); } + @Test + public void node_with_pending_merges_only_set_to_maintenance_if_eligible() { + Fixture f = new Fixture(); + Arrays.asList(1, 2, 3).forEach(idx -> when(f.mockPendingChecker.mayHaveMergesPending(globalSpace(), idx)).thenReturn(true)); + Arrays.asList(1, 2, 4).forEach(idx -> when(f.mockTransitionConstraint.maintenanceTransitionAllowed(idx)).thenReturn(false)); + AnnotatedClusterState derived = f.deriver.derivedFrom(stateFromString("distributor:5 storage:5"), defaultSpace()); + assertThat(derived, equalTo(AnnotatedClusterStateBuilder.ofState("distributor:5 storage:5 .3.s:m") + .reason(MAY_HAVE_MERGES_PENDING, 3).build())); + } + } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/UpEdgeMaintenanceTransitionConstraintTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/UpEdgeMaintenanceTransitionConstraintTest.java new file mode 100644 index 00000000000..bbbcb654e57 --- /dev/null +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/UpEdgeMaintenanceTransitionConstraintTest.java @@ -0,0 +1,46 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.clustercontroller.core; + +import com.yahoo.vdslib.state.ClusterState; +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class UpEdgeMaintenanceTransitionConstraintTest { + + private static UpEdgeMaintenanceTransitionConstraint makeChecker(String state) { + return UpEdgeMaintenanceTransitionConstraint.forPreviouslyPublishedState(ClusterState.stateFromString(state)); + } + + private static boolean nodeMayTransitionToMaintenanceInState(int contentNodeIndex, String state) { + UpEdgeMaintenanceTransitionConstraint checker = makeChecker(state); + return checker.maintenanceTransitionAllowed(contentNodeIndex); + } + + @Test + public void transition_allowed_when_previous_state_is_down() { + assertTrue(nodeMayTransitionToMaintenanceInState(1, "distributor:5 storage:5 .1.s:d")); + } + + @Test + public void transition_allowed_when_previous_state_is_maintenance() { + assertTrue(nodeMayTransitionToMaintenanceInState(1, "distributor:5 storage:5 .1.s:m")); + } + + @Test + public void transition_not_allowed_when_previous_state_is_up() { + assertFalse(nodeMayTransitionToMaintenanceInState(0, "distributor:5 storage:5")); + } + + @Test + public void transition_not_allowed_when_previous_state_is_initializing() { + assertFalse(nodeMayTransitionToMaintenanceInState(0, "distributor:5 storage:5 .0.s:i")); + } + + @Test + public void transition_not_allowed_when_previous_state_is_retired() { + assertFalse(nodeMayTransitionToMaintenanceInState(0, "distributor:5 storage:5 .0.s:r")); + } + +} |