diff options
19 files changed, 442 insertions, 368 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingChecker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingChecker.java index 336f43043d7..f5f76f97a4d 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingChecker.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingChecker.java @@ -22,7 +22,7 @@ public class AggregatedStatsMergePendingChecker implements MergePendingChecker { if (!stats.hasUpdatesFromAllDistributors()) { return true; } - ContentNodeStats nodeStats = stats.getStats().getContentNode(contentNodeIndex); + ContentNodeStats nodeStats = stats.getStats().getNodeStats(contentNodeIndex); if (nodeStats != null) { ContentNodeStats.BucketSpaceStats bucketSpaceStats = nodeStats.getBucketSpace(bucketSpace); return (bucketSpaceStats != null && diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java index a21185ce41d..f28e2edbff5 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java @@ -93,13 +93,13 @@ public class ClusterStatsAggregator { for (ContentNodeStats contentNode : aggregatedStats) { Integer nodeIndex = contentNode.getNodeIndex(); - ContentNodeStats statsToAdd = clusterStats.getContentNode(nodeIndex); + ContentNodeStats statsToAdd = clusterStats.getNodeStats(nodeIndex); if (statsToAdd != null) { contentNode.add(statsToAdd); } if (prevClusterStats != null) { - ContentNodeStats statsToSubtract = prevClusterStats.getContentNode(nodeIndex); + ContentNodeStats statsToSubtract = prevClusterStats.getNodeStats(nodeIndex); if (statsToSubtract != null) { contentNode.subtract(statsToSubtract); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java index 6f6dd664f31..7f421e8bff9 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStats.java @@ -1,7 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core; -import java.util.*; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import java.util.Objects; +import java.util.Set; /** * Class for storing pending content node stats for all content nodes in the cluster. @@ -29,9 +34,7 @@ public class ContentClusterStats implements Iterable<ContentNodeStats> { return mapToNodeStats.values().iterator(); } - public ContentNodeStats getContentNode(Integer index) { - return mapToNodeStats.get(index); - } + public ContentNodeStats getNodeStats(Integer index) { return mapToNodeStats.get(index);} public int size() { return mapToNodeStats.size(); @@ -52,7 +55,6 @@ public class ContentClusterStats implements Iterable<ContentNodeStats> { @Override public String toString() { - return String.format("{mapToNodeStats=[%s]}", - Arrays.toString(mapToNodeStats.entrySet().toArray())); + return String.format("{mapToNodeStats=[%s]}", Arrays.toString(mapToNodeStats.entrySet().toArray())); } } 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 3ed03e94fda..9d8141020c3 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 @@ -8,18 +8,26 @@ import com.yahoo.vdslib.distribution.Group; import com.yahoo.vdslib.state.ClusterState; 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.HostInfo; 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.Map; import java.util.Objects; import java.util.Optional; +import static com.yahoo.vdslib.state.NodeType.STORAGE; +import static com.yahoo.vdslib.state.State.DOWN; +import static com.yahoo.vdslib.state.State.RETIRED; +import static com.yahoo.vdslib.state.State.UP; +import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result.allowSettingOfWantedState; +import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result.createAlreadySet; +import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result.createDisallowed; +import static com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest.Condition.FORCE; +import static com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest.Condition.SAFE; + /** * Checks if a node can be upgraded. * @@ -91,29 +99,28 @@ public class NodeStateChangeChecker { } } - public Result evaluateTransition( - Node node, ClusterState clusterState, SetUnitStateRequest.Condition condition, - NodeState oldWantedState, NodeState newWantedState) { - if (condition == SetUnitStateRequest.Condition.FORCE) { - return Result.allowSettingOfWantedState(); + public Result evaluateTransition(Node node, ClusterState clusterState, SetUnitStateRequest.Condition condition, + NodeState oldWantedState, NodeState newWantedState) { + if (condition == FORCE) { + return allowSettingOfWantedState(); } if (inMoratorium) { - return Result.createDisallowed("Master cluster controller is bootstrapping and in moratorium"); + return createDisallowed("Master cluster controller is bootstrapping and in moratorium"); } - if (condition != SetUnitStateRequest.Condition.SAFE) { - return Result.createDisallowed("Condition not implemented: " + condition.name()); + if (condition != SAFE) { + return createDisallowed("Condition not implemented: " + condition.name()); } - if (node.getType() != NodeType.STORAGE) { - return Result.createDisallowed("Safe-set of node state is only supported for storage nodes! " + + if (node.getType() != STORAGE) { + return createDisallowed("Safe-set of node state is only supported for storage nodes! " + "Requested node type: " + node.getType().toString()); } StorageNodeInfo nodeInfo = clusterInfo.getStorageNodeInfo(node.getIndex()); if (nodeInfo == null) { - return Result.createDisallowed("Unknown node " + node); + return createDisallowed("Unknown node " + node); } // If the new state and description equals the existing, we're done. This is done for 2 cases: @@ -123,41 +130,37 @@ public class NodeStateChangeChecker { // MUST_SET_WANTED_STATE if re-evaluated. This is important for implementing idempotent clients. if (newWantedState.getState().equals(oldWantedState.getState()) && Objects.equals(newWantedState.getDescription(), oldWantedState.getDescription())) { - return Result.createAlreadySet(); + return createAlreadySet(); } - switch (newWantedState.getState()) { - case UP: - return canSetStateUp(nodeInfo, oldWantedState); - case MAINTENANCE: - return canSetStateMaintenanceTemporarily(nodeInfo, clusterState, newWantedState.getDescription()); - case DOWN: - return canSetStateDownPermanently(nodeInfo, clusterState, newWantedState.getDescription()); - default: - return Result.createDisallowed("Destination node state unsupported in safe mode: " + newWantedState); - } + return switch (newWantedState.getState()) { + case UP -> canSetStateUp(nodeInfo, oldWantedState); + case MAINTENANCE -> canSetStateMaintenanceTemporarily(nodeInfo, clusterState, newWantedState.getDescription()); + case DOWN -> canSetStateDownPermanently(nodeInfo, clusterState, newWantedState.getDescription()); + default -> createDisallowed("Destination node state unsupported in safe mode: " + newWantedState); + }; } private Result canSetStateDownPermanently(NodeInfo nodeInfo, ClusterState clusterState, String newDescription) { NodeState oldWantedState = nodeInfo.getUserWantedState(); - if (oldWantedState.getState() != State.UP && !oldWantedState.getDescription().equals(newDescription)) { + if (oldWantedState.getState() != UP && !oldWantedState.getDescription().equals(newDescription)) { // Refuse to override whatever an operator or unknown entity is doing. // // Note: The new state&description is NOT equal to the old state&description: // that would have been short-circuited prior to this. - return Result.createDisallowed("A conflicting wanted state is already set: " + + return createDisallowed("A conflicting wanted state is already set: " + oldWantedState.getState() + ": " + oldWantedState.getDescription()); } State reportedState = nodeInfo.getReportedState().getState(); - if (reportedState != State.UP) { - return Result.createDisallowed("Reported state (" + reportedState + if (reportedState != UP) { + return createDisallowed("Reported state (" + reportedState + ") is not UP, so no bucket data is available"); } 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 " + if (currentState != RETIRED) { + return createDisallowed("Only retired nodes are allowed to be set to DOWN in safe mode - is " + currentState); } @@ -165,7 +168,7 @@ public class NodeStateChangeChecker { Integer hostInfoNodeVersion = hostInfo.getClusterStateVersionOrNull(); int clusterControllerVersion = clusterState.getVersion(); if (hostInfoNodeVersion == null || hostInfoNodeVersion != clusterControllerVersion) { - return Result.createDisallowed("Cluster controller at version " + clusterControllerVersion + return createDisallowed("Cluster controller at version " + clusterControllerVersion + " got info for storage node " + nodeInfo.getNodeIndex() + " at a different version " + hostInfoNodeVersion); } @@ -173,43 +176,43 @@ public class NodeStateChangeChecker { Optional<Metrics.Value> bucketsMetric; bucketsMetric = hostInfo.getMetrics().getValueAt(BUCKETS_METRIC_NAME, BUCKETS_METRIC_DIMENSIONS); if (bucketsMetric.isEmpty() || bucketsMetric.get().getLast() == null) { - return Result.createDisallowed("Missing last value of the " + BUCKETS_METRIC_NAME + + return createDisallowed("Missing last value of the " + BUCKETS_METRIC_NAME + " metric for storage node " + nodeInfo.getNodeIndex()); } long lastBuckets = bucketsMetric.get().getLast(); if (lastBuckets > 0) { - return Result.createDisallowed("The storage node manages " + lastBuckets + " buckets"); + return createDisallowed("The storage node manages " + lastBuckets + " buckets"); } - return Result.allowSettingOfWantedState(); + return allowSettingOfWantedState(); } private Result canSetStateUp(NodeInfo nodeInfo, NodeState oldWantedState) { - if (oldWantedState.getState() == State.UP) { + if (oldWantedState.getState() == UP) { // The description is not significant when setting wanting to set the state to UP - return Result.createAlreadySet(); + return createAlreadySet(); } - if (nodeInfo.getReportedState().getState() != State.UP) { - return Result.createDisallowed("Refuse to set wanted state to UP, " + + if (nodeInfo.getReportedState().getState() != UP) { + return createDisallowed("Refuse to set wanted state to UP, " + "since the reported state is not UP (" + nodeInfo.getReportedState().getState() + ")"); } - return Result.allowSettingOfWantedState(); + return allowSettingOfWantedState(); } private Result canSetStateMaintenanceTemporarily(StorageNodeInfo nodeInfo, ClusterState clusterState, String newDescription) { NodeState oldWantedState = nodeInfo.getUserWantedState(); - if (oldWantedState.getState() != State.UP && !oldWantedState.getDescription().equals(newDescription)) { + if (oldWantedState.getState() != UP && !oldWantedState.getDescription().equals(newDescription)) { // Refuse to override whatever an operator or unknown entity is doing. If the description is // identical, we assume it is the same operator. // // Note: The new state&description is NOT equal to the old state&description: // that would have been short-circuited prior to this. - return Result.createDisallowed("A conflicting wanted state is already set: " + + return createDisallowed("A conflicting wanted state is already set: " + oldWantedState.getState() + ": " + oldWantedState.getDescription()); } @@ -218,12 +221,12 @@ public class NodeStateChangeChecker { return otherGroupCheck; } - if (clusterState.getNodeState(nodeInfo.getNode()).getState() == State.DOWN) { - return Result.allowSettingOfWantedState(); + if (clusterState.getNodeState(nodeInfo.getNode()).getState() == DOWN) { + return allowSettingOfWantedState(); } if (anotherNodeInGroupAlreadyAllowed(nodeInfo, newDescription)) { - return Result.allowSettingOfWantedState(); + return allowSettingOfWantedState(); } Result allNodesAreUpCheck = checkAllNodesAreUp(clusterState); @@ -236,7 +239,7 @@ public class NodeStateChangeChecker { return checkDistributorsResult; } - return Result.allowSettingOfWantedState(); + return allowSettingOfWantedState(); } /** @@ -270,52 +273,50 @@ public class NodeStateChangeChecker { /** Returns a disallow-result, if there is a node in the group with wanted state != UP. */ private Result otherNodeInGroupHasWantedState(Group group) { for (var configuredNode : group.getNodes()) { - StorageNodeInfo storageNodeInfo = clusterInfo.getStorageNodeInfo(configuredNode.index()); + int index = configuredNode.index(); + StorageNodeInfo storageNodeInfo = clusterInfo.getStorageNodeInfo(index); if (storageNodeInfo == null) continue; // needed for tests only - State storageNodeWantedState = storageNodeInfo - .getUserWantedState().getState(); - if (storageNodeWantedState != State.UP) { - return Result.createDisallowed( - "At most one group can have wanted state: Other storage node " + configuredNode.index() + + State storageNodeWantedState = storageNodeInfo.getUserWantedState().getState(); + if (storageNodeWantedState != UP) { + return createDisallowed( + "At most one group can have wanted state: Other storage node " + index + " in group " + group.getIndex() + " has wanted state " + storageNodeWantedState); } - State distributorWantedState = clusterInfo.getDistributorNodeInfo(configuredNode.index()) - .getUserWantedState().getState(); - if (distributorWantedState != State.UP) { - return Result.createDisallowed( - "At most one group can have wanted state: Other distributor " + configuredNode.index() + + State distributorWantedState = clusterInfo.getDistributorNodeInfo(index).getUserWantedState().getState(); + if (distributorWantedState != UP) { + return createDisallowed( + "At most one group can have wanted state: Other distributor " + index + " in group " + group.getIndex() + " has wanted state " + distributorWantedState); } } - return Result.allowSettingOfWantedState(); + return allowSettingOfWantedState(); } private Result otherNodeHasWantedState(StorageNodeInfo nodeInfo) { for (var configuredNode : clusterInfo.getConfiguredNodes().values()) { - if (configuredNode.index() == nodeInfo.getNodeIndex()) { + int index = configuredNode.index(); + if (index == nodeInfo.getNodeIndex()) { continue; } - State storageNodeWantedState = clusterInfo.getStorageNodeInfo(configuredNode.index()) - .getUserWantedState().getState(); - if (storageNodeWantedState != State.UP) { - return Result.createDisallowed( + State storageNodeWantedState = clusterInfo.getStorageNodeInfo(index).getUserWantedState().getState(); + if (storageNodeWantedState != UP) { + return createDisallowed( "At most one node can have a wanted state when #groups = 1: Other storage node " + - configuredNode.index() + " has wanted state " + storageNodeWantedState); + index + " has wanted state " + storageNodeWantedState); } - State distributorWantedState = clusterInfo.getDistributorNodeInfo(configuredNode.index()) - .getUserWantedState().getState(); - if (distributorWantedState != State.UP) { - return Result.createDisallowed( + State distributorWantedState = clusterInfo.getDistributorNodeInfo(index).getUserWantedState().getState(); + if (distributorWantedState != UP) { + return createDisallowed( "At most one node can have a wanted state when #groups = 1: Other distributor " + - configuredNode.index() + " has wanted state " + distributorWantedState); + index + " has wanted state " + distributorWantedState); } } - return Result.allowSettingOfWantedState(); + return allowSettingOfWantedState(); } private boolean anotherNodeInGroupAlreadyAllowed(StorageNodeInfo nodeInfo, String newDescription) { @@ -361,33 +362,33 @@ public class NodeStateChangeChecker { for (NodeInfo storageNodeInfo : clusterInfo.getStorageNodeInfos()) { State wantedState = storageNodeInfo.getUserWantedState().getState(); - if (wantedState != State.UP && wantedState != State.RETIRED) { - return Result.createDisallowed("Another storage node wants state " + + if (wantedState != UP && wantedState != RETIRED) { + return createDisallowed("Another storage node wants state " + wantedState.toString().toUpperCase() + ": " + storageNodeInfo.getNodeIndex()); } State state = clusterState.getNodeState(storageNodeInfo.getNode()).getState(); - if (state != State.UP && state != State.RETIRED) { - return Result.createDisallowed("Another storage node has state " + state.toString().toUpperCase() + + if (state != UP && state != RETIRED) { + return createDisallowed("Another storage node has state " + state.toString().toUpperCase() + ": " + storageNodeInfo.getNodeIndex()); } } for (NodeInfo distributorNodeInfo : clusterInfo.getDistributorNodeInfos()) { State wantedState = distributorNodeInfo.getUserWantedState().getState(); - if (wantedState != State.UP && wantedState != State.RETIRED) { - return Result.createDisallowed("Another distributor wants state " + wantedState.toString().toUpperCase() + + if (wantedState != UP && wantedState != RETIRED) { + return createDisallowed("Another distributor wants state " + wantedState.toString().toUpperCase() + ": " + distributorNodeInfo.getNodeIndex()); } State state = clusterState.getNodeState(distributorNodeInfo.getNode()).getState(); - if (state != State.UP && state != State.RETIRED) { - return Result.createDisallowed("Another distributor has state " + state.toString().toUpperCase() + + if (state != UP && state != RETIRED) { + return createDisallowed("Another distributor has state " + state.toString().toUpperCase() + ": " + distributorNodeInfo.getNodeIndex()); } } - return Result.allowSettingOfWantedState(); + return allowSettingOfWantedState(); } private Result checkStorageNodesForDistributor( @@ -397,19 +398,19 @@ public class NodeStateChangeChecker { Integer minReplication = storageNode.getMinCurrentReplicationFactorOrNull(); // Why test on != null? Missing min-replication is OK (indicate empty/few buckets on system). if (minReplication != null && minReplication < requiredRedundancy) { - return Result.createDisallowed("Distributor " + return createDisallowed("Distributor " + distributorNodeInfo.getNodeIndex() + " says storage node " + node.getIndex() + " has buckets with redundancy as low as " + storageNode.getMinCurrentReplicationFactorOrNull() + ", but we require at least " + requiredRedundancy); } else { - return Result.allowSettingOfWantedState(); + return allowSettingOfWantedState(); } } } - return Result.allowSettingOfWantedState(); + return allowSettingOfWantedState(); } /** @@ -419,15 +420,15 @@ public class NodeStateChangeChecker { */ private Result checkDistributors(Node node, int clusterStateVersion) { if (clusterInfo.getDistributorNodeInfos().isEmpty()) { - return Result.createDisallowed("Not aware of any distributors, probably not safe to upgrade?"); + return createDisallowed("Not aware of any distributors, probably not safe to upgrade?"); } for (DistributorNodeInfo distributorNodeInfo : clusterInfo.getDistributorNodeInfos()) { Integer distributorClusterStateVersion = distributorNodeInfo.getHostInfo().getClusterStateVersionOrNull(); if (distributorClusterStateVersion == null) { - return Result.createDisallowed("Distributor node " + distributorNodeInfo.getNodeIndex() + return createDisallowed("Distributor node " + distributorNodeInfo.getNodeIndex() + " has not reported any cluster state version yet."); } else if (distributorClusterStateVersion != clusterStateVersion) { - return Result.createDisallowed("Distributor node " + distributorNodeInfo.getNodeIndex() + return createDisallowed("Distributor node " + distributorNodeInfo.getNodeIndex() + " does not report same version (" + distributorNodeInfo.getHostInfo().getClusterStateVersionOrNull() + ") as fleetcontroller (" + clusterStateVersion + ")"); @@ -440,7 +441,7 @@ public class NodeStateChangeChecker { } } - return Result.allowSettingOfWantedState(); + return allowSettingOfWantedState(); } } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerStateRestAPI.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerStateRestAPI.java index 5ca76d40fe1..4d57b92491c 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerStateRestAPI.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerStateRestAPI.java @@ -17,7 +17,6 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.UnitStateRe import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.SetResponse; import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitResponse; import java.util.Map; -import java.util.Objects; import java.util.logging.Logger; public class ClusterControllerStateRestAPI implements StateRestAPI { @@ -28,32 +27,8 @@ public class ClusterControllerStateRestAPI implements StateRestAPI { Map<String, RemoteClusterControllerTaskScheduler> getFleetControllers(); } - public static class Socket { - public final String hostname; - public final int port; - - public Socket(String hostname, int port) { - this.hostname = hostname; - this.port = port; - } - - @Override - public String toString() { - return hostname + ":" + port; - } - - @Override - public int hashCode() { - return Objects.hash(hostname, port); - } - - @Override - public boolean equals(Object object) { - if (this == object) return true; - if (!(object instanceof Socket)) return false; - Socket socket = (Socket) object; - return Objects.equals(hostname, socket.hostname) && port == socket.port; - } + public record Socket(String hostname, int port) { + @Override public String toString() { return hostname + ":" + port; } } private final FleetControllerResolver fleetControllerResolver; @@ -98,9 +73,9 @@ public class ClusterControllerStateRestAPI implements StateRestAPI { } else { log.fine("Scheduling state request: " + req.getClass().toString()); resolver.resolveFleetController(request.getUnitPath()).schedule(req); - log.finest("Scheduled state request: " + req.getClass().toString()); + log.finest("Scheduled state request: " + req.getClass()); req.waitForCompletion(); - log.finest("Completed processing state request: " + req.getClass().toString()); + log.finest("Completed processing state request: " + req.getClass()); } try { return req.getResult(); @@ -119,7 +94,7 @@ public class ClusterControllerStateRestAPI implements StateRestAPI { { @Override public Request<? extends SetResponse> visitCluster(Id.Cluster id) { - return new SetNodeStatesForClusterRequest(id, request); + return new SetNodeStatesForClusterRequest(request); } @Override public Request<? extends SetResponse> visitNode(Id.Node id) { diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java index 1cc8f2860c6..3dbeb6c86dd 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java @@ -6,7 +6,6 @@ import com.yahoo.vdslib.distribution.ConfiguredNode; import com.yahoo.vdslib.state.Node; import com.yahoo.vdslib.state.NodeType; import com.yahoo.vespa.clustercontroller.core.RemoteClusterControllerTask; -import com.yahoo.vespa.clustercontroller.core.restapiv2.Id; import com.yahoo.vespa.clustercontroller.core.restapiv2.Request; import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.InternalFailure; import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.InvalidContentException; @@ -14,25 +13,20 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.StateRestApiE import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest; import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.SetResponse; import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitState; - import java.time.Instant; import java.util.Map; import java.util.Optional; -import java.util.logging.Logger; public class SetNodeStatesForClusterRequest extends Request<SetResponse> { - private static final Logger log = Logger.getLogger(SetNodeStateRequest.class.getName()); - private final Id.Cluster cluster; private final Map<String, UnitState> newStates; private final SetUnitStateRequest.Condition condition; private final TimeBudget timeBudget; private final boolean probe; - public SetNodeStatesForClusterRequest(Id.Cluster cluster, SetUnitStateRequest request) { + public SetNodeStatesForClusterRequest(SetUnitStateRequest request) { super(MasterState.MUST_BE_MASTER); - this.cluster = cluster; this.newStates = request.getNewState(); this.condition = request.getCondition(); this.timeBudget = request.timeBudget(); @@ -42,7 +36,7 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> { @Override public SetResponse calculateResult(RemoteClusterControllerTask.Context context) throws StateRestApiException { if (condition != SetUnitStateRequest.Condition.FORCE) { - // Setting all nodes to e.g. maintainence is by design unsafe in the sense + // Setting all nodes to e.g. maintenance is by design unsafe in the sense // that it allows effective redundancy to drop to 0, many/all nodes may // go down, etc. This is prohibited in Condition.SAFE. throw new InvalidContentException( diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRenderer.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRenderer.java index 363eec4e8e4..547647e82e6 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRenderer.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/statuspage/VdsClusterHtmlRenderer.java @@ -419,7 +419,7 @@ public class VdsClusterHtmlRenderer { private static ContentNodeStats.BucketSpaceStats getStatsForContentNode(ClusterStatsAggregator statsAggregator, NodeInfo nodeInfo, String bucketSpace) { - ContentNodeStats nodeStats = statsAggregator.getAggregatedStats().getStats().getContentNode(nodeInfo.getNodeIndex()); + ContentNodeStats nodeStats = statsAggregator.getAggregatedStats().getStats().getNodeStats(nodeInfo.getNodeIndex()); if (nodeStats != null) { return nodeStats.getBucketSpace(bucketSpace); } 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 6af852aa3ed..4718453ee51 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 @@ -8,33 +8,42 @@ import com.yahoo.vdslib.distribution.GroupVisitor; import com.yahoo.vdslib.state.ClusterState; 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.HostInfo; -import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest; import com.yahoo.vespa.config.content.StorDistributionConfig; import org.junit.jupiter.api.Test; - import java.text.ParseException; import java.util.ArrayList; import java.util.Collection; import java.util.List; -import static org.junit.jupiter.api.Assertions.*; +import static com.yahoo.vdslib.state.NodeType.DISTRIBUTOR; +import static com.yahoo.vdslib.state.NodeType.STORAGE; +import static com.yahoo.vdslib.state.State.DOWN; +import static com.yahoo.vdslib.state.State.INITIALIZING; +import static com.yahoo.vdslib.state.State.UP; +import static com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest.Condition.FORCE; +import static com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest.Condition.SAFE; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static com.yahoo.vespa.clustercontroller.core.NodeStateChangeChecker.Result; + public class NodeStateChangeCheckerTest { private static final int requiredRedundancy = 4; private static final int currentClusterStateVersion = 2; - private static final Node nodeDistributor = new Node(NodeType.DISTRIBUTOR, 1); - private static final Node nodeStorage = new Node(NodeType.STORAGE, 1); + private static final Node nodeDistributor = new Node(DISTRIBUTOR, 1); + private static final Node nodeStorage = new Node(STORAGE, 1); - private static final NodeState UP_NODE_STATE = new NodeState(NodeType.STORAGE, State.UP); + private static final NodeState UP_NODE_STATE = new NodeState(STORAGE, UP); private static final NodeState MAINTENANCE_NODE_STATE = createNodeState(State.MAINTENANCE, "Orchestrator"); - private static final NodeState DOWN_NODE_STATE = createNodeState(State.DOWN, "RetireEarlyExpirer"); + private static final NodeState DOWN_NODE_STATE = createNodeState(DOWN, "RetireEarlyExpirer"); private static final HierarchicalGroupVisiting noopVisiting = new HierarchicalGroupVisiting() { @Override public boolean isHierarchical() { return false; } @@ -42,7 +51,7 @@ public class NodeStateChangeCheckerTest { }; private static NodeState createNodeState(State state, String description) { - return new NodeState(NodeType.STORAGE, state).setDescription(description); + return new NodeState(STORAGE, state).setDescription(description); } private static ClusterState clusterState(String state) { @@ -61,7 +70,8 @@ public class NodeStateChangeCheckerTest { return new NodeStateChangeChecker(requiredRedundancy, noopVisiting, cluster.clusterInfo(), false); } - private ContentCluster createCluster(Collection<ConfiguredNode> nodes) { + private ContentCluster createCluster(int nodeCount) { + Collection<ConfiguredNode> nodes = createNodes(nodeCount); Distribution distribution = mock(Distribution.class); Group group = new Group(2, "two"); when(distribution.getRootGroup()).thenReturn(group); @@ -97,18 +107,18 @@ public class NodeStateChangeCheckerTest { final ClusterInfo clusterInfo = cluster.clusterInfo(); final int configuredNodeCount = cluster.clusterInfo().getConfiguredNodes().size(); for (int i = 0; i < configuredNodeCount; i++) { - clusterInfo.getDistributorNodeInfo(i).setReportedState(new NodeState(NodeType.DISTRIBUTOR, State.UP), 0); + clusterInfo.getDistributorNodeInfo(i).setReportedState(new NodeState(DISTRIBUTOR, UP), 0); clusterInfo.getDistributorNodeInfo(i).setHostInfo(HostInfo.createHostInfo(createDistributorHostInfo(4, 5, 6))); - clusterInfo.getStorageNodeInfo(i).setReportedState(new NodeState(NodeType.STORAGE, State.UP), 0); + clusterInfo.getStorageNodeInfo(i).setReportedState(new NodeState(STORAGE, UP), 0); } } @Test void testCanUpgradeForce() { - NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(createCluster(createNodes(1))); - NodeState newState = new NodeState(NodeType.STORAGE, State.INITIALIZING); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - nodeDistributor, defaultAllUpClusterState(), SetUnitStateRequest.Condition.FORCE, + var nodeStateChangeChecker = createChangeChecker(createCluster(1)); + NodeState newState = new NodeState(STORAGE, INITIALIZING); + Result result = nodeStateChangeChecker.evaluateTransition( + nodeDistributor, defaultAllUpClusterState(), FORCE, UP_NODE_STATE, newState); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -116,11 +126,11 @@ public class NodeStateChangeCheckerTest { @Test void testDeniedInMoratorium() { - ContentCluster cluster = createCluster(createNodes(4)); - NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( + ContentCluster cluster = createCluster(4); + var nodeStateChangeChecker = new NodeStateChangeChecker( requiredRedundancy, noopVisiting, cluster.clusterInfo(), true); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - new Node(NodeType.STORAGE, 10), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + Result result = nodeStateChangeChecker.evaluateTransition( + new Node(STORAGE, 10), defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -129,11 +139,11 @@ public class NodeStateChangeCheckerTest { @Test void testUnknownStorageNode() { - ContentCluster cluster = createCluster(createNodes(4)); - NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( + ContentCluster cluster = createCluster(4); + var nodeStateChangeChecker = new NodeStateChangeChecker( requiredRedundancy, noopVisiting, cluster.clusterInfo(), false); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - new Node(NodeType.STORAGE, 10), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + Result result = nodeStateChangeChecker.evaluateTransition( + new Node(STORAGE, 10), defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -143,17 +153,17 @@ public class NodeStateChangeCheckerTest { @Test void testSafeMaintenanceDisallowedWhenOtherStorageNodeInFlatClusterIsSuspended() { // Nodes 0-3, storage node 0 being in maintenance with "Orchestrator" description. - ContentCluster cluster = createCluster(createNodes(4)); - cluster.clusterInfo().getStorageNodeInfo(0).setWantedState(new NodeState(NodeType.STORAGE, State.MAINTENANCE).setDescription("Orchestrator")); - NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( + ContentCluster cluster = createCluster(4); + cluster.clusterInfo().getStorageNodeInfo(0).setWantedState(new NodeState(STORAGE, State.MAINTENANCE).setDescription("Orchestrator")); + var nodeStateChangeChecker = new NodeStateChangeChecker( requiredRedundancy, noopVisiting, cluster.clusterInfo(), false); ClusterState clusterStateWith0InMaintenance = clusterState(String.format( "version:%d distributor:4 storage:4 .0.s:m", currentClusterStateVersion)); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - new Node(NodeType.STORAGE, 1), clusterStateWith0InMaintenance, - SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); + Result result = nodeStateChangeChecker.evaluateTransition( + new Node(STORAGE, 1), clusterStateWith0InMaintenance, + SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertEquals("At most one node can have a wanted state when #groups = 1: Other storage node 0 has wanted state Maintenance", @@ -163,18 +173,18 @@ public class NodeStateChangeCheckerTest { @Test void testSafeMaintenanceDisallowedWhenOtherDistributorInFlatClusterIsSuspended() { // Nodes 0-3, storage node 0 being in maintenance with "Orchestrator" description. - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); cluster.clusterInfo().getDistributorNodeInfo(0) - .setWantedState(new NodeState(NodeType.DISTRIBUTOR, State.DOWN).setDescription("Orchestrator")); - NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( + .setWantedState(new NodeState(DISTRIBUTOR, DOWN).setDescription("Orchestrator")); + var nodeStateChangeChecker = new NodeStateChangeChecker( requiredRedundancy, noopVisiting, cluster.clusterInfo(), false); ClusterState clusterStateWith0InMaintenance = clusterState(String.format( "version:%d distributor:4 .0.s:d storage:4", currentClusterStateVersion)); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - new Node(NodeType.STORAGE, 1), clusterStateWith0InMaintenance, - SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); + Result result = nodeStateChangeChecker.evaluateTransition( + new Node(STORAGE, 1), clusterStateWith0InMaintenance, + SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertEquals("At most one node can have a wanted state when #groups = 1: Other distributor 0 has wanted state Down", @@ -185,11 +195,11 @@ public class NodeStateChangeCheckerTest { void testSafeMaintenanceDisallowedWhenDistributorInGroupIsDown() { // Nodes 0-3, distributor 0 being in maintenance with "Orchestrator" description. // 2 groups: nodes 0-1 is group 0, 2-3 is group 1. - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); cluster.clusterInfo().getDistributorNodeInfo(0) - .setWantedState(new NodeState(NodeType.STORAGE, State.DOWN).setDescription("Orchestrator")); + .setWantedState(new NodeState(STORAGE, DOWN).setDescription("Orchestrator")); HierarchicalGroupVisiting visiting = makeHierarchicalGroupVisitingWith2Groups(4); - NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( + var nodeStateChangeChecker = new NodeStateChangeChecker( requiredRedundancy, visiting, cluster.clusterInfo(), false); ClusterState clusterStateWith0InMaintenance = clusterState(String.format( "version:%d distributor:4 .0.s:d storage:4", @@ -197,9 +207,9 @@ public class NodeStateChangeCheckerTest { { // Denied for node 2 in group 1, since distributor 0 in group 0 is down - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - new Node(NodeType.STORAGE, 2), clusterStateWith0InMaintenance, - SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); + Result result = nodeStateChangeChecker.evaluateTransition( + new Node(STORAGE, 2), clusterStateWith0InMaintenance, + SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertEquals("At most one group can have wanted state: Other distributor 0 in group 0 has wanted state Down", result.getReason()); @@ -208,9 +218,9 @@ public class NodeStateChangeCheckerTest { { // Even node 1 of group 0 is not permitted, as node 0 is not considered // suspended since only the distributor has been set down. - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - new Node(NodeType.STORAGE, 1), clusterStateWith0InMaintenance, - SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); + Result result = nodeStateChangeChecker.evaluateTransition( + new Node(STORAGE, 1), clusterStateWith0InMaintenance, + SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed(), result.getReason()); assertEquals("Another distributor wants state DOWN: 0", result.getReason()); } @@ -220,10 +230,10 @@ public class NodeStateChangeCheckerTest { void testSafeMaintenanceWhenOtherStorageNodeInGroupIsSuspended() { // Nodes 0-3, storage node 0 being in maintenance with "Orchestrator" description. // 2 groups: nodes 0-1 is group 0, 2-3 is group 1. - ContentCluster cluster = createCluster(createNodes(4)); - cluster.clusterInfo().getStorageNodeInfo(0).setWantedState(new NodeState(NodeType.STORAGE, State.MAINTENANCE).setDescription("Orchestrator")); + ContentCluster cluster = createCluster(4); + cluster.clusterInfo().getStorageNodeInfo(0).setWantedState(new NodeState(STORAGE, State.MAINTENANCE).setDescription("Orchestrator")); HierarchicalGroupVisiting visiting = makeHierarchicalGroupVisitingWith2Groups(4); - NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( + var nodeStateChangeChecker = new NodeStateChangeChecker( requiredRedundancy, visiting, cluster.clusterInfo(), false); ClusterState clusterStateWith0InMaintenance = clusterState(String.format( "version:%d distributor:4 storage:4 .0.s:m", @@ -231,9 +241,9 @@ public class NodeStateChangeCheckerTest { { // Denied for node 2 in group 1, since node 0 in group 0 is in maintenance - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - new Node(NodeType.STORAGE, 2), clusterStateWith0InMaintenance, - SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); + Result result = nodeStateChangeChecker.evaluateTransition( + new Node(STORAGE, 2), clusterStateWith0InMaintenance, + SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertEquals("At most one group can have wanted state: Other storage node 0 in group 0 has wanted state Maintenance", @@ -243,9 +253,9 @@ public class NodeStateChangeCheckerTest { { // Permitted for node 1 in group 0, since node 0 is already in maintenance with // description Orchestrator, and it is in the same group - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - new Node(NodeType.STORAGE, 1), clusterStateWith0InMaintenance, - SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); + Result result = nodeStateChangeChecker.evaluateTransition( + new Node(STORAGE, 1), clusterStateWith0InMaintenance, + SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertTrue(result.settingWantedStateIsAllowed(), result.getReason()); assertFalse(result.wantedStateAlreadySet()); } @@ -293,9 +303,9 @@ public class NodeStateChangeCheckerTest { @Test void testSafeSetStateDistributors() { - NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(createCluster(createNodes(1))); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - nodeDistributor, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(createCluster(1)); + Result result = nodeStateChangeChecker.evaluateTransition( + nodeDistributor, defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -305,18 +315,18 @@ public class NodeStateChangeCheckerTest { @Test void testCanUpgradeSafeMissingStorage() { // Create a content cluster with 4 nodes, and storage node with index 3 down. - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); setAllNodesUp(cluster, HostInfo.createHostInfo(createDistributorHostInfo(4, 5, 6))); - cluster.clusterInfo().getStorageNodeInfo(3).setReportedState(new NodeState(NodeType.STORAGE, State.DOWN), 0); + cluster.clusterInfo().getStorageNodeInfo(3).setReportedState(new NodeState(STORAGE, DOWN), 0); ClusterState clusterStateWith3Down = clusterState(String.format( "version:%d distributor:4 storage:4 .3.s:d", currentClusterStateVersion)); // We should then be denied setting storage node 1 safely to maintenance. - NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( + var nodeStateChangeChecker = new NodeStateChangeChecker( requiredRedundancy, noopVisiting, cluster.clusterInfo(), false); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - nodeStorage, clusterStateWith3Down, SetUnitStateRequest.Condition.SAFE, + Result result = nodeStateChangeChecker.evaluateTransition( + nodeStorage, clusterStateWith3Down, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -325,19 +335,19 @@ public class NodeStateChangeCheckerTest { @Test void testCanUpgradeStorageSafeYes() { - NodeStateChangeChecker.Result result = transitionToMaintenanceWithNoStorageNodesDown(); + Result result = transitionToMaintenanceWithNoStorageNodesDown(createCluster(4), defaultAllUpClusterState()); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } @Test void testSetUpFailsIfReportedIsDown() { - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); // Not setting nodes up -> all are down - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + Result result = nodeStateChangeChecker.evaluateTransition( + nodeStorage, defaultAllUpClusterState(), SAFE, MAINTENANCE_NODE_STATE, UP_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -347,7 +357,7 @@ public class NodeStateChangeCheckerTest { // nodes taken down implicitly due to a group having too low node availability. @Test void testSetUpSucceedsIfReportedIsUpButGeneratedIsDown() { - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); markAllNodesAsReportingStateUp(cluster); @@ -356,8 +366,8 @@ public class NodeStateChangeCheckerTest { "version:%d distributor:4 storage:4 .%d.s:d", currentClusterStateVersion, nodeStorage.getIndex())); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - nodeStorage, stateWithNodeDown, SetUnitStateRequest.Condition.SAFE, + Result result = nodeStateChangeChecker.evaluateTransition( + nodeStorage, stateWithNodeDown, SAFE, MAINTENANCE_NODE_STATE, UP_NODE_STATE); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -365,25 +375,25 @@ public class NodeStateChangeCheckerTest { @Test void testCanSetUpEvenIfOldWantedStateIsDown() { - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); setAllNodesUp(cluster, HostInfo.createHostInfo(createDistributorHostInfo(4, 3, 6))); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, - new NodeState(NodeType.STORAGE, State.DOWN), UP_NODE_STATE); + Result result = nodeStateChangeChecker.evaluateTransition( + nodeStorage, defaultAllUpClusterState(), SAFE, + new NodeState(STORAGE, DOWN), UP_NODE_STATE); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } @Test void testCanUpgradeStorageSafeNo() { - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); setAllNodesUp(cluster, HostInfo.createHostInfo(createDistributorHostInfo(4, 3, 6))); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + Result result = nodeStateChangeChecker.evaluateTransition( + nodeStorage, defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -393,12 +403,12 @@ public class NodeStateChangeCheckerTest { @Test void testCanUpgradeIfMissingMinReplicationFactor() { - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); setAllNodesUp(cluster, HostInfo.createHostInfo(createDistributorHostInfo(4, 3, 6))); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - new Node(NodeType.STORAGE, 3), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + Result result = nodeStateChangeChecker.evaluateTransition( + new Node(STORAGE, 3), defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -406,7 +416,7 @@ public class NodeStateChangeCheckerTest { @Test void testCanUpgradeIfStorageNodeMissingFromNodeInfo() { - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); String hostInfo = "{\n" + " \"cluster-state-version\": 2,\n" + @@ -421,8 +431,8 @@ public class NodeStateChangeCheckerTest { "}\n"; setAllNodesUp(cluster, HostInfo.createHostInfo(hostInfo)); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - new Node(NodeType.STORAGE, 1), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + Result result = nodeStateChangeChecker.evaluateTransition( + new Node(STORAGE, 1), defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -430,115 +440,104 @@ public class NodeStateChangeCheckerTest { @Test void testMissingDistributorState() { - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); - cluster.clusterInfo().getStorageNodeInfo(1).setReportedState(new NodeState(NodeType.STORAGE, State.UP), 0); + cluster.clusterInfo().getStorageNodeInfo(1).setReportedState(new NodeState(STORAGE, UP), 0); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); + Result result = nodeStateChangeChecker.evaluateTransition( + nodeStorage, defaultAllUpClusterState(), SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertEquals("Distributor node 0 has not reported any cluster state version yet.", result.getReason()); } - private NodeStateChangeChecker.Result transitionToSameState(State state, String oldDescription, String newDescription) { - ContentCluster cluster = createCluster(createNodes(4)); + private Result transitionToSameState(State state, String oldDescription, String newDescription) { + ContentCluster cluster = createCluster(4); NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); NodeState currentNodeState = createNodeState(state, oldDescription); NodeState newNodeState = createNodeState(state, newDescription); return nodeStateChangeChecker.evaluateTransition( - nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + nodeStorage, defaultAllUpClusterState(), SAFE, currentNodeState, newNodeState); } - private NodeStateChangeChecker.Result transitionToSameState(String oldDescription, String newDescription) { + private Result transitionToSameState(String oldDescription, String newDescription) { return transitionToSameState(State.MAINTENANCE, oldDescription, newDescription); } @Test void testSettingUpWhenUpCausesAlreadySet() { - NodeStateChangeChecker.Result result = transitionToSameState(State.UP, "foo", "bar"); + Result result = transitionToSameState(UP, "foo", "bar"); assertTrue(result.wantedStateAlreadySet()); } @Test void testSettingAlreadySetState() { - NodeStateChangeChecker.Result result = transitionToSameState("foo", "foo"); + Result result = transitionToSameState("foo", "foo"); assertFalse(result.settingWantedStateIsAllowed()); assertTrue(result.wantedStateAlreadySet()); } @Test void testDifferentDescriptionImpliesDenied() { - NodeStateChangeChecker.Result result = transitionToSameState("foo", "bar"); + Result result = transitionToSameState("foo", "bar"); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } - private NodeStateChangeChecker.Result transitionToMaintenanceWithOneStorageNodeDown( - int storageNodeIndex, boolean alternatingUpRetiredAndInitializing) { - ContentCluster cluster = createCluster(createNodes(4)); + private Result transitionToMaintenanceWithOneStorageNodeDown(ContentCluster cluster, ClusterState clusterState) { NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); for (int x = 0; x < cluster.clusterInfo().getConfiguredNodes().size(); x++) { - State state = State.UP; - // Pick some retired and initializing nodes too - if (alternatingUpRetiredAndInitializing) { // TODO: Move this into the calling test - if (x % 3 == 1) state = State.RETIRED; - else if (x % 3 == 2) state = State.INITIALIZING; - } - cluster.clusterInfo().getDistributorNodeInfo(x).setReportedState(new NodeState(NodeType.DISTRIBUTOR, state), 0); + State state = UP; + cluster.clusterInfo().getDistributorNodeInfo(x).setReportedState(new NodeState(DISTRIBUTOR, state), 0); cluster.clusterInfo().getDistributorNodeInfo(x).setHostInfo(HostInfo.createHostInfo(createDistributorHostInfo(4, 5, 6))); - cluster.clusterInfo().getStorageNodeInfo(x).setReportedState(new NodeState(NodeType.STORAGE, state), 0); - } - - ClusterState clusterState = defaultAllUpClusterState(); - - if (storageNodeIndex >= 0) { // TODO: Move this into the calling test - NodeState downNodeState = new NodeState(NodeType.STORAGE, State.DOWN); - cluster.clusterInfo().getStorageNodeInfo(storageNodeIndex).setReportedState(downNodeState, 4 /* time */); - clusterState.setNodeState(new Node(NodeType.STORAGE, storageNodeIndex), downNodeState); + cluster.clusterInfo().getStorageNodeInfo(x).setReportedState(new NodeState(STORAGE, state), 0); } return nodeStateChangeChecker.evaluateTransition( - nodeStorage, clusterState, SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); + nodeStorage, clusterState, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); } private void setAllNodesUp(ContentCluster cluster, HostInfo distributorHostInfo) { for (int x = 0; x < cluster.clusterInfo().getConfiguredNodes().size(); x++) { - State state = State.UP; - cluster.clusterInfo().getDistributorNodeInfo(x).setReportedState(new NodeState(NodeType.DISTRIBUTOR, state), 0); + State state = UP; + cluster.clusterInfo().getDistributorNodeInfo(x).setReportedState(new NodeState(DISTRIBUTOR, state), 0); cluster.clusterInfo().getDistributorNodeInfo(x).setHostInfo(distributorHostInfo); - cluster.clusterInfo().getStorageNodeInfo(x).setReportedState(new NodeState(NodeType.STORAGE, state), 0); + cluster.clusterInfo().getStorageNodeInfo(x).setReportedState(new NodeState(STORAGE, state), 0); } } - private NodeStateChangeChecker.Result transitionToMaintenanceWithOneStorageNodeDown(int storageNodeIndex) { - return transitionToMaintenanceWithOneStorageNodeDown(storageNodeIndex, false); - } - - private NodeStateChangeChecker.Result transitionToMaintenanceWithNoStorageNodesDown() { - return transitionToMaintenanceWithOneStorageNodeDown(-1, false); + private Result transitionToMaintenanceWithNoStorageNodesDown(ContentCluster cluster, ClusterState clusterState) { + return transitionToMaintenanceWithOneStorageNodeDown(cluster, clusterState); } @Test void testCanUpgradeWhenAllUp() { - NodeStateChangeChecker.Result result = transitionToMaintenanceWithNoStorageNodesDown(); + Result result = transitionToMaintenanceWithNoStorageNodesDown(createCluster(4), defaultAllUpClusterState()); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } @Test void testCanUpgradeWhenAllUpOrRetired() { - NodeStateChangeChecker.Result result = transitionToMaintenanceWithNoStorageNodesDown(); + Result result = transitionToMaintenanceWithNoStorageNodesDown(createCluster(4), defaultAllUpClusterState()); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } @Test void testCanUpgradeWhenStorageIsDown() { - NodeStateChangeChecker.Result result = transitionToMaintenanceWithOneStorageNodeDown(nodeStorage.getIndex()); + ClusterState clusterState = defaultAllUpClusterState(); + var storageNodeIndex = nodeStorage.getIndex(); + + ContentCluster cluster = createCluster(4); + NodeState downNodeState = new NodeState(STORAGE, DOWN); + cluster.clusterInfo().getStorageNodeInfo(storageNodeIndex).setReportedState(downNodeState, 4 /* time */); + clusterState.setNodeState(new Node(STORAGE, storageNodeIndex), downNodeState); + + Result result = transitionToMaintenanceWithOneStorageNodeDown(cluster, clusterState); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } @@ -549,7 +548,13 @@ public class NodeStateChangeCheckerTest { // If this fails, just set otherIndex to some other valid index. assertNotEquals(nodeStorage.getIndex(), otherIndex); - NodeStateChangeChecker.Result result = transitionToMaintenanceWithOneStorageNodeDown(otherIndex); + ContentCluster cluster = createCluster(4); + ClusterState clusterState = defaultAllUpClusterState(); + NodeState downNodeState = new NodeState(STORAGE, DOWN); + cluster.clusterInfo().getStorageNodeInfo(otherIndex).setReportedState(downNodeState, 4 /* time */); + clusterState.setNodeState(new Node(STORAGE, otherIndex), downNodeState); + + Result result = transitionToMaintenanceWithOneStorageNodeDown(cluster, clusterState); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); assertTrue(result.getReason().contains("Another storage node has state DOWN: 2")); @@ -557,7 +562,7 @@ public class NodeStateChangeCheckerTest { @Test void testNodeRatioRequirementConsidersGeneratedNodeStates() { - ContentCluster cluster = createCluster(createNodes(4)); + ContentCluster cluster = createCluster(4); NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); markAllNodesAsReportingStateUp(cluster); @@ -570,8 +575,8 @@ public class NodeStateChangeCheckerTest { "version:%d distributor:4 storage:4 .3.s:d", currentClusterStateVersion)); - NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( - nodeStorage, stateWithNodeDown, SetUnitStateRequest.Condition.SAFE, + Result result = nodeStateChangeChecker.evaluateTransition( + nodeStorage, stateWithNodeDown, SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); @@ -579,9 +584,9 @@ public class NodeStateChangeCheckerTest { @Test void testDownDisallowedByNonRetiredState() { - NodeStateChangeChecker.Result result = evaluateDownTransition( + Result result = evaluateDownTransition( defaultAllUpClusterState(), - State.UP, + UP, currentClusterStateVersion, 0); assertFalse(result.settingWantedStateIsAllowed()); @@ -591,9 +596,9 @@ public class NodeStateChangeCheckerTest { @Test void testDownDisallowedByBuckets() { - NodeStateChangeChecker.Result result = evaluateDownTransition( + Result result = evaluateDownTransition( retiredClusterStateSuffix(), - State.UP, + UP, currentClusterStateVersion, 1); assertFalse(result.settingWantedStateIsAllowed()); @@ -603,9 +608,9 @@ public class NodeStateChangeCheckerTest { @Test void testDownDisallowedByReportedState() { - NodeStateChangeChecker.Result result = evaluateDownTransition( + Result result = evaluateDownTransition( retiredClusterStateSuffix(), - State.INITIALIZING, + INITIALIZING, currentClusterStateVersion, 0); assertFalse(result.settingWantedStateIsAllowed()); @@ -615,9 +620,9 @@ public class NodeStateChangeCheckerTest { @Test void testDownDisallowedByVersionMismatch() { - NodeStateChangeChecker.Result result = evaluateDownTransition( + Result result = evaluateDownTransition( retiredClusterStateSuffix(), - State.UP, + UP, currentClusterStateVersion - 1, 0); assertFalse(result.settingWantedStateIsAllowed()); @@ -628,29 +633,28 @@ public class NodeStateChangeCheckerTest { @Test void testAllowedToSetDown() { - NodeStateChangeChecker.Result result = evaluateDownTransition( + Result result = evaluateDownTransition( retiredClusterStateSuffix(), - State.UP, + UP, currentClusterStateVersion, 0); assertTrue(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); } - private NodeStateChangeChecker.Result evaluateDownTransition( - ClusterState clusterState, - State reportedState, - int hostInfoClusterStateVersion, - int lastAlldisksBuckets) { - ContentCluster cluster = createCluster(createNodes(4)); + private Result evaluateDownTransition(ClusterState clusterState, + State reportedState, + int hostInfoClusterStateVersion, + int lastAlldisksBuckets) { + ContentCluster cluster = createCluster(4); NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster); StorageNodeInfo nodeInfo = cluster.clusterInfo().getStorageNodeInfo(nodeStorage.getIndex()); - nodeInfo.setReportedState(new NodeState(NodeType.STORAGE, reportedState), 0); + nodeInfo.setReportedState(new NodeState(STORAGE, reportedState), 0); nodeInfo.setHostInfo(createHostInfoWithMetrics(hostInfoClusterStateVersion, lastAlldisksBuckets)); return nodeStateChangeChecker.evaluateTransition( - nodeStorage, clusterState, SetUnitStateRequest.Condition.SAFE, + nodeStorage, clusterState, SAFE, UP_NODE_STATE, DOWN_NODE_STATE); } 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 9d41b1c0153..58208cb453b 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 @@ -18,6 +18,7 @@ import com.yahoo.vespa.clustercontroller.core.hostinfo.HostInfo; import com.yahoo.vespa.clustercontroller.utils.staterestapi.StateRestAPI; import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.UnitStateRequest; import com.yahoo.vespa.clustercontroller.utils.staterestapi.server.JsonWriter; +import com.yahoo.vespa.config.content.StorDistributionConfig; import java.util.Collection; import java.util.HashMap; @@ -51,7 +52,7 @@ public abstract class StateRestApiTest { } protected void setUp(boolean dontInitializeNode2) throws Exception { - Distribution distribution = new Distribution(Distribution.getSimpleGroupConfig(2, 10)); + Distribution distribution = new Distribution(getSimpleGroupConfig(2, 10)); jsonWriter.setDefaultPathPrefix("/cluster/v2"); { Set<ConfiguredNode> nodes = FleetControllerTest.toNodes(0, 1, 2, 3); @@ -94,7 +95,7 @@ public abstract class StateRestApiTest { protected void setUpMusicGroup(int nodeCount, String node1StateString) { books = null; - Distribution distribution = new Distribution(Distribution.getSimpleGroupConfig(2, nodeCount)); + Distribution distribution = new Distribution(getSimpleGroupConfig(2, nodeCount)); jsonWriter.setDefaultPathPrefix("/cluster/v2"); ContentCluster cluster = new ContentCluster("music", distribution.getNodes(), distribution); initializeCluster(cluster, distribution.getNodes()); @@ -167,4 +168,45 @@ public abstract class StateRestApiTest { " }\n" + "}"; } + + private static String getSimpleGroupConfig(int redundancy, int nodeCount) { + return getSimpleGroupConfig(redundancy, nodeCount, StorDistributionConfig.Disk_distribution.Enum.MODULO_BID); + } + + // TODO: Use config builder instead of creating raw: config + private static String getSimpleGroupConfig(int redundancy, int nodeCount, StorDistributionConfig.Disk_distribution.Enum diskDistribution) { + StringBuilder sb = new StringBuilder(); + sb.append("raw:redundancy ").append(redundancy).append("\n").append("group[4]\n"); + + int group = 0; + sb.append("group[" + group + "].index \"invalid\"\n") + .append("group[" + group + "].name \"invalid\"\n") + .append("group[" + group + "].partitions \"1|*\"\n"); + + ++group; + sb.append("group[" + group + "].index \"0\"\n") + .append("group[" + group + "].name \"east\"\n") + .append("group[" + group + "].partitions \"*\"\n"); + + ++group; + sb.append("group[" + group + "].index \"0.0\"\n") + .append("group[" + group + "].name \"g1\"\n") + .append("group[" + group + "].partitions \"*\"\n") + .append("group[" + group + "].nodes[").append((nodeCount + 1) / 2).append("]\n"); + for (int i=0; i<nodeCount; i += 2) { + sb.append("group[" + group + "].nodes[").append(i / 2).append("].index ").append(i).append("\n"); + } + + ++group; + sb.append("group[" + group + "].index \"0.1\"\n") + .append("group[" + group + "].name \"g2\"\n") + .append("group[" + group + "].partitions \"*\"\n") + .append("group[" + group + "].nodes[").append(nodeCount / 2).append("]\n"); + for (int i=1; i<nodeCount; i += 2) { + sb.append("group[" + group + "].nodes[").append(i / 2).append("].index ").append(i).append("\n"); + } + sb.append("disk_distribution ").append(diskDistribution.toString()).append("\n"); + return sb.toString(); + } + } diff --git a/docprocs/src/test/cfg/ilscripts.cfg b/docprocs/src/test/cfg/ilscripts.cfg index ce5e209458d..c58028f1056 100644 --- a/docprocs/src/test/cfg/ilscripts.cfg +++ b/docprocs/src/test/cfg/ilscripts.cfg @@ -9,5 +9,5 @@ ilscript[0].content[0] "input artist | attribute artist" ilscript[0].content[1] "input title | attribute title" ilscript[0].content[2] "input song | attribute song" ilscript[0].content[4] "input artist . " ". input title | index combined" -ilscript[0].content[5] "(input artist || "") . " ". (input title || "") | index combinedWithFallback" +ilscript[0].content[5] "(input artist || "") . " " . (input title || "") | index combinedWithFallback" diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExecutionValueExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExecutionValueExpression.java new file mode 100644 index 00000000000..a4c430125a4 --- /dev/null +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExecutionValueExpression.java @@ -0,0 +1,54 @@ +package com.yahoo.vespa.indexinglanguage.expressions; + +import com.yahoo.document.DataType; +import com.yahoo.document.DocumentType; +import com.yahoo.document.FieldPath; +import com.yahoo.vespa.objects.ObjectOperation; +import com.yahoo.vespa.objects.ObjectPredicate; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + +/** + * Returns the current execution value, that is the value passed to this expression. + * Referring to this explicitly is useful e.g to concatenate it to some other string: + * ... | input foo . " " . _ | ... + * + * @author bratseth + */ +public final class ExecutionValueExpression extends Expression { + + public ExecutionValueExpression() { + super(null); + } + + @Override + protected void doExecute(ExecutionContext context) { + // Noop: Set the output execution value to the current execution value + } + + @Override + protected void doVerify(VerificationContext context) {} + + @Override + public DataType createdOutputType() { + return UnresolvedDataType.INSTANCE; + } + + @Override + public String toString() { + return "_"; + } + + @Override + public boolean equals(Object obj) { + return obj instanceof ExecutionValueExpression; + } + + @Override + public int hashCode() { + return 9875876; + } + +} diff --git a/indexinglanguage/src/main/javacc/IndexingParser.jj b/indexinglanguage/src/main/javacc/IndexingParser.jj index 9f0b4733119..a039ad137ee 100644 --- a/indexinglanguage/src/main/javacc/IndexingParser.jj +++ b/indexinglanguage/src/main/javacc/IndexingParser.jj @@ -205,6 +205,7 @@ TOKEN : <ZCURVE: "zcurve"> | <TRUE: "true" > | <FALSE: "false" > | + <UNDERSCORE: "_"> | <IDENTIFIER: ["a"-"z","A"-"Z", "_"] (["a"-"z","A"-"Z","0"-"9","_","-"])*> } @@ -343,6 +344,7 @@ Expression value() : val = trimExp() | val = literalBoolExp() | val = zcurveExp() | + val = executionValueExp() | ( <LPAREN> val = statement() <RPAREN> { val = new ParenthesisExpression(val); } ) ) { return val; } } @@ -752,6 +754,12 @@ Expression zcurveExp() : { } { return new ZCurveExpression(); } } +Expression executionValueExp() : { } +{ + ( <UNDERSCORE> ) + { return new ExecutionValueExpression(); } +} + String identifier() : { String val; diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java index ce42f4e727f..98458dd965c 100644 --- a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java +++ b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java @@ -276,6 +276,45 @@ public class ScriptTestCase { } @Test + public void testArrayEmbedWithConcatenation() throws ParseException { + Map<String, Embedder> embedders = Map.of("emb1", new MockEmbedder("myDocument.mySparseTensor")); + + TensorType tensorType = TensorType.fromSpec("tensor(passage{}, d[4])"); + var expression = Expression.fromString("input myTextArray | for_each { input title . \" \" . _ } | embed | attribute 'mySparseTensor'", + new SimpleLinguistics(), + embedders); + + SimpleTestAdapter adapter = new SimpleTestAdapter(); + adapter.createField(new Field("myTextArray", new ArrayDataType(DataType.STRING))); + + var tensorField = new Field("mySparseTensor", new TensorDataType(tensorType)); + adapter.createField(tensorField); + + var array = new Array<StringFieldValue>(new ArrayDataType(DataType.STRING)); + array.add(new StringFieldValue("first")); + array.add(new StringFieldValue("second")); + adapter.setValue("myTextArray", array); + + var titleField = new Field("title", DataType.STRING); + adapter.createField(titleField); + adapter.setValue("title", new StringFieldValue("title1")); + + expression.setStatementOutput(new DocumentType("myDocument"), tensorField); + + // Necessary to resolve output type + VerificationContext verificationContext = new VerificationContext(adapter); + assertEquals(new TensorDataType(tensorType), expression.verify(verificationContext)); + + ExecutionContext context = new ExecutionContext(adapter); + context.setValue(array); + expression.execute(context); + assertTrue(adapter.values.containsKey("mySparseTensor")); + var sparseTensor = (TensorFieldValue)adapter.values.get("mySparseTensor"); + assertEquals(Tensor.from(tensorType, "{ '0':[116.0, 105.0, 116.0, 108.0], 1:[116.0, 105.0, 116.0, 108.0]}"), + sparseTensor.getTensor().get()); + } + + @Test public void testArrayEmbedToSparseTensor() throws ParseException { Map<String, Embedder> embedders = Map.of("emb1", new MockEmbedder("myDocument.mySparseTensor")); diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.cpp b/storage/src/vespa/storage/distributor/distributor_stripe.cpp index 782dd9d7e12..616fd77fdd7 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe.cpp +++ b/storage/src/vespa/storage/distributor/distributor_stripe.cpp @@ -127,6 +127,12 @@ void DistributorStripe::send_shutdown_abort_reply(const std::shared_ptr<api::Sto } void DistributorStripe::flush_and_close() { + // This function is called from a different thread than that of the stripe + // itself, so we need to take the same mutex to form a memory visibility pair. + // It is important that no flushing ever sends any _requests_, as these + // will most likely synchronously be bounced by the already shut down RPC + // layer, causing a deadlock when the response call chain arrives back here. + std::lock_guard lock(_external_message_mutex); for (auto& msg : _messageQueue) { if (!msg->getType().isReply()) { send_shutdown_abort_reply(msg); diff --git a/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java b/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java index bafe56e92ae..0e8e755c236 100644 --- a/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java +++ b/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java @@ -526,43 +526,4 @@ public class Distribution { return sb.toString(); } - public static String getSimpleGroupConfig(int redundancy, int nodeCount) { - return getSimpleGroupConfig(redundancy, nodeCount, StorDistributionConfig.Disk_distribution.Enum.MODULO_BID); - } - - private static String getSimpleGroupConfig(int redundancy, int nodeCount, StorDistributionConfig.Disk_distribution.Enum diskDistribution) { - StringBuilder sb = new StringBuilder(); - sb.append("raw:redundancy ").append(redundancy).append("\n").append("group[4]\n"); - - int group = 0; - sb.append("group[" + group + "].index \"invalid\"\n") - .append("group[" + group + "].name \"invalid\"\n") - .append("group[" + group + "].partitions \"1|*\"\n"); - - ++group; - sb.append("group[" + group + "].index \"0\"\n") - .append("group[" + group + "].name \"east\"\n") - .append("group[" + group + "].partitions \"*\"\n"); - - ++group; - sb.append("group[" + group + "].index \"0.0\"\n") - .append("group[" + group + "].name \"g1\"\n") - .append("group[" + group + "].partitions \"*\"\n") - .append("group[" + group + "].nodes[").append((nodeCount + 1) / 2).append("]\n"); - for (int i=0; i<nodeCount; i += 2) { - sb.append("group[" + group + "].nodes[").append(i / 2).append("].index ").append(i).append("\n"); - } - - ++group; - sb.append("group[" + group + "].index \"0.1\"\n") - .append("group[" + group + "].name \"g2\"\n") - .append("group[" + group + "].partitions \"*\"\n") - .append("group[" + group + "].nodes[").append(nodeCount / 2).append("]\n"); - for (int i=1; i<nodeCount; i += 2) { - sb.append("group[" + group + "].nodes[").append(i / 2).append("].index ").append(i).append("\n"); - } - sb.append("disk_distribution ").append(diskDistribution.toString()).append("\n"); - return sb.toString(); - } - } diff --git a/vdslib/src/main/java/com/yahoo/vdslib/state/ClusterState.java b/vdslib/src/main/java/com/yahoo/vdslib/state/ClusterState.java index e8af249422f..4bf305e65e0 100644 --- a/vdslib/src/main/java/com/yahoo/vdslib/state/ClusterState.java +++ b/vdslib/src/main/java/com/yahoo/vdslib/state/ClusterState.java @@ -23,7 +23,7 @@ public class ClusterState implements Cloneable { private static final NodeState DEFAULT_DISTRIBUTOR_DOWN_NODE_STATE = new NodeState(NodeType.DISTRIBUTOR, State.DOWN); /** - * Maintains a bitset where all non-down nodes have a bit set. All nodes that differs from defaultUp + * Maintains a bitset where all non-down nodes have a bit set. All nodes that differ from defaultUp * and defaultDown are store explicit in a hash map. */ private static class Nodes { @@ -102,19 +102,19 @@ public class ClusterState implements Cloneable { } } - boolean similarToImpl(Nodes other, final NodeStateCmp nodeStateCmp) { + boolean notSimilarTo(Nodes other, final NodeStateCmp nodeStateCmp) { // TODO verify behavior of C++ impl against this - if (logicalNodeCount != other.logicalNodeCount) return false; - if (type != other.type) return false; - if ( ! upNodes.equals(other.upNodes)) return false; + if (logicalNodeCount != other.logicalNodeCount) return true; + if (type != other.type) return true; + if ( ! upNodes.equals(other.upNodes)) return true; for (Integer node : unionNodeSetWith(other.nodeStates.keySet())) { final NodeState lhs = nodeStates.get(node); final NodeState rhs = other.nodeStates.get(node); if (!nodeStateCmp.similar(type, lhs, rhs)) { - return false; + return true; } } - return true; + return false; } private Set<Integer> unionNodeSetWith(final Set<Integer> otherNodes) { @@ -144,8 +144,7 @@ public class ClusterState implements Cloneable { @Override public boolean equals(Object obj) { - if (! (obj instanceof Nodes)) return false; - Nodes b = (Nodes) obj; + if (! (obj instanceof Nodes b)) return false; if (logicalNodeCount != b.logicalNodeCount) return false; if (type != b.type) return false; if (!upNodes.equals(b.upNodes)) return false; @@ -218,8 +217,7 @@ public class ClusterState implements Cloneable { @Override public boolean equals(Object o) { - if (!(o instanceof ClusterState)) { return false; } - ClusterState other = (ClusterState) o; + if (!(o instanceof ClusterState other)) { return false; } if (version != other.version || !state.equals(other.state) || distributionBits != other.distributionBits @@ -242,8 +240,7 @@ public class ClusterState implements Cloneable { } public boolean similarTo(Object o) { - if (!(o instanceof ClusterState)) { return false; } - final ClusterState other = (ClusterState) o; + if (!(o instanceof final ClusterState other)) { return false; } return similarToImpl(other, this::normalizedNodeStateSimilarTo); } @@ -265,19 +262,15 @@ public class ClusterState implements Cloneable { if (!metaInformationSimilarTo(other)) { return false; } - if ( !distributorNodes.similarToImpl(other.distributorNodes, nodeStateCmp)) return false; - if ( !storageNodes.similarToImpl(other.storageNodes, nodeStateCmp)) return false; + if (distributorNodes.notSimilarTo(other.distributorNodes, nodeStateCmp)) return false; + if (storageNodes.notSimilarTo(other.storageNodes, nodeStateCmp)) return false; return true; } private boolean metaInformationSimilarTo(final ClusterState other) { - if (version != other.version || !state.equals(other.state)) { - return false; - } - if (distributionBits != other.distributionBits) { - return false; - } - return true; + return version == other.version + && state.equals(other.state) + && distributionBits == other.distributionBits; } private boolean normalizedNodeStateSimilarTo(final NodeType nodeType, final NodeState lhs, final NodeState rhs) { @@ -356,8 +349,8 @@ public class ClusterState implements Cloneable { break; case 'v': if (key.equals("version")) { - try{ - setVersion(Integer.valueOf(value)); + try { + setVersion(Integer.parseInt(value)); } catch (Exception e) { throw new ParseException("Illegal version '" + value + "'. Must be an integer, in state: " + serialized, 0); } @@ -382,7 +375,7 @@ public class ClusterState implements Cloneable { if (dot < 0) { int nodeCount; try{ - nodeCount = Integer.valueOf(value); + nodeCount = Integer.parseInt(value); } catch (Exception e) { throw new ParseException("Illegal node count '" + value + "' in state: " + serialized, 0); } @@ -392,9 +385,9 @@ public class ClusterState implements Cloneable { int dot2 = key.indexOf('.', dot + 1); Node node; if (dot2 < 0) { - node = new Node(nodeType, Integer.valueOf(key.substring(dot + 1))); + node = new Node(nodeType, Integer.parseInt(key.substring(dot + 1))); } else { - node = new Node(nodeType, Integer.valueOf(key.substring(dot + 1, dot2))); + node = new Node(nodeType, Integer.parseInt(key.substring(dot + 1, dot2))); } if (node.getIndex() >= getNodeCount(nodeType)) { throw new ParseException("Cannot index " + nodeType + " node " + node.getIndex() + " of " + getNodeCount(nodeType) + " in state: " + serialized, 0); diff --git a/vdslib/src/main/java/com/yahoo/vdslib/state/Node.java b/vdslib/src/main/java/com/yahoo/vdslib/state/Node.java index 0147ab0d056..445472bba4c 100644 --- a/vdslib/src/main/java/com/yahoo/vdslib/state/Node.java +++ b/vdslib/src/main/java/com/yahoo/vdslib/state/Node.java @@ -18,7 +18,7 @@ public class Node implements Comparable<Node> { int dot = serialized.lastIndexOf('.'); if (dot < 0) throw new IllegalArgumentException("Not a legal node string '" + serialized + "'."); type = NodeType.get(serialized.substring(0, dot)); - index = Integer.valueOf(serialized.substring(dot + 1)); + index = Integer.parseInt(serialized.substring(dot + 1)); } public static Node ofStorage(int index) { @@ -52,8 +52,7 @@ public class Node implements Comparable<Node> { @Override public boolean equals(Object o) { - if (!(o instanceof Node)) return false; - Node n = (Node) o; + if (!(o instanceof Node n)) return false; return (type.equals(n.type) && index == n.index); } diff --git a/vdslib/src/main/java/com/yahoo/vdslib/state/NodeState.java b/vdslib/src/main/java/com/yahoo/vdslib/state/NodeState.java index 9f6efd7b5e0..4bb27e8599e 100644 --- a/vdslib/src/main/java/com/yahoo/vdslib/state/NodeState.java +++ b/vdslib/src/main/java/com/yahoo/vdslib/state/NodeState.java @@ -49,8 +49,7 @@ public class NodeState implements Cloneable { } public boolean equals(Object o) { - if (!(o instanceof NodeState)) { return false; } - NodeState ns = (NodeState) o; + if (!(o instanceof NodeState ns)) { return false; } // Note that 'description' is not considered as it carries semantics. if (state != ns.state || Math.abs(capacity - ns.capacity) > 0.0000000001 @@ -235,7 +234,7 @@ public class NodeState implements Cloneable { if (key.length() > 1) break; if (type != null && !type.equals(NodeType.STORAGE)) break; try{ - newState.setCapacity(Float.valueOf(value)); + newState.setCapacity(Float.parseFloat(value)); } catch (Exception e) { throw new ParseException("Illegal capacity '" + value + "'. Capacity must be a positive floating point number", 0); } @@ -243,7 +242,7 @@ public class NodeState implements Cloneable { case 'i': if (key.length() > 1) break; try{ - newState.setInitProgress(Float.valueOf(value)); + newState.setInitProgress(Float.parseFloat(value)); } catch (Exception e) { throw new ParseException("Illegal init progress '" + value + "'. Init progress must be a floating point number from 0.0 to 1.0", 0); } @@ -251,7 +250,7 @@ public class NodeState implements Cloneable { case 't': if (key.length() > 1) break; try{ - newState.setStartTimestamp(Long.valueOf(value)); + newState.setStartTimestamp(Long.parseLong(value)); if (newState.getStartTimestamp() < 0) throw new Exception(); } catch (Exception e) { throw new ParseException("Illegal start timestamp " + value + ". Start timestamp must be 0 or a positive long.", 0); @@ -265,8 +264,8 @@ public class NodeState implements Cloneable { if (type != null && !type.equals(NodeType.STORAGE)) break; int size = 0; if (key.length() == 1) { - try{ - size = Integer.valueOf(value); + try { + size = Integer.parseInt(value); } catch (Exception e) { throw new ParseException("Invalid disk count '" + value + "'. Need a positive integer value", 0); } @@ -277,7 +276,7 @@ public class NodeState implements Cloneable { int endp = key.indexOf('.', 2); String indexStr = (endp < 0 ? key.substring(2) : key.substring(2, endp)); try{ - diskIndex = Integer.valueOf(indexStr); + diskIndex = Integer.parseInt(indexStr); } catch (Exception e) { throw new ParseException("Invalid disk index '" + indexStr + "'. need a positive integer value", 0); } diff --git a/vdslib/src/main/java/com/yahoo/vdslib/state/NodeType.java b/vdslib/src/main/java/com/yahoo/vdslib/state/NodeType.java index 1472525a1ea..8e7bce3503c 100644 --- a/vdslib/src/main/java/com/yahoo/vdslib/state/NodeType.java +++ b/vdslib/src/main/java/com/yahoo/vdslib/state/NodeType.java @@ -1,13 +1,15 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vdslib.state; +import java.util.List; + public enum NodeType { STORAGE("storage"), DISTRIBUTOR("distributor"); private final String serializeAs; - private NodeType(String serializeAs) { + NodeType(String serializeAs) { this.serializeAs = serializeAs; } @@ -16,17 +18,12 @@ public enum NodeType { } public static NodeType get(String serialized) { - for(NodeType type : values()) { + for (NodeType type : values()) { if (type.serializeAs.equals(serialized)) return type; } throw new IllegalArgumentException("Unknown node type '" + serialized + "'. Legal values are 'storage' and 'distributor'."); } - public static NodeType[] getTypes() { - NodeType types[] = new NodeType[2]; - types[0] = STORAGE; - types[1] = DISTRIBUTOR; - return types; - } + public static List<NodeType> getTypes() { return List.of(STORAGE, DISTRIBUTOR); } } |