From 7b99c625f39d334638b13f5a083c680e4ee3018b Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Sun, 26 Jan 2020 00:46:51 +0100 Subject: Remove use-bucket-space-metric feature flag The flag controlled config read by the Cluster Controller. Therefore, I have left the ModelContextImpl.Properties method and implementation (now always returning true), but the model has stopped using that method internally, and the config is no longer used in the CC. The field in the fleetcontroller.def is left unchanged and documented as deprecated. --- .../clustercontroller/core/ContentCluster.java | 9 +++------ .../clustercontroller/core/FleetController.java | 2 +- .../core/FleetControllerOptions.java | 3 --- .../core/NodeStateChangeChecker.java | 22 +++++----------------- .../clustercontroller/core/ClusterFixture.java | 4 ++-- .../core/FleetControllerTest.java | 2 +- .../core/NodeStateChangeCheckerTest.java | 10 +++++----- .../core/StateChangeHandlerTest.java | 2 +- .../clustercontroller/core/StateChangeTest.java | 2 +- .../clustercontroller/core/restapiv2/NodeTest.java | 2 +- .../core/restapiv2/StateRestApiTest.java | 8 ++++---- 11 files changed, 24 insertions(+), 42 deletions(-) (limited to 'clustercontroller-core') 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 ae12a6dabb1..5e775116bf3 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,7 +22,6 @@ public class ContentCluster { private final ClusterInfo clusterInfo = new ClusterInfo(); private final Map nodeStartTimestamps = new TreeMap<>(); - private final boolean determineBucketsFromBucketSpaceMetric; private int slobrokGenerationCount = 0; @@ -33,9 +32,7 @@ public class ContentCluster { private double minRatioOfStorageNodesUp; public ContentCluster(String clusterName, Collection configuredNodes, Distribution distribution, - int minStorageNodesUp, double minRatioOfStorageNodesUp, - boolean determineBucketsFromBucketSpaceMetric) { - this.determineBucketsFromBucketSpaceMetric = determineBucketsFromBucketSpaceMetric; + int minStorageNodesUp, double minRatioOfStorageNodesUp) { if (configuredNodes == null) throw new IllegalArgumentException("Nodes must be set"); this.clusterName = clusterName; this.distribution = distribution; @@ -186,8 +183,8 @@ public class ContentCluster { minStorageNodesUp, minRatioOfStorageNodesUp, distribution.getRedundancy(), - clusterInfo, - determineBucketsFromBucketSpaceMetric); + clusterInfo + ); 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 02f52d5f0c7..d943cf27f9c 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, true); + options.minRatioOfStorageNodesUp); 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 7ad6765cc47..553b3332ee8 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,9 +128,6 @@ 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 6bcb5b07f28..4ae45701344 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 @@ -21,7 +21,6 @@ import java.util.Optional; * @author Haakon Dybdahl */ public class NodeStateChangeChecker { - 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"); @@ -29,19 +28,16 @@ public class NodeStateChangeChecker { 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, - boolean determineBucketsFromBucketSpaceMetric) { + ClusterInfo clusterInfo) { this.minStorageNodesUp = minStorageNodesUp; this.minRatioOfStorageNodesUp = minRatioOfStorageNodesUp; this.requiredRedundancy = requiredRedundancy; this.clusterInfo = clusterInfo; - this.determineBucketsFromBucketSpaceMetric = determineBucketsFromBucketSpaceMetric; } public static class Result { @@ -159,18 +155,10 @@ public class NodeStateChangeChecker { } 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()); - } + 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()); } long lastBuckets = bucketsMetric.get().getLast(); 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 36c49fdf5e2..2df9279e450 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, true); + ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0); 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, true); + ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0); 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 d569feb6f14..059d7d68754 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 @@ -157,7 +157,7 @@ public abstract class FleetControllerTest implements Waiter { options.nodes, options.storageDistribution, options.minStorageNodesUp, - options.minRatioOfStorageNodesUp, true); + options.minRatioOfStorageNodesUp); 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 153d570adaf..e918f94abbf 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(), true); + return new NodeStateChangeChecker(minStorageNodesUp, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo()); } 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, true); + return new ContentCluster("Clustername", nodes, distribution, minStorageNodesUp, 0.0); } 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, true); + ContentCluster cluster = new ContentCluster(clusterName, configuredNodeIndexes, distribution, minStorageNodesUp, 0.0); 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(), true); + 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); @@ -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(), true); + 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo()); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); 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 dc76b381f51..68926b4d10d 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 nodes = FleetControllerTest.toNodes(0, 1, 2, 3); ContentCluster cluster = new ContentCluster( - "books", nodes, distribution, 6 /* minStorageNodesUp*/, 0.9 /* minRatioOfStorageNodesUp */, - true /* determineBucketsFromBucketSpaceMetric */); + "books", nodes, distribution, 6 /* minStorageNodesUp*/, 0.9 /* minRatioOfStorageNodesUp */ + /* determineBucketsFromBucketSpaceMetric */); initializeCluster(cluster, nodes); AnnotatedClusterState baselineState = AnnotatedClusterState.withoutAnnotations(ClusterState.stateFromString("distributor:4 storage:4")); Map bucketSpaceStates = new HashMap<>(); @@ -57,8 +57,8 @@ public abstract class StateRestApiTest { Set nodesInSlobrok = FleetControllerTest.toNodes(1, 3, 5, 7); ContentCluster cluster = new ContentCluster( - "music", nodes, distribution, 4 /* minStorageNodesUp*/, 0.0 /* minRatioOfStorageNodesUp */, - true /* determineBucketsFromBucketSpaceMetric */); + "music", nodes, distribution, 4 /* minStorageNodesUp*/, 0.0 /* minRatioOfStorageNodesUp */ + /* determineBucketsFromBucketSpaceMetric */); 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