diff options
63 files changed, 924 insertions, 521 deletions
diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index 75c837cc593..d2188e37123 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -35,7 +35,7 @@ <junit5.platform.version>1.7.0</junit5.platform.version> <org.lz4.version>1.7.1</org.lz4.version> <org.json.version>20090211</org.json.version><!-- TODO Vespa 8: remove as provided dependency --> - <slf4j.version>1.7.5</slf4j.version> + <slf4j.version>1.7.30</slf4j.version> <tensorflow.version>1.12.0</tensorflow.version> <xml-apis.version>1.4.01</xml-apis.version> @@ -94,7 +94,7 @@ <include>com.sun.activation:javax.activation:[1.2.0]:jar:provided</include> <include>com.sun.xml.bind:jaxb-core:[${jaxb.version}]:jar:provided</include> <include>com.sun.xml.bind:jaxb-impl:[${jaxb.version}]:jar:provided</include> - <include>commons-logging:commons-logging:[1.1.3]:jar:provided</include> + <include>commons-logging:commons-logging:[1.2]:jar:provided</include> <include>jakarta.activation:jakarta.activation-api:[1.2.1]:jar:provided</include> <include>jakarta.xml.bind:jakarta.xml.bind-api:[2.3.2]:jar:provided</include> <include>javax.annotation:javax.annotation-api:[${javax.annotation-api.version}]:jar:provided</include> 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 8cdaa33d521..41c7c985c0b 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 @@ -177,15 +177,17 @@ public class ContentCluster { * @param condition the upgrade condition * @param oldState the old/current wanted state * @param newState state wanted to be set @return NodeUpgradePrechecker.Response + * @param inMoratorium whether the CC is in moratorium */ public NodeStateChangeChecker.Result calculateEffectOfNewState( Node node, ClusterState clusterState, SetUnitStateRequest.Condition condition, - NodeState oldState, NodeState newState) { + NodeState oldState, NodeState newState, boolean inMoratorium) { NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( distribution.getRedundancy(), new HierarchicalGroupVisitingAdapter(distribution), - clusterInfo + clusterInfo, + inMoratorium ); 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 bdc8b8497aa..2a07f9ac300 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 @@ -89,6 +89,7 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd private final MetricUpdater metricUpdater; private boolean isMaster = false; + private boolean inMasterMoratorium = false; private boolean isStateGatherer = false; private long firstAllowedStateBroadcast = Long.MAX_VALUE; private long tickStartTime = Long.MAX_VALUE; @@ -712,6 +713,12 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd if ((currentTime >= firstAllowedStateBroadcast || cluster.allStatesReported()) && currentTime >= nextStateSendTime) { + if (inMasterMoratorium) { + log.fine(currentTime < firstAllowedStateBroadcast ? + "Master moratorium complete: all nodes have reported in" : + "Master moratorium complete: timed out waiting for all nodes to report in"); + inMasterMoratorium = false; + } if (currentTime < firstAllowedStateBroadcast) { log.log(Level.FINE, "Not set to broadcast states just yet, but as we have gotten info from all nodes we can do so safely."); // Reset timer to only see warning once. @@ -780,7 +787,12 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd context.cluster = cluster; context.currentConsolidatedState = consolidatedClusterState(); context.publishedClusterStateBundle = stateVersionTracker.getVersionedClusterStateBundle(); - context.masterInfo = masterElectionHandler; + context.masterInfo = new MasterInterface() { + @Override public boolean isMaster() { return isMaster; } + @Override public Integer getMaster() { return masterElectionHandler.getMaster(); } + @Override public boolean inMasterMoratorium() { return inMasterMoratorium; } + }; + context.nodeStateOrHostInfoChangeHandler = this; context.nodeAddedOrRemovedListener = this; return context; @@ -1078,11 +1090,12 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd + stateVersionTracker.getCurrentVersion() + " to be in line.", timer.getCurrentTimeInMillis())); long currentTime = timer.getCurrentTimeInMillis(); firstAllowedStateBroadcast = currentTime + options.minTimeBeforeFirstSystemStateBroadcast; + isMaster = true; + inMasterMoratorium = true; log.log(Level.FINE, "At time " + currentTime + " we set first system state broadcast time to be " + options.minTimeBeforeFirstSystemStateBroadcast + " ms after at time " + firstAllowedStateBroadcast + "."); didWork = true; } - isMaster = true; if (wantedStateChanged) { database.saveWantedStates(databaseContext); wantedStateChanged = false; @@ -1102,6 +1115,7 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd } wantedStateChanged = false; isMaster = false; + inMasterMoratorium = false; } public void run() { 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 528a9d79a7b..c841f4741ba 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 @@ -94,7 +94,9 @@ public class FleetControllerOptions implements Cloneable { * Minimum time to pass (in milliseconds) before broadcasting our first systemstate. Set small in unit tests, * but should be a few seconds in a real system to prevent new nodes taking over from disturbing the system by * putting out a different systemstate just because all nodes don't answer witihin a single cycle. - * If all nodes have reported before this time, the min time is ignored and system state is broadcasted. + * The cluster state is allowed to be broadcasted before this time if all nodes have successfully + * reported their state in Slobrok and getnodestate. This value should typically be in the order of + * maxSlobrokDisconnectGracePeriod and nodeStateRequestTimeoutMS. */ public long minTimeBeforeFirstSystemStateBroadcast = 0; diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MasterElectionHandler.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MasterElectionHandler.java index 0dd26026c5d..2c03520ec01 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MasterElectionHandler.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MasterElectionHandler.java @@ -1,10 +1,10 @@ // Copyright Verizon Media. 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.logging.Level; import com.yahoo.vespa.clustercontroller.core.database.DatabaseHandler; import java.util.Map; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -69,6 +69,11 @@ public class MasterElectionHandler implements MasterInterface { } @Override + public boolean inMasterMoratorium() { + return false; + } + + @Override public Integer getMaster() { // If too few followers there can be no master if (2 * followers <= totalCount) { diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MasterInterface.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MasterInterface.java index c1d2f829a85..59e5bdd9db2 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MasterInterface.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MasterInterface.java @@ -5,5 +5,6 @@ public interface MasterInterface { boolean isMaster(); Integer getMaster(); + boolean inMasterMoratorium(); } 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 918f01eef16..dd33646dd31 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 @@ -32,14 +32,17 @@ public class NodeStateChangeChecker { private final int requiredRedundancy; private final HierarchicalGroupVisiting groupVisiting; private final ClusterInfo clusterInfo; + private final boolean inMoratorium; public NodeStateChangeChecker( int requiredRedundancy, HierarchicalGroupVisiting groupVisiting, - ClusterInfo clusterInfo) { + ClusterInfo clusterInfo, + boolean inMoratorium) { this.requiredRedundancy = requiredRedundancy; this.groupVisiting = groupVisiting; this.clusterInfo = clusterInfo; + this.inMoratorium = inMoratorium; } public static class Result { @@ -94,6 +97,10 @@ public class NodeStateChangeChecker { return Result.allowSettingOfWantedState(); } + if (inMoratorium) { + return Result.createDisallowed("Master cluster controller is bootstrapping and in moratorium"); + } + if (condition != SetUnitStateRequest.Condition.SAFE) { return Result.createDisallowed("Condition not implemented: " + condition.name()); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java index 94b1a9e1fbc..dcd7a176aa7 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core.restapiv2.requests; -import java.util.logging.Level; import com.yahoo.time.TimeBudget; import com.yahoo.vdslib.state.ClusterState; import com.yahoo.vdslib.state.Node; @@ -26,6 +25,7 @@ import java.time.Instant; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.logging.Level; import java.util.logging.Logger; public class SetNodeStateRequest extends Request<SetResponse> { @@ -64,6 +64,7 @@ public class SetNodeStateRequest extends Request<SetResponse> { id.getNode(), context.nodeStateOrHostInfoChangeHandler, context.currentConsolidatedState, + context.masterInfo.inMasterMoratorium(), probe); } @@ -104,6 +105,7 @@ public class SetNodeStateRequest extends Request<SetResponse> { Node node, NodeStateOrHostInfoChangeHandler stateListener, ClusterState currentClusterState, + boolean inMasterMoratorium, boolean probe) throws StateRestApiException { if ( ! cluster.hasConfiguredNode(node.getIndex())) { throw new MissingIdException(cluster.getName(), node); @@ -115,7 +117,7 @@ public class SetNodeStateRequest extends Request<SetResponse> { NodeState wantedState = nodeInfo.getUserWantedState(); NodeState newWantedState = getRequestedNodeState(newStates, node); NodeStateChangeChecker.Result result = cluster.calculateEffectOfNewState( - node, currentClusterState, condition, wantedState, newWantedState); + node, currentClusterState, condition, wantedState, newWantedState, inMasterMoratorium); log.log(Level.FINE, "node=" + node + " current-cluster-state=" + currentClusterState + // Includes version in output format 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 d7820722887..a855c39156f 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 @@ -72,6 +72,7 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> { node, context.nodeStateOrHostInfoChangeHandler, context.currentConsolidatedState, + context.masterInfo.inMasterMoratorium(), probe); if (!setResponse.getWasModified()) { diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java index c3090a5e832..ef9885c7cb4 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java @@ -23,5 +23,6 @@ public interface WantedStateSetter { Node node, NodeStateOrHostInfoChangeHandler stateListener, ClusterState currentClusterState, + boolean inMasterMoratorium, boolean probe) throws StateRestApiException; } 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 41284cc95d0..5e3dbbe713b 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 @@ -57,7 +57,7 @@ public class NodeStateChangeCheckerTest { } private NodeStateChangeChecker createChangeChecker(ContentCluster cluster) { - return new NodeStateChangeChecker(requiredRedundancy, visitor -> {}, cluster.clusterInfo()); + return new NodeStateChangeChecker(requiredRedundancy, visitor -> {}, cluster.clusterInfo(), false); } private ContentCluster createCluster(Collection<ConfiguredNode> nodes) { @@ -114,10 +114,23 @@ public class NodeStateChangeCheckerTest { } @Test + public void testDeniedInMoratorium() { + ContentCluster cluster = createCluster(createNodes(4)); + NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( + requiredRedundancy, visitor -> {}, cluster.clusterInfo(), true); + NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( + new Node(NodeType.STORAGE, 10), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, + UP_NODE_STATE, MAINTENANCE_NODE_STATE); + assertFalse(result.settingWantedStateIsAllowed()); + assertFalse(result.wantedStateAlreadySet()); + assertThat(result.getReason(), is("Master cluster controller is bootstrapping and in moratorium")); + } + + @Test public void testUnknownStorageNode() { ContentCluster cluster = createCluster(createNodes(4)); NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( - requiredRedundancy, visitor -> {}, cluster.clusterInfo()); + requiredRedundancy, visitor -> {}, cluster.clusterInfo(), false); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( new Node(NodeType.STORAGE, 10), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); @@ -149,7 +162,7 @@ public class NodeStateChangeCheckerTest { // We should then be denied setting storage node 1 safely to maintenance. NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( - requiredRedundancy, visitor -> {}, cluster.clusterInfo()); + requiredRedundancy, visitor -> {}, cluster.clusterInfo(), false); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, clusterStateWith3Down, SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerMock.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerMock.java index 3a859d5a27a..66ad1305878 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerMock.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/ClusterControllerMock.java @@ -31,6 +31,11 @@ public class ClusterControllerMock implements RemoteClusterControllerTaskSchedul } @Override + public boolean inMasterMoratorium() { + return false; + } + + @Override public Integer getMaster() { return fleetControllerMaster; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java index 03fdb15971f..712c34eae4b 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java @@ -572,7 +572,7 @@ public class SetNodeStateTest extends StateRestApiTest { new SetUnitStateRequestImpl("music/storage/1").setNewState("user", "maintenance", "whatever reason."), wantedStateSetter); SetResponse response = new SetResponse("some reason", wasModified); - when(wantedStateSetter.set(any(), any(), any(), any(), any(), any(), anyBoolean())).thenReturn(response); + when(wantedStateSetter.set(any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean())).thenReturn(response); RemoteClusterControllerTask.Context context = mock(RemoteClusterControllerTask.Context.class); MasterInterface masterInterface = mock(MasterInterface.class); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java index 8c6ef1d70d8..eef8a4e34d5 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Optional; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -39,6 +40,7 @@ public class SetNodeStateRequestTest { private final Node storageNode = new Node(NodeType.STORAGE, NODE_INDEX); private final NodeStateOrHostInfoChangeHandler stateListener = mock(NodeStateOrHostInfoChangeHandler.class); private final ClusterState currentClusterState = mock(ClusterState.class); + private boolean inMasterMoratorium = false; private boolean probe = false; @Before @@ -127,7 +129,7 @@ public class SetNodeStateRequestTest { when(unitState.getId()).thenReturn(wantedStateString); when(unitState.getReason()).thenReturn(REASON); - when(cluster.calculateEffectOfNewState(any(), any(), any(), any(), any())).thenReturn(result); + when(cluster.calculateEffectOfNewState(any(), any(), any(), any(), any(), anyBoolean())).thenReturn(result); when(storageNodeInfo.isStorage()).thenReturn(storageNode.getType() == NodeType.STORAGE); when(storageNodeInfo.getNodeIndex()).thenReturn(storageNode.getIndex()); @@ -173,6 +175,7 @@ public class SetNodeStateRequestTest { storageNode, stateListener, currentClusterState, + inMasterMoratorium, probe); } }
\ No newline at end of file diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/DictionaryProcessor.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/DictionaryProcessor.java index 92ceb8e5e44..fd567ec2d54 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/DictionaryProcessor.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/DictionaryProcessor.java @@ -27,7 +27,7 @@ public class DictionaryProcessor extends Processor { if (dictionary == null) continue; Attribute attribute = field.getAttribute(); - if (attribute.getDataType() instanceof NumericDataType ) { + if (attribute.getDataType().getPrimitiveType() instanceof NumericDataType ) { if (attribute.isFastSearch()) { attribute.setDictionary(dictionary); } else { diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/DictionaryTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/DictionaryTestCase.java index 256858b372e..ba51caca0f7 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/DictionaryTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/DictionaryTestCase.java @@ -50,12 +50,13 @@ public class DictionaryTestCase { void verifyNumericDictionaryControl(Dictionary.Type expected, AttributesConfig.Attribute.Dictionary.Type.Enum expectedConfig, + String type, String ... cfg) throws ParseException { String def = TestUtil.joinLines( "search test {", " document test {", - " field n1 type int {", + " field n1 type " + type + " {", " indexing: summary | attribute", " attribute:fast-search", TestUtil.joinLines(cfg), @@ -72,18 +73,35 @@ public class DictionaryTestCase { public void testNumericBtreeSettings() throws ParseException { verifyNumericDictionaryControl(Dictionary.Type.BTREE, AttributesConfig.Attribute.Dictionary.Type.BTREE, + "int", "dictionary:btree"); } @Test public void testNumericHashSettings() throws ParseException { verifyNumericDictionaryControl(Dictionary.Type.HASH, AttributesConfig.Attribute.Dictionary.Type.HASH, + "int", "dictionary:hash"); } @Test public void testNumericBtreeAndHashSettings() throws ParseException { verifyNumericDictionaryControl(Dictionary.Type.BTREE_AND_HASH, AttributesConfig.Attribute.Dictionary.Type.BTREE_AND_HASH, + "int", + "dictionary:btree", "dictionary:hash"); + } + @Test + public void testNumericArrayBtreeAndHashSettings() throws ParseException { + verifyNumericDictionaryControl(Dictionary.Type.BTREE_AND_HASH, + AttributesConfig.Attribute.Dictionary.Type.BTREE_AND_HASH, + "array<int>", + "dictionary:btree", "dictionary:hash"); + } + @Test + public void testNumericWSetBtreeAndHashSettings() throws ParseException { + verifyNumericDictionaryControl(Dictionary.Type.BTREE_AND_HASH, + AttributesConfig.Attribute.Dictionary.Type.BTREE_AND_HASH, + "weightedset<int>", "dictionary:btree", "dictionary:hash"); } @Test diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java index 7f563b876a7..0548bc7520f 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java @@ -316,4 +316,6 @@ public class NodeResources { return value; } + public static NodeResources zero() { return new NodeResources(0, 0, 0, 0); } + } diff --git a/configdefinitions/src/vespa/fleetcontroller.def b/configdefinitions/src/vespa/fleetcontroller.def index d2d746363f0..b6ea50507ca 100644 --- a/configdefinitions/src/vespa/fleetcontroller.def +++ b/configdefinitions/src/vespa/fleetcontroller.def @@ -112,11 +112,12 @@ min_storage_up_ratio double default=0.01 cycle_wait_time double default=0.1 ## Minimum time to pass in seconds before broadcasting our first systemstate as -## a new fleetcontroller. (Will broadcast earlier than this if we have gathered -## state from all before this). To prevent disturbance when taking over as +## a new fleetcontroller. Will broadcast earlier than this if we have gathered +## state from all before this. To prevent disturbance when taking over as ## fleetcontroller, give nodes a bit of time to answer so we dont temporarily -## report nodes as down. -min_time_before_first_system_state_broadcast double default=5.0 +## report nodes as down. See also max_slobrok_disconnect_grace_period and +## get_node_state_request_timeout. +min_time_before_first_system_state_broadcast double default=30.0 ## Request timeout of node state requests. Keeping a high timeout allows us to ## always have a pending operation with very low cost. Keeping a low timeout is diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index adbe15ff94d..8392f370dda 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -78,7 +78,9 @@ import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.stats.LockStats; import com.yahoo.vespa.curator.stats.ThreadLockStats; import com.yahoo.vespa.defaults.Defaults; +import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.orchestrator.Orchestrator; @@ -144,6 +146,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye private final Metric metric; private final SecretStoreValidator secretStoreValidator; private final ClusterReindexingStatusClient clusterReindexingStatusClient; + private final BooleanFlag waitForAllConfigServersWhenDeletingApplication; @Inject public ApplicationRepository(TenantRepository tenantRepository, @@ -155,7 +158,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye Orchestrator orchestrator, TesterClient testerClient, Metric metric, - SecretStore secretStore) { + SecretStore secretStore, + FlagSource flagSource) { this(tenantRepository, hostProvisionerProvider.getHostProvisioner(), infraDeployerProvider.getInfraDeployer(), @@ -168,7 +172,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye testerClient, metric, new SecretStoreValidator(secretStore), - new DefaultClusterReindexingStatusClient()); + new DefaultClusterReindexingStatusClient(), + flagSource); } private ApplicationRepository(TenantRepository tenantRepository, @@ -183,7 +188,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye TesterClient testerClient, Metric metric, SecretStoreValidator secretStoreValidator, - ClusterReindexingStatusClient clusterReindexingStatusClient) { + ClusterReindexingStatusClient clusterReindexingStatusClient, + FlagSource flagSource) { this.tenantRepository = Objects.requireNonNull(tenantRepository); this.hostProvisioner = Objects.requireNonNull(hostProvisioner); this.infraDeployer = Objects.requireNonNull(infraDeployer); @@ -197,6 +203,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye this.metric = Objects.requireNonNull(metric); this.secretStoreValidator = Objects.requireNonNull(secretStoreValidator); this.clusterReindexingStatusClient = clusterReindexingStatusClient; + this.waitForAllConfigServersWhenDeletingApplication = Flags.WAIT_FOR_ALL_CONFIG_SERVERS_WHEN_DELETING_APPLICATION.bindTo(flagSource); } public static class Builder { @@ -287,7 +294,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye testerClient, metric, secretStoreValidator, - ClusterReindexingStatusClient.DUMMY_INSTANCE); + ClusterReindexingStatusClient.DUMMY_INSTANCE, + flagSource); } } @@ -516,6 +524,9 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } Curator curator = tenantRepository.getCurator(); + Optional<CompletionWaiter> waiter = Optional.empty(); + if (waitForAllConfigServersWhenDeletingApplication.value()) + waiter = Optional.of(tenantApplications.createRemoveApplicationWaiter(applicationId)); transaction.add(new ContainerEndpointsCache(tenant.getPath(), curator).delete(applicationId)); // TODO: Not unit tested // Delete any application roles transaction.add(new ApplicationRolesStore(curator, tenant.getPath()).delete(applicationId)); @@ -532,6 +543,10 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } else { transaction.commit(); } + + // Wait for app being removed on other servers + waiter.ifPresent(w -> w.awaitCompletion(Duration.ofSeconds(30))); + return true; } finally { applicationTransaction.ifPresent(ApplicationTransaction::close); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java index f9c1f831049..c7c07f0ae33 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java @@ -24,6 +24,7 @@ import com.yahoo.vespa.config.server.monitoring.MetricUpdater; import com.yahoo.vespa.config.server.monitoring.Metrics; import com.yahoo.vespa.config.server.rpc.ConfigResponseFactory; import com.yahoo.vespa.config.server.tenant.TenantRepository; +import com.yahoo.vespa.curator.CompletionTimeoutException; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.curator.transaction.CuratorTransaction; @@ -33,6 +34,8 @@ import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; import java.nio.file.Files; import java.nio.file.Paths; import java.time.Clock; +import java.time.Duration; +import java.time.Instant; import java.util.Collection; import java.util.LinkedHashSet; import java.util.List; @@ -43,6 +46,8 @@ import java.util.concurrent.ExecutorService; import java.util.logging.Level; import java.util.logging.Logger; +import static com.yahoo.vespa.config.server.tenant.TenantRepository.getBarriersPath; +import static com.yahoo.vespa.curator.Curator.CompletionWaiter; import static java.util.stream.Collectors.toSet; /** @@ -55,6 +60,7 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica private static final Logger log = Logger.getLogger(TenantApplications.class.getName()); + private final Curator curator; private final ApplicationCuratorDatabase database; private final Curator.DirectoryCache directoryCache; private final Executor zkWatcherExecutor; @@ -67,11 +73,13 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica private final MetricUpdater tenantMetricUpdater; private final Clock clock; private final TenantFileSystemDirs tenantFileSystemDirs; + private final ConfigserverConfig configserverConfig; public TenantApplications(TenantName tenant, Curator curator, StripedExecutor<TenantName> zkWatcherExecutor, ExecutorService zkCacheExecutor, Metrics metrics, ReloadListener reloadListener, ConfigserverConfig configserverConfig, HostRegistry hostRegistry, TenantFileSystemDirs tenantFileSystemDirs, Clock clock) { + this.curator = curator; this.database = new ApplicationCuratorDatabase(tenant, curator); this.tenant = tenant; this.zkWatcherExecutor = command -> zkWatcherExecutor.execute(tenant, command); @@ -85,6 +93,7 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica this.hostRegistry = hostRegistry; this.tenantFileSystemDirs = tenantFileSystemDirs; this.clock = clock; + this.configserverConfig = configserverConfig; } // For testing only @@ -252,21 +261,23 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica } } + // Note: Assumes that caller already holds the application lock + // (when getting event from zookeeper to remove application, + // the lock should be held by the thread that causes the event to happen) public void removeApplication(ApplicationId applicationId) { - try (Lock lock = lock(applicationId)) { - if (exists(applicationId)) { - log.log(Level.INFO, "Tried removing application " + applicationId + ", but it seems to have been deployed again"); - return; - } + if (exists(applicationId)) { + log.log(Level.INFO, "Tried removing application " + applicationId + ", but it seems to have been deployed again"); + return; + } - if (hasApplication(applicationId)) { - applicationMapper.remove(applicationId); - hostRegistry.removeHostsForKey(applicationId); - reloadListenersOnRemove(applicationId); - tenantMetricUpdater.setApplications(applicationMapper.numApplications()); - metrics.removeMetricUpdater(Metrics.createDimensions(applicationId)); - log.log(Level.INFO, "Application removed: " + applicationId); - } + if (hasApplication(applicationId)) { + applicationMapper.remove(applicationId); + hostRegistry.removeHostsForKey(applicationId); + reloadListenersOnRemove(applicationId); + tenantMetricUpdater.setApplications(applicationMapper.numApplications()); + metrics.removeMetricUpdater(Metrics.createDimensions(applicationId)); + getRemoveApplicationWaiter(applicationId).notifyCompletion(); + log.log(Level.INFO, "Application removed: " + applicationId); } } @@ -277,7 +288,9 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica public void removeApplicationsExcept(Set<ApplicationId> applications) { for (ApplicationId activeApplication : applicationMapper.listApplicationIds()) { if ( ! applications.contains(activeApplication)) { - removeApplication(activeApplication); + try (var applicationLock = lock(activeApplication)){ + removeApplication(activeApplication); + } } } } @@ -409,4 +422,141 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica public TenantFileSystemDirs getTenantFileSystemDirs() { return tenantFileSystemDirs; } + public CompletionWaiter createRemoveApplicationWaiter(ApplicationId applicationId) { + return RemoveApplicationWaiter.createAndInitialize(curator, applicationId, configserverConfig.serverId()); + } + + public CompletionWaiter getRemoveApplicationWaiter(ApplicationId applicationId) { + return RemoveApplicationWaiter.create(curator, applicationId, configserverConfig.serverId()); + } + + /** + * + * Waiter for removing application. Will wait for some time for all servers to remove application, + * but will accept majority of servers to have removed app if it takes a long time. + */ + static class RemoveApplicationWaiter implements CompletionWaiter { + + private static final java.util.logging.Logger log = Logger.getLogger(RemoveApplicationWaiter.class.getName()); + private static final Duration waitForAllDefault = Duration.ofSeconds(5); + + private final Curator curator; + private final Path barrierPath; + private final Path waiterNode; + private final Duration waitForAll; + private final Clock clock = Clock.systemUTC(); + + RemoveApplicationWaiter(Curator curator, ApplicationId applicationId, String serverId) { + this(curator, applicationId, serverId, waitForAllDefault); + } + + RemoveApplicationWaiter(Curator curator, ApplicationId applicationId, String serverId, Duration waitForAll) { + this.barrierPath = TenantRepository.getBarriersPath().append(applicationId.tenant().value()) + .append("delete-application") + .append(applicationId.serializedForm()); + this.waiterNode = barrierPath.append(serverId); + this.curator = curator; + this.waitForAll = waitForAll; + } + + @Override + public void awaitCompletion(Duration timeout) { + List<String> respondents; + try { + respondents = awaitInternal(timeout); + } catch (Exception e) { + throw new RuntimeException(e); + } + if (respondents.size() < barrierMemberCount()) { + throw new CompletionTimeoutException("Timed out waiting for peer config servers to remove application " + + "(waited for barrier " + barrierPath + ")." + + "Got response from " + respondents + ", but need response from " + + "at least " + barrierMemberCount() + " server(s). " + + "Timeout passed as argument was " + timeout.toMillis() + " ms"); + } + } + + private List<String> awaitInternal(Duration timeout) throws Exception { + Instant startTime = clock.instant(); + Instant endTime = startTime.plus(timeout); + Instant gotQuorumTime = Instant.EPOCH; + List<String> respondents; + do { + respondents = curator.framework().getChildren().forPath(barrierPath.getAbsolute()); + if (log.isLoggable(Level.FINE)) { + log.log(Level.FINE, respondents.size() + "/" + curator.zooKeeperEnsembleCount() + " responded: " + + respondents + ", all participants: " + curator.zooKeeperEnsembleConnectionSpec()); + } + + // If all config servers responded, return + if (respondents.size() == curator.zooKeeperEnsembleCount()) { + log.log(Level.FINE, barrierCompletedMessage(respondents, startTime)); + break; + } + + // If some are missing, quorum is enough, but wait for all up to 5 seconds before returning + if (respondents.size() >= barrierMemberCount()) { + if (gotQuorumTime.isBefore(startTime)) + gotQuorumTime = Instant.now(); + + // Give up if more than some time has passed since we got quorum, otherwise continue + if (Duration.between(Instant.now(), gotQuorumTime.plus(waitForAll)).isNegative()) { + log.log(Level.FINE, barrierCompletedMessage(respondents, startTime)); + break; + } + } + + Thread.sleep(100); + } while (clock.instant().isBefore(endTime)); + + return respondents; + } + + private String barrierCompletedMessage(List<String> respondents, Instant startTime) { + return barrierPath + " completed in " + Duration.between(startTime, Instant.now()).toString() + + ", " + respondents.size() + "/" + curator.zooKeeperEnsembleCount() + " responded: " + respondents; + } + + @Override + public void notifyCompletion() { + try { + curator.framework().create().forPath(waiterNode.getAbsolute()); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + @Override + public String toString() { return "'" + barrierPath + "', " + barrierMemberCount() + " members"; } + + public static CompletionWaiter create(Curator curator, ApplicationId applicationId, String serverId) { + return new RemoveApplicationWaiter(curator, applicationId, serverId); + } + + public static CompletionWaiter create(Curator curator, ApplicationId applicationId, String serverId, Duration waitForAll) { + return new RemoveApplicationWaiter(curator, applicationId, serverId, waitForAll); + } + + public static CompletionWaiter createAndInitialize(Curator curator, ApplicationId applicationId, String serverId) { + return createAndInitialize(curator, applicationId, serverId, waitForAllDefault); + } + + public static CompletionWaiter createAndInitialize(Curator curator, ApplicationId applicationId, String serverId, Duration waitForAll) { + RemoveApplicationWaiter waiter = new RemoveApplicationWaiter(curator, applicationId, serverId, waitForAll); + + // Cleanup and create a new barrier path + Path barrierPath = waiter.barrierPath(); + curator.delete(barrierPath); + curator.create(barrierPath.getParentPath()); + curator.createAtomically(barrierPath); + + return waiter; + } + + private int barrierMemberCount() { return (curator.zooKeeperEnsembleCount() / 2) + 1; /* majority */ } + + private Path barrierPath() { return barrierPath; } + + } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java index b4d3f305f61..1734b84ec43 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java @@ -77,6 +77,8 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import static com.yahoo.vespa.curator.Curator.CompletionWaiter; + /** * * Session repository for a tenant. Stores session state in zookeeper and file system. There are two @@ -225,7 +227,7 @@ public class SessionRepository { logger.log(Level.FINE, "Created application " + params.getApplicationId()); long sessionId = session.getSessionId(); SessionZooKeeperClient sessionZooKeeperClient = createSessionZooKeeperClient(sessionId); - Curator.CompletionWaiter waiter = sessionZooKeeperClient.createPrepareWaiter(); + CompletionWaiter waiter = sessionZooKeeperClient.createPrepareWaiter(); Optional<ApplicationSet> activeApplicationSet = getActiveApplicationSet(params.getApplicationId()); ConfigChangeActions actions = sessionPreparer.prepare(applicationRepo.getHostValidator(), logger, params, activeApplicationSet, now, getSessionAppDir(sessionId), @@ -409,11 +411,11 @@ public class SessionRepository { void activate(RemoteSession session) { long sessionId = session.getSessionId(); - Curator.CompletionWaiter waiter = createSessionZooKeeperClient(sessionId).getActiveWaiter(); + CompletionWaiter waiter = createSessionZooKeeperClient(sessionId).getActiveWaiter(); log.log(Level.FINE, () -> session.logPre() + "Activating " + sessionId); applicationRepo.activateApplication(ensureApplicationLoaded(session), sessionId); log.log(Level.FINE, () -> session.logPre() + "Notifying " + waiter); - notifyCompletion(waiter, session); + notifyCompletion(waiter); log.log(Level.INFO, session.logPre() + "Session activated: " + sessionId); } @@ -431,9 +433,9 @@ public class SessionRepository { void prepareRemoteSession(RemoteSession session) { SessionZooKeeperClient sessionZooKeeperClient = createSessionZooKeeperClient(session.getSessionId()); - Curator.CompletionWaiter waiter = sessionZooKeeperClient.getPrepareWaiter(); + CompletionWaiter waiter = sessionZooKeeperClient.getPrepareWaiter(); ensureApplicationLoaded(session); - notifyCompletion(waiter, session); + notifyCompletion(waiter); } public ApplicationSet ensureApplicationLoaded(RemoteSession session) { @@ -453,14 +455,14 @@ public class SessionRepository { } void confirmUpload(Session session) { - Curator.CompletionWaiter waiter = session.getSessionZooKeeperClient().getUploadWaiter(); + CompletionWaiter waiter = session.getSessionZooKeeperClient().getUploadWaiter(); long sessionId = session.getSessionId(); log.log(Level.FINE, "Notifying upload waiter for session " + sessionId); - notifyCompletion(waiter, session); + notifyCompletion(waiter); log.log(Level.FINE, "Done notifying upload for session " + sessionId); } - void notifyCompletion(Curator.CompletionWaiter completionWaiter, Session session) { + void notifyCompletion(CompletionWaiter completionWaiter) { try { completionWaiter.notifyCompletion(); } catch (RuntimeException e) { @@ -474,8 +476,7 @@ public class SessionRepository { KeeperException.NodeExistsException.class); Class<? extends Throwable> exceptionClass = e.getCause().getClass(); if (acceptedExceptions.contains(exceptionClass)) - log.log(Level.FINE, "Not able to notify completion for session " + session.getSessionId() + - " (" + completionWaiter + ")," + + log.log(Level.FINE, "Not able to notify completion for session (" + completionWaiter + ")," + " node " + (exceptionClass.equals(KeeperException.NoNodeException.class) ? "has been deleted" : "already exists")); @@ -618,7 +619,7 @@ public class SessionRepository { log.log(Level.FINE, () -> TenantRepository.logPre(tenantName) + "Creating session " + sessionId + " in ZooKeeper"); SessionZooKeeperClient sessionZKClient = createSessionZooKeeperClient(sessionId); sessionZKClient.createNewSession(clock.instant()); - Curator.CompletionWaiter waiter = sessionZKClient.getUploadWaiter(); + CompletionWaiter waiter = sessionZKClient.getUploadWaiter(); LocalSession session = new LocalSession(tenantName, sessionId, app, sessionZKClient); waiter.awaitCompletion(Duration.ofSeconds(Math.min(60, timeoutBudget.timeLeft().getSeconds()))); addLocalSession(session); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java index 92d7f60d69b..c7c4f1926d7 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.session; import com.yahoo.component.Version; @@ -34,6 +34,7 @@ import java.util.List; import java.util.Optional; import java.util.logging.Level; +import static com.yahoo.vespa.curator.Curator.CompletionWaiter; import static com.yahoo.yolean.Exceptions.uncheck; /** @@ -95,23 +96,15 @@ public class SessionZooKeeperClient { } } - Curator.CompletionWaiter createPrepareWaiter() { - return createCompletionWaiter(PREPARE_BARRIER); - } + public CompletionWaiter createActiveWaiter() { return createCompletionWaiter(ACTIVE_BARRIER); } - public Curator.CompletionWaiter createActiveWaiter() { - return createCompletionWaiter(ACTIVE_BARRIER); - } + CompletionWaiter createPrepareWaiter() { return createCompletionWaiter(PREPARE_BARRIER); } - Curator.CompletionWaiter getPrepareWaiter() { - return getCompletionWaiter(getWaiterPath(PREPARE_BARRIER)); - } + CompletionWaiter getPrepareWaiter() { return getCompletionWaiter(getWaiterPath(PREPARE_BARRIER)); } - Curator.CompletionWaiter getActiveWaiter() { - return getCompletionWaiter(getWaiterPath(ACTIVE_BARRIER)); - } + CompletionWaiter getActiveWaiter() { return getCompletionWaiter(getWaiterPath(ACTIVE_BARRIER)); } - Curator.CompletionWaiter getUploadWaiter() { return getCompletionWaiter(getWaiterPath(UPLOAD_BARRIER)); } + CompletionWaiter getUploadWaiter() { return getCompletionWaiter(getWaiterPath(UPLOAD_BARRIER)); } private static final String PREPARE_BARRIER = "prepareBarrier"; private static final String ACTIVE_BARRIER = "activeBarrier"; @@ -126,11 +119,11 @@ public class SessionZooKeeperClient { return (curator.zooKeeperEnsembleCount() / 2) + 1; // majority } - private Curator.CompletionWaiter createCompletionWaiter(String waiterNode) { + private CompletionWaiter createCompletionWaiter(String waiterNode) { return curator.createCompletionWaiter(sessionPath, waiterNode, getNumberOfMembers(), serverId); } - private Curator.CompletionWaiter getCompletionWaiter(Path path) { + private CompletionWaiter getCompletionWaiter(Path path) { return curator.getCompletionWaiter(path, getNumberOfMembers(), serverId); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java index babb7e4a596..0c2d69ad882 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java @@ -89,6 +89,7 @@ public class TenantRepository { private static final Path tenantsPath = Path.fromString("/config/v2/tenants/"); private static final Path locksPath = Path.fromString("/config/v2/locks/"); + private static final Path barriersPath = Path.fromString("/config/v2/barriers/"); private static final Path vespaPath = Path.fromString("/vespa"); private static final Duration checkForRemovedApplicationsInterval = Duration.ofMinutes(1); private static final Logger log = Logger.getLogger(TenantRepository.class.getName()); @@ -199,6 +200,7 @@ public class TenantRepository { curator.create(tenantsPath); curator.create(locksPath); + curator.create(barriersPath); createSystemTenants(configserverConfig); curator.create(vespaPath); @@ -596,6 +598,10 @@ public class TenantRepository { return locksPath.append(tenantName.value()); } + public static Path getBarriersPath() { + return barriersPath; + } + public Curator getCurator() { return curator; } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java index e4e9afcc12f..d63d862d629 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java @@ -17,8 +17,10 @@ import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry; import com.yahoo.vespa.config.server.monitoring.MetricUpdater; import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.config.server.tenant.TestTenantRepository; +import com.yahoo.vespa.curator.CompletionTimeoutException; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.curator.mock.MockCuratorFramework; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.VespaModelFactory; import org.apache.curator.framework.CuratorFramework; @@ -31,6 +33,7 @@ import org.xml.sax.SAXException; import java.io.File; import java.io.IOException; import java.time.Clock; +import java.time.Duration; import java.util.Arrays; import java.util.Collection; import java.util.LinkedHashMap; @@ -39,13 +42,16 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.IntStream; +import static com.yahoo.vespa.config.server.application.TenantApplications.RemoveApplicationWaiter; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author Ulf Lilleengen @@ -217,6 +223,41 @@ public class TenantApplicationsTest { assertEquals(applications.appendOneLevelOfId("", "search/music/qrservers/default/qr.0"), "search"); } + @Test + public void testRemoveApplication2of3Respond() throws InterruptedException { + Curator curator = new MockCurator3ConfigServers(); + Thread t1 = setupWaiter(curator); + notifyCompletion(curator, 2); + t1.join(); + } + + @Test + public void testRemoveApplicationAllRespond() throws InterruptedException { + Curator curator = new MockCurator3ConfigServers(); + Thread t1 = setupWaiter(curator); + notifyCompletion(curator, 3); + t1.join(); + } + + private Thread setupWaiter(Curator curator) { + Curator.CompletionWaiter waiter = RemoveApplicationWaiter.createAndInitialize(curator, createApplicationId(), "cfg1", Duration.ofSeconds(1)); + Thread t1 = new Thread(() -> { + try { + waiter.awaitCompletion(Duration.ofSeconds(120)); + } catch (CompletionTimeoutException e) { + fail("Waiting failed due to timeout"); + } + }); + t1.start(); + return t1; + } + + private void notifyCompletion(Curator curator, int respondentCount) { + IntStream.range(0, respondentCount) + .forEach(i -> RemoveApplicationWaiter.create(curator, createApplicationId(), "cfg" + i, Duration.ofSeconds(1)) + .notifyCompletion()); + } + private TenantApplications createZKAppRepo() { return TenantApplications.create(new HostRegistry(), tenantName, @@ -226,6 +267,11 @@ public class TenantApplicationsTest { new TenantApplicationsTest.MockReloadListener()); } + + private static ApplicationId createApplicationId() { + return createApplicationId("foo"); + } + private static ApplicationId createApplicationId(String name) { return ApplicationId.from(tenantName.value(), name, "myinst"); } @@ -247,4 +293,12 @@ public class TenantApplicationsTest { new TestModelFactory(new Version(3, 2, 1)))); } + private static class MockCurator3ConfigServers extends Curator { + + public MockCurator3ConfigServers() { + super("host1:2181,host2:2181,host3:2181", "host1:2181,host2:2181,host3:2181", (retryPolicy) -> new MockCuratorFramework(true, false)); + } + + } + } diff --git a/container-dependencies-enforcer/pom.xml b/container-dependencies-enforcer/pom.xml index c2d0fd8510d..ab2cfdda1a1 100644 --- a/container-dependencies-enforcer/pom.xml +++ b/container-dependencies-enforcer/pom.xml @@ -89,7 +89,7 @@ <include>com.sun.activation:javax.activation:[1.2.0]:jar:provided</include> <include>com.sun.xml.bind:jaxb-core:[${jaxb.version}]:jar:provided</include> <include>com.sun.xml.bind:jaxb-impl:[${jaxb.version}]:jar:provided</include> - <include>commons-logging:commons-logging:[1.1.3]:jar:provided</include> + <include>commons-logging:commons-logging:[1.2]:jar:provided</include> <include>jakarta.activation:jakarta.activation-api:[1.2.1]:jar:provided</include> <include>jakarta.xml.bind:jakarta.xml.bind-api:[2.3.2]:jar:provided</include> <include>javax.annotation:javax.annotation-api:[${javax.annotation-api.version}]:jar:provided</include> diff --git a/container-dependency-versions/pom.xml b/container-dependency-versions/pom.xml index a278421f603..04ce3405d83 100644 --- a/container-dependency-versions/pom.xml +++ b/container-dependency-versions/pom.xml @@ -118,7 +118,7 @@ <groupId>commons-logging</groupId> <artifactId>commons-logging</artifactId> <!-- This version is exported by jdisc via jcl-over-slf4j. --> - <version>1.1.3</version> + <version>1.2</version> </dependency> <dependency> <groupId>javax.annotation</groupId> @@ -461,7 +461,7 @@ <jetty.version>9.4.38.v20210224</jetty.version> <org.lz4.version>1.7.1</org.lz4.version> <org.json.version>20090211</org.json.version> - <slf4j.version>1.7.5</slf4j.version> + <slf4j.version>1.7.30</slf4j.version> <xml-apis.version>1.4.01</xml-apis.version> <!-- These must be kept in sync with version used by current jersey2.version. --> diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java index 0b424213112..bbe982bd5fe 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java @@ -181,6 +181,8 @@ public interface NodeRepository { case proxyhost: return NodeType.proxyhost; case config: return NodeType.config; case confighost: return NodeType.confighost; + case controller: return NodeType.controller; + case controllerhost: return NodeType.controllerhost; default: throw new IllegalArgumentException("Unknown type: " + nodeType); } } diff --git a/document/src/main/java/com/yahoo/document/json/readers/StructReader.java b/document/src/main/java/com/yahoo/document/json/readers/StructReader.java index dc2c001672d..454d93c7eab 100644 --- a/document/src/main/java/com/yahoo/document/json/readers/StructReader.java +++ b/document/src/main/java/com/yahoo/document/json/readers/StructReader.java @@ -34,9 +34,10 @@ public class StructReader { public static Field getField(TokenBuffer buffer, StructuredFieldValue parent) { Field field = parent.getField(buffer.currentName()); - if (field == null) + if (field == null) { throw new IllegalArgumentException("No field '" + buffer.currentName() + "' in the structure of type '" + - parent.getDataType().getDataTypeName() + "'"); + parent.getDataType().getDataTypeName() + "', which has the fields:" + parent.getDataType().getFields()); + } return field; } diff --git a/eval/src/vespa/eval/eval/test/tensor_conformance.cpp b/eval/src/vespa/eval/eval/test/tensor_conformance.cpp index 17ad75ae455..30933c78325 100644 --- a/eval/src/vespa/eval/eval/test/tensor_conformance.cpp +++ b/eval/src/vespa/eval/eval/test/tensor_conformance.cpp @@ -9,6 +9,7 @@ #include <vespa/eval/eval/simple_value.h> #include <vespa/eval/eval/value_type_spec.h> #include <vespa/vespalib/testkit/test_kit.h> +#include <vespa/vespalib/util/require.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/objects/nbostream.h> #include <vespa/vespalib/data/slime/slime.h> @@ -47,7 +48,7 @@ TensorSpec eval(const ValueBuilderFactory &factory, const vespalib::string &expr } NodeTypes types(*fun, param_types); const auto &expect_type = types.get_type(fun->root()); - ASSERT_FALSE(expect_type.is_error()); + REQUIRE(!expect_type.is_error()); InterpretedFunction ifun(factory, *fun, types); InterpretedFunction::Context ctx(ifun); const Value &result = ifun.eval(ctx, SimpleObjectParams{param_refs}); @@ -451,7 +452,7 @@ struct TestContext { {x({"a","b","c"}),y(5)}, float_cells({y(5),z({"i","j","k","l"})}), float_cells({x({"a","b","c"}),y(5)}), float_cells({y(5),z({"i","j","k","l"})}) }; - ASSERT_TRUE((layouts.size() % 2) == 0); + REQUIRE((layouts.size() % 2) == 0); for (size_t i = 0; i < layouts.size(); i += 2) { TensorSpec lhs_input = spec(layouts[i], seq); TensorSpec rhs_input = spec(layouts[i + 1], seq); diff --git a/eval/src/vespa/eval/eval/test/test_io.cpp b/eval/src/vespa/eval/eval/test/test_io.cpp index 4ecfb788f28..b53ee864cbe 100644 --- a/eval/src/vespa/eval/eval/test/test_io.cpp +++ b/eval/src/vespa/eval/eval/test/test_io.cpp @@ -1,7 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "test_io.h" -#include <vespa/vespalib/testkit/test_kit.h> +#include <vespa/vespalib/util/require.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/data/slime/json_format.h> #include <vespa/vespalib/util/size_literals.h> @@ -75,8 +75,8 @@ void TestWriter::maybe_write_test() { if (_test.get().type().getId() != slime::NIX::ID) { - ASSERT_GREATER(_test.get().fields(), 0u); - ASSERT_FALSE(_test[num_tests_str].valid()); + REQUIRE(_test.get().fields() > 0u); + REQUIRE(!_test[num_tests_str].valid()); write_compact(_test, _out); ++_num_tests; } @@ -116,21 +116,21 @@ void for_each_test(Input &in, if (JsonFormat::decode(in, slime)) { bool is_summary = slime[num_tests_str].valid(); bool is_test = (!is_summary && (slime.get().fields() > 0)); - ASSERT_TRUE(is_test != is_summary); + REQUIRE(is_test != is_summary); if (is_test) { ++num_tests; - ASSERT_TRUE(!got_summary); + REQUIRE(!got_summary); handle_test(slime); } else { got_summary = true; - ASSERT_EQUAL(slime[num_tests_str].asLong(), int64_t(num_tests)); + REQUIRE_EQ(slime[num_tests_str].asLong(), int64_t(num_tests)); handle_summary(slime); } } else { - ASSERT_EQUAL(in.obtain().size, 0u); + REQUIRE_EQ(in.obtain().size, 0u); } } - ASSERT_TRUE(got_summary); + REQUIRE(got_summary); } //----------------------------------------------------------------------------- diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 50419577e7a..4576b546d2e 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -274,6 +274,12 @@ public class Flags { "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); + public static final UnboundBooleanFlag WAIT_FOR_ALL_CONFIG_SERVERS_WHEN_DELETING_APPLICATION = defineFeatureFlag( + "wait-for-all-config-servers-when-deleting-application", false, + List.of("hmusum"), "2021-03-24", "2021-06-24", + "Whether to wait for all participating servers to delete application on config servers (with timeout) on", + "Takes effect on next delete of an application"); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners, String createdAt, String expiresAt, String description, diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 2d9b61b6ceb..3133a5568f7 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -617,10 +617,8 @@ public class NodeAgentImpl implements NodeAgent { Optional<NodeMembership> membership = context.node().membership(); return zone.getSystemName().isCd() || zone.getEnvironment().isTest() - || (context.nodeType() != NodeType.tenant) - || membership.map(mem -> ! (mem.type().isContainer() || - mem.type().isCombined())) - .orElse(false) + || context.nodeType() != NodeType.tenant + || membership.map(mem -> ! (mem.type().isContainer() || mem.type().isAdmin())).orElse(false) ? Duration.ofSeconds(-1) : warmUpDuration; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepoStats.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepoStats.java index e11a57f04df..ca18028ad5a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepoStats.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepoStats.java @@ -1,11 +1,20 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision; +import com.yahoo.collections.Pair; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.autoscale.Load; import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricSnapshot; +import com.yahoo.vespa.hosted.provision.autoscale.NodeTimeseries; import java.time.Duration; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -18,10 +27,12 @@ public class NodeRepoStats { private final Load load; private final Load activeLoad; + private final List<ApplicationStats> applicationStats; - private NodeRepoStats(Load load, Load activeLoad) { + private NodeRepoStats(Load load, Load activeLoad, List<ApplicationStats> applicationStats) { this.load = load; this.activeLoad = activeLoad; + this.applicationStats = List.copyOf(applicationStats); } /** @@ -33,36 +44,71 @@ public class NodeRepoStats { /** Returns the current average utilization in this node repo over all active nodes. */ public Load activeLoad() { return activeLoad; } + /** Returns stats on each application, sorted by decreasing unutilized allocation measured in dollars per hour. */ + public List<ApplicationStats> applicationStats() { return applicationStats; } + public static NodeRepoStats computeOver(NodeRepository nodeRepository) { NodeList allNodes = nodeRepository.nodes().list(); + List<NodeTimeseries> allNodeTimeseries = nodeRepository.metricsDb().getNodeTimeseries(Duration.ofHours(1), Set.of()); + + Pair<Load, Load> load = computeLoad(allNodes, allNodeTimeseries); + List<ApplicationStats> applicationStats = computeApplicationStats(allNodes, allNodeTimeseries); + return new NodeRepoStats(load.getFirst(), load.getSecond(), applicationStats); + } - NodeResources totalActiveResources = new NodeResources(0, 0, 0, 0); - double cpu = 0, memory = 0, disk = 0; - for (var nodeTimeseries : nodeRepository.metricsDb().getNodeTimeseries(Duration.ofHours(1), Set.of())) { + private static Pair<Load, Load> computeLoad(NodeList allNodes, List<NodeTimeseries> allNodeTimeseries) { + NodeResources totalActiveResources = NodeResources.zero(); + Load load = Load.zero(); + for (var nodeTimeseries : allNodeTimeseries) { Optional<Node> node = allNodes.node(nodeTimeseries.hostname()); if (node.isEmpty() || node.get().state() != Node.State.active) continue; Optional<NodeMetricSnapshot> snapshot = nodeTimeseries.last(); if (snapshot.isEmpty()) continue; - cpu += snapshot.get().cpu() * node.get().resources().vcpu(); - memory += snapshot.get().memory() * node.get().resources().memoryGb(); - disk += snapshot.get().disk() * node.get().resources().diskGb(); + load = load.add(snapshot.get().load().multiply(node.get().resources())); totalActiveResources = totalActiveResources.add(node.get().resources().justNumbers()); } - NodeResources totalHostResources = new NodeResources(0, 0, 0, 0); - for (var host : allNodes.hosts()) { + NodeResources totalHostResources = NodeResources.zero(); + for (var host : allNodes.hosts()) totalHostResources = totalHostResources.add(host.resources().justNumbers()); + + return new Pair<>(load.divide(totalHostResources), load.divide(totalActiveResources)); + } + + private static List<ApplicationStats> computeApplicationStats(NodeList allNodes, + List<NodeTimeseries> allNodeTimeseries) { + List<ApplicationStats> applicationStats = new ArrayList<>(); + Map<String, NodeMetricSnapshot> snapshotsByHost = byHost(allNodeTimeseries); + for (var applicationNodes : allNodes.state(Node.State.active) + .nodeType(NodeType.tenant) + .matching(node -> ! node.allocation().get().owner().instance().isTester()) + .groupingBy(node -> node.allocation().get().owner()).entrySet()) { + + NodeResources totalResources = NodeResources.zero(); + NodeResources totalUtilizedResources = NodeResources.zero(); + for (var node : applicationNodes.getValue()) { + var snapshot = snapshotsByHost.get(node.hostname()); + if (snapshot == null) continue; + + totalResources = totalResources.add(node.resources().justNumbers()); + totalUtilizedResources = totalUtilizedResources.add(snapshot.load().scaled(node.resources().justNumbers())); + } + applicationStats.add(new ApplicationStats(applicationNodes.getKey(), + Load.byDividing(totalUtilizedResources, totalResources), + totalResources.cost(), + totalUtilizedResources.cost())); } + Collections.sort(applicationStats); + return applicationStats; + } - Load load = new Load(divide(cpu, totalHostResources.vcpu()), - divide(memory, totalHostResources.memoryGb()), - divide(disk, totalHostResources.diskGb())); - Load activeLoad = new Load(divide(cpu, totalActiveResources.vcpu()), - divide(memory, totalActiveResources.memoryGb()), - divide(disk, totalActiveResources.diskGb())); - return new NodeRepoStats(load, activeLoad); + private static Map<String, NodeMetricSnapshot> byHost(List<NodeTimeseries> allNodeTimeseries) { + Map<String, NodeMetricSnapshot> snapshots = new HashMap<>(); + for (var nodeTimeseries : allNodeTimeseries) + nodeTimeseries.last().ifPresent(last -> snapshots.put(nodeTimeseries.hostname(), last)); + return snapshots; } private static double divide(double a, double b) { @@ -70,4 +116,37 @@ public class NodeRepoStats { return a / b; } + public static class ApplicationStats implements Comparable<ApplicationStats> { + + private final ApplicationId id; + private final Load load; + private final double cost; + private final double utilizedCost; + + public ApplicationStats(ApplicationId id, Load load, double cost, double utilizedCost) { + this.id = id; + this.load = load; + this.cost = cost; + this.utilizedCost = utilizedCost; + } + + public ApplicationId id() { return id; } + public Load load() { return load; } + + /** The standard cost of this application */ + public double cost() { return cost; } + + /** The amount of that cost which is currently utilized */ + public double utilizedCost() { return utilizedCost; } + + /** Cost - utilizedCost */ + public double unutilizedCost() { return cost - utilizedCost; } + + @Override + public int compareTo(NodeRepoStats.ApplicationStats other) { + return -Double.compare(this.unutilizedCost(), other.unutilizedCost()); + } + + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java index a06ad89e299..79b09348d21 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java @@ -96,18 +96,18 @@ public class AllocationOptimizer { // Memory and disk: Scales with group size // The fixed cost portion of cpu does not scale with changes to the node count - double queryCpuPerGroup = fixedCpuCostFraction * target.nodeCpu() + - (1 - fixedCpuCostFraction) * target.nodeCpu() * current.groupSize() / groupSize; + double queryCpuPerGroup = fixedCpuCostFraction * target.resources().vcpu() + + (1 - fixedCpuCostFraction) * target.resources().vcpu() * current.groupSize() / groupSize; double queryCpu = queryCpuPerGroup * current.groups() / groups; - double writeCpu = target.nodeCpu() * current.groupSize() / groupSize; + double writeCpu = target.resources().vcpu() * current.groupSize() / groupSize; cpu = clusterModel.queryCpuFraction() * queryCpu + (1 - clusterModel.queryCpuFraction()) * writeCpu; - memory = target.nodeMemory() * current.groupSize() / groupSize; - disk = target.nodeDisk() * current.groupSize() / groupSize; + memory = target.resources().memoryGb() * current.groupSize() / groupSize; + disk = target.resources().diskGb() * current.groupSize() / groupSize; } else { - cpu = target.nodeCpu() * current.nodes() / nodes; - memory = target.nodeMemory(); - disk = target.nodeDisk(); + cpu = target.resources().vcpu() * current.nodes() / nodes; + memory = target.resources().memoryGb(); + disk = target.resources().diskGb(); } // Combine the scaled resource values computed here diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java index acf227e3de2..e3622c8f076 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java @@ -96,15 +96,10 @@ public class ClusterModel { return queryFractionOfMax = clusterTimeseries().queryFractionOfMax(scalingDuration(), clock); } - public double averageLoad(Resource resource) { return nodeTimeseries().averageLoad(resource); } - - public double idealLoad(Resource resource) { - switch (resource) { - case cpu : return idealCpuLoad(); - case memory : return idealMemoryLoad; - case disk : return idealDiskLoad; - default : throw new IllegalStateException("No ideal load defined for " + resource); - } + public Load averageLoad() { return nodeTimeseries().averageLoad(); } + + public Load idealLoad() { + return new Load(idealCpuLoad(), idealMemoryLoad, idealDiskLoad); } /** Ideal cpu load must take the application traffic fraction into account */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterNodesTimeseries.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterNodesTimeseries.java index c097abd8208..a7396f29d92 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterNodesTimeseries.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterNodesTimeseries.java @@ -42,21 +42,17 @@ public class ClusterNodesTimeseries { /** Returns the number of nodes measured in this */ public int nodesMeasured() { return timeseries.size(); } - /** Returns the average load of this resource in this */ - public double averageLoad(Resource resource) { - int measurementCount = timeseries.stream().mapToInt(m -> m.size()).sum(); - if (measurementCount == 0) return 0; - double measurementSum = timeseries.stream().flatMap(m -> m.asList().stream()).mapToDouble(m -> value(resource, m)).sum(); - return measurementSum / measurementCount; - } - - private double value(Resource resource, NodeMetricSnapshot snapshot) { - switch (resource) { - case cpu: return snapshot.cpu(); - case memory: return snapshot.memory(); - case disk: return snapshot.disk(); - default: throw new IllegalArgumentException("Got an unknown resource " + resource); + /** Returns the average load in this */ + public Load averageLoad() { + Load total = Load.zero(); + int count = 0; + for (var nodeTimeseries : timeseries) { + for (var snapshot : nodeTimeseries.asList()) { + total = total.add(snapshot.load()); + count++; + } } + return total.divide(count); } private List<NodeTimeseries> filter(List<NodeTimeseries> timeseries, Predicate<NodeMetricSnapshot> filter) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Load.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Load.java index 1a13c1bb6d8..1e400bd2627 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Load.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Load.java @@ -1,6 +1,8 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.autoscale; +import com.yahoo.config.provision.NodeResources; + /** * The load of a node or system, measured as fractions of max (1.0) in three dimensions. * @@ -20,11 +22,41 @@ public class Load { public double memory() { return memory; } public double disk() { return disk; } + public Load add(Load other) { + return new Load(cpu + other.cpu(), memory + other.memory(), disk + other.disk()); + } + + public Load multiply(NodeResources resources) { + return new Load(cpu * resources.vcpu(), memory * resources.memoryGb(), disk * resources.diskGb()); + } + + public Load multiply(double factor) { + return new Load(cpu * factor, memory * factor, disk * factor); + } + + public Load divide(NodeResources resources) { + return new Load(divide(cpu, resources.vcpu()), divide(memory, resources.memoryGb()), divide(disk, resources.diskGb())); + } + + public Load divide(Load divisor) { + return new Load(divide(cpu, divisor.cpu()), divide(memory, divisor.memory()), divide(disk, divisor.disk())); + } + + public Load divide(double divisor) { + return new Load(divide(cpu, divisor), divide(memory, divisor), divide(disk, divisor)); + } + + public NodeResources scaled(NodeResources resources) { + return resources.withVcpu(cpu * resources.vcpu()) + .withMemoryGb(memory * resources.memoryGb()) + .withDiskGb(disk * resources.diskGb()); + } + private double requireNormalized(double value, String name) { if (Double.isNaN(value)) - throw new IllegalArgumentException(name + " must be a number between 0 and 1, but is NaN"); - if (value < 0 || value > 1) - throw new IllegalArgumentException(name + " must be between 0 and 1, but is " + value); + throw new IllegalArgumentException(name + " must be a number but is NaN"); + if (value < 0) + throw new IllegalArgumentException(name + " must be zero or lager, but is " + value); return value; } @@ -35,4 +67,13 @@ public class Load { public static Load zero() { return new Load(0, 0, 0); } + private static double divide(double a, double b) { + if (a == 0 && b == 0) return 0; + return a / b; + } + + public static Load byDividing(NodeResources a, NodeResources b) { + return new Load(divide(a.vcpu(), b.vcpu()), divide(a.memoryGb(), b.memoryGb()), divide(a.diskGb(), b.diskGb())); + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java index b3cf6c1e962..210388db7b8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java @@ -67,9 +67,9 @@ public class MetricsResponse { consumeServiceMetrics(nodeObject.field("services"), nodeValues); nodeMetrics.add(new Pair<>(hostname, new NodeMetricSnapshot(at, - Metric.cpu.from(nodeValues), - Metric.memory.from(nodeValues), - Metric.disk.from(nodeValues), + new Load(Metric.cpu.from(nodeValues), + Metric.memory.from(nodeValues), + Metric.disk.from(nodeValues)), (long)Metric.generation.from(nodeValues), Metric.inService.from(nodeValues) > 0, clusterIsStable(node.get(), applicationNodes, nodeRepository), diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricSnapshot.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricSnapshot.java index be9f7bd4819..6329d350642 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricSnapshot.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricSnapshot.java @@ -12,21 +12,17 @@ public class NodeMetricSnapshot implements Comparable<NodeMetricSnapshot> { private final Instant at; - private final double cpu; - private final double memory; - private final double disk; + private final Load load; private final long generation; private final boolean inService; private final boolean stable; private final double queryRate; - public NodeMetricSnapshot(Instant at, double cpu, double memory, double disk, + public NodeMetricSnapshot(Instant at, Load load, long generation, boolean inService, boolean stable, double queryRate) { this.at = at; - this.cpu = cpu; - this.memory = memory; - this.disk = disk; + this.load = load; this.generation = generation; this.inService = inService; this.stable = stable; @@ -34,9 +30,7 @@ public class NodeMetricSnapshot implements Comparable<NodeMetricSnapshot> { } public Instant at() { return at; } - public double cpu() { return cpu; } - public double memory() { return memory; } - public double disk() { return disk; } + public Load load() { return load; } /** Queries per second */ public double queryRate() { return queryRate; } @@ -53,10 +47,8 @@ public class NodeMetricSnapshot implements Comparable<NodeMetricSnapshot> { } @Override - public String toString() { return "metrics at " + at + ":" + - " cpu: " + cpu + - " memory: " + memory + - " disk: " + disk + + public String toString() { return "metrics at " + at + ": " + + load + " generation: " + generation + " inService: " + inService + " stable: " + stable + diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java index 459a7919bbe..c933e16041a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDb.java @@ -113,9 +113,9 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { TableWriter.Row row = writer.newRow(atMillis * 1000); // in microseconds row.putStr(0, snapshot.getFirst()); // (1 is timestamp) - row.putFloat(2, (float)snapshot.getSecond().cpu()); - row.putFloat(3, (float)snapshot.getSecond().memory()); - row.putFloat(4, (float)snapshot.getSecond().disk()); + row.putFloat(2, (float)snapshot.getSecond().load().cpu()); + row.putFloat(3, (float)snapshot.getSecond().load().memory()); + row.putFloat(4, (float)snapshot.getSecond().load().disk()); row.putLong(5, snapshot.getSecond().generation()); row.putBool(6, snapshot.getSecond().inService()); row.putBool(7, snapshot.getSecond().stable()); @@ -358,9 +358,9 @@ public class QuestMetricsDb extends AbstractComponent implements MetricsDb { if (hostnames.isEmpty() || hostnames.contains(hostname)) { snapshots.put(hostname, new NodeMetricSnapshot(Instant.ofEpochMilli(record.getTimestamp(1) / 1000), - record.getFloat(2), - record.getFloat(3), - record.getFloat(4), + new Load(record.getFloat(2), + record.getFloat(3), + record.getFloat(4)), record.getLong(5), record.getBool(6), record.getBool(7), diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java deleted file mode 100644 index c639ad1f779..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.autoscale; - -import com.yahoo.config.provision.NodeResources; - -/** - * A resource subject to autoscaling - * - * @author bratseth - */ -public enum Resource { - - /** Cpu utilization ratio */ - cpu { - double valueFrom(NodeResources resources) { return resources.vcpu(); } - }, - - /** Memory utilization ratio */ - memory { - double valueFrom(NodeResources resources) { return resources.memoryGb(); } - }, - - /** Disk utilization ratio */ - disk { - double valueFrom(NodeResources resources) { return resources.diskGb(); } - }; - - abstract double valueFrom(NodeResources resources); - -} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java index b1a1e86b08d..3b8b7f08684 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java @@ -1,6 +1,7 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.autoscale; +import com.yahoo.config.provision.NodeResources; import com.yahoo.vespa.hosted.provision.applications.Application; import java.time.Clock; @@ -18,56 +19,34 @@ public class ResourceTarget { private final boolean adjustForRedundancy; /** The target real resources per node, assuming the node assignment where this was decided */ - private final double cpu, memory, disk; + private final NodeResources resources; - private ResourceTarget(double cpu, double memory, double disk, boolean adjustForRedundancy) { - this.cpu = cpu; - this.memory = memory; - this.disk = disk; + private ResourceTarget(NodeResources resources, boolean adjustForRedundancy) { + this.resources = resources; this.adjustForRedundancy = adjustForRedundancy; } /** Are the target resources given by this including redundancy or not */ public boolean adjustForRedundancy() { return adjustForRedundancy; } - /** Returns the target cpu per node, in terms of the current allocation */ - public double nodeCpu() { return cpu; } - - /** Returns the target memory per node, in terms of the current allocation */ - public double nodeMemory() { return memory; } - - /** Returns the target disk per node, in terms of the current allocation */ - public double nodeDisk() { return disk; } + /** Returns the target resources per node in terms of the current allocation */ + public NodeResources resources() { return resources; } @Override public String toString() { - return "target " + - (adjustForRedundancy ? "(with redundancy adjustment) " : "") + - "[vcpu " + cpu + ", memoryGb " + memory + ", diskGb " + disk + "]"; - } - - private static double nodeUsage(Resource resource, double load, AllocatableClusterResources current) { - return load * resource.valueFrom(current.realResources().nodeResources()); + return "target " + resources + (adjustForRedundancy ? "(with redundancy adjustment) " : ""); } /** Create a target of achieving ideal load given a current load */ public static ResourceTarget idealLoad(ClusterModel clusterModel, AllocatableClusterResources current) { - return new ResourceTarget(nodeUsage(Resource.cpu, clusterModel.averageLoad(Resource.cpu), current) - / clusterModel.idealLoad(Resource.cpu), - nodeUsage(Resource.memory, clusterModel.averageLoad(Resource.memory), current) - / clusterModel.idealLoad(Resource.memory), - nodeUsage(Resource.disk, clusterModel.averageLoad(Resource.disk), current) - / clusterModel.idealLoad(Resource.disk), - true); + var loadAdjustment = clusterModel.averageLoad().divide(clusterModel.idealLoad()); + return new ResourceTarget(loadAdjustment.scaled(current.realResources().nodeResources()), true); } /** Crete a target of preserving a current allocation */ public static ResourceTarget preserve(AllocatableClusterResources current) { - return new ResourceTarget(current.realResources().nodeResources().vcpu(), - current.realResources().nodeResources().memoryGb(), - current.realResources().nodeResources().diskGb(), - false); + return new ResourceTarget(current.realResources().nodeResources(), false); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java index 176bf195f1f..692d757f41d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java @@ -1,27 +1,18 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.restapi; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterResources; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; -import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Application; import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.applications.ScalingEvent; import com.yahoo.vespa.hosted.provision.autoscale.ClusterModel; -import com.yahoo.vespa.hosted.provision.autoscale.ClusterNodesTimeseries; -import com.yahoo.vespa.hosted.provision.autoscale.ClusterTimeseries; import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; -import com.yahoo.vespa.hosted.provision.autoscale.Resource; -import com.yahoo.vespa.hosted.provision.autoscale.ResourceTarget; import java.net.URI; -import java.time.Clock; -import java.time.Duration; -import java.util.Collection; import java.util.List; /** @@ -94,12 +85,12 @@ public class ApplicationSerializer { } private static void clusterUtilizationToSlime(ClusterModel clusterModel, Cursor utilizationObject) { - utilizationObject.setDouble("cpu", clusterModel.averageLoad(Resource.cpu)); - utilizationObject.setDouble("idealCpu", clusterModel.idealLoad(Resource.cpu)); - utilizationObject.setDouble("memory", clusterModel.averageLoad(Resource.memory)); - utilizationObject.setDouble("idealMemory", clusterModel.idealLoad(Resource.memory)); - utilizationObject.setDouble("disk", clusterModel.averageLoad(Resource.disk)); - utilizationObject.setDouble("idealDisk", clusterModel.idealLoad(Resource.disk)); + utilizationObject.setDouble("cpu", clusterModel.averageLoad().cpu()); + utilizationObject.setDouble("idealCpu", clusterModel.idealLoad().cpu()); + utilizationObject.setDouble("memory", clusterModel.averageLoad().memory()); + utilizationObject.setDouble("idealMemory", clusterModel.idealLoad().memory()); + utilizationObject.setDouble("disk", clusterModel.averageLoad().disk()); + utilizationObject.setDouble("idealDisk", clusterModel.idealLoad().disk()); } private static void scalingEventsToSlime(List<ScalingEvent> scalingEvents, Cursor scalingEventsArray) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepoStatsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepoStatsTest.java index 5d136016a71..44376fc103c 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepoStatsTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepoStatsTest.java @@ -2,15 +2,24 @@ package com.yahoo.vespa.hosted.provision; import com.yahoo.collections.Pair; +import com.yahoo.component.Version; +import com.yahoo.config.provision.Capacity; +import com.yahoo.config.provision.ClusterResources; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.provision.autoscale.Load; import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricSnapshot; +import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; import org.junit.Test; import java.time.Duration; import java.util.List; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * @author bratseth @@ -24,6 +33,7 @@ public class NodeRepoStatsTest { var tester = new NodeRepositoryTester(); assertLoad(Load.zero(), tester.nodeRepository().computeStats().load()); assertLoad(Load.zero(), tester.nodeRepository().computeStats().activeLoad()); + assertTrue(tester.nodeRepository().computeStats().applicationStats().isEmpty()); } @Test @@ -34,33 +44,83 @@ public class NodeRepoStatsTest { tester.addHost("host3", "small"); assertLoad(Load.zero(), tester.nodeRepository().computeStats().load()); assertLoad(Load.zero(), tester.nodeRepository().computeStats().activeLoad()); + assertTrue(tester.nodeRepository().computeStats().applicationStats().isEmpty()); } @Test - public void testNodesAndMetrics() { - var tester = new NodeRepositoryTester(); - tester.addHost("host1", "default"); - tester.addHost("host2", "default"); - tester.addHost("host3", "small"); - tester.addNode("node1", "host1", new NodeResources(0.2, 0.5, 4, 1)); - tester.addNode("node2", "host1", new NodeResources(0.3, 1.0, 8, 1)); - tester.addNode("node3", "host3", new NodeResources(0.3, 1.5, 12, 1)); - tester.setNodeState("node1", Node.State.active); - tester.setNodeState("node2", Node.State.active); - tester.setNodeState("node3", Node.State.active); - assertLoad(Load.zero(), tester.nodeRepository().computeStats().load()); - assertLoad(Load.zero(), tester.nodeRepository().computeStats().activeLoad()); + public void testStats() { + var hostResources = new NodeResources(10, 100, 1000, 10, + NodeResources.DiskSpeed.fast, NodeResources.StorageType.local); + + ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build(); + tester.makeReadyHosts(10, hostResources).activateTenantHosts(); + + var app1 = ProvisioningTester.applicationId("app1"); + var app2 = ProvisioningTester.applicationId("app2"); + var app3 = ProvisioningTester.applicationId("app3"); + var cluster1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("cluster1")).vespaVersion(Version.fromString("7")).build(); + var cluster2 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("cluster2")).vespaVersion(Version.fromString("7")).build(); + var small = new NodeResources(1, 10, 100, 1, NodeResources.DiskSpeed.any); + var large = new NodeResources(2, 20, 200, 1); - var before = tester.clock().instant(); - tester.clock().advance(Duration.ofMinutes(5)); + // Deploy apps + var hostsApp1 = tester.prepare(app1, cluster1, Capacity.from(new ClusterResources(6, 1, small))); + hostsApp1.addAll(tester.prepare(app1, cluster2, Capacity.from(new ClusterResources(4, 1, large)))); + tester.activate(app1, hostsApp1); + tester.activate(app2, cluster1, Capacity.from(new ClusterResources(8, 1, small))); + tester.activate(app3, cluster1, Capacity.from(new ClusterResources(5, 1, large))); + + // Add metrics + double loadApp1Cluster1 = 0.2; + double loadApp1Cluster2 = 0.3; + double loadApp2 = 0.4; + double loadApp3 = 0.5; var now = tester.clock().instant(); - tester.nodeRepository().metricsDb().addNodeMetrics( - List.of(new Pair<>("node1", new NodeMetricSnapshot(before, 0, 0, 0, 1, true, true, 1.0)), - new Pair<>("node1", new NodeMetricSnapshot(now, 0.5, 0.1, 0.8, 1, true, true, 1.0)), - new Pair<>("node2", new NodeMetricSnapshot(now, 0.1, 0.8, 0.1, 1, true, true, 1.0)), - new Pair<>("node3", new NodeMetricSnapshot(now, 1.0, 0.1, 0.2, 1, true, true, 1.0)))); - assertLoad(new Load(0.0860, 0.1000, 0.0256), tester.nodeRepository().computeStats().load()); - assertLoad(new Load(0.5375, 0.3333, 0.2667), tester.nodeRepository().computeStats().activeLoad()); + for (Node node : tester.nodeRepository().nodes().list(Node.State.active)) { + double loadFactor; + var allocation = node.allocation().get(); + if (allocation.owner().equals(app1)) { + if (allocation.membership().cluster().id().equals(cluster1.id())) + loadFactor = loadApp1Cluster1; + else + loadFactor = loadApp1Cluster2; + } + else if (allocation.owner().equals(app2)) { + loadFactor = loadApp2; + } + else { + loadFactor = loadApp3; + } + var snapshot = new NodeMetricSnapshot(now, new Load(1.0, 0.9, 0.8).multiply(loadFactor), 1, true, true, 1.0 ); + tester.nodeRepository().metricsDb().addNodeMetrics(List.of(new Pair<>(node.hostname(), snapshot))); + } + + var stats = tester.nodeRepository().computeStats(); + + assertLoad(new Load(0.6180,0.5562,0.4944), stats.load()); + assertLoad(new Load(0.4682,0.4214,0.3745), stats.activeLoad()); + + var app1Stats = stats.applicationStats().get(0); + var app2Stats = stats.applicationStats().get(2); + var app3Stats = stats.applicationStats().get(1); + + assertEquals(app1, app1Stats.id()); + assertEquals(2.940, app1Stats.cost(), delta); + assertEquals(0.702, app1Stats.utilizedCost(), delta); + assertEquals(2.238, app1Stats.unutilizedCost(), delta); + assertLoad(new Load(0.2571, 0.2314, 0.2057), app1Stats.load()); + + assertEquals(app2, app2Stats.id()); + assertEquals(1.680, app2Stats.cost(), delta); + assertEquals(0.624, app2Stats.utilizedCost(), delta); + assertEquals(1.056, app2Stats.unutilizedCost(), delta); + assertLoad(new Load(.40, 0.36, 0.32), app2Stats.load()); + + assertEquals(app3, app3Stats.id()); + assertEquals(2.100, app3Stats.cost(), delta); + assertEquals(0.975, app3Stats.utilizedCost(), delta); + assertEquals(1.125, app3Stats.unutilizedCost(), delta); + assertLoad(new Load(0.5, 0.45, 0.40), app3Stats.load()); } private static void assertLoad(Load expected, Load actual) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java index a5d8256439f..5fe6023e5af 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java @@ -138,14 +138,12 @@ class AutoscalingTester { for (int i = 0; i < count; i++) { clock().advance(Duration.ofMinutes(5)); for (Node node : nodes) { - float cpu = value * oneExtraNodeFactor; - float memory = (float) ClusterModel.idealMemoryLoad * otherResourcesLoad * oneExtraNodeFactor; - float disk = (float) ClusterModel.idealDiskLoad * otherResourcesLoad * oneExtraNodeFactor; + Load load = new Load(value, + ClusterModel.idealMemoryLoad * otherResourcesLoad, + ClusterModel.idealDiskLoad * otherResourcesLoad).multiply(oneExtraNodeFactor); nodeMetricsDb().addNodeMetrics(List.of(new Pair<>(node.hostname(), new NodeMetricSnapshot(clock().instant(), - cpu, - memory, - disk, + load, 0, true, true, @@ -174,11 +172,12 @@ class AutoscalingTester { float cpu = (float) 0.2 * otherResourcesLoad * oneExtraNodeFactor; float memory = value * oneExtraNodeFactor; float disk = (float) ClusterModel.idealDiskLoad * otherResourcesLoad * oneExtraNodeFactor; + Load load = new Load(0.2 * otherResourcesLoad, + value, + ClusterModel.idealDiskLoad * otherResourcesLoad).multiply(oneExtraNodeFactor); nodeMetricsDb().addNodeMetrics(List.of(new Pair<>(node.hostname(), new NodeMetricSnapshot(clock().instant(), - cpu, - memory, - disk, + load, 0, true, true, @@ -199,9 +198,7 @@ class AutoscalingTester { for (Node node : nodes) { nodeMetricsDb().addNodeMetrics(List.of(new Pair<>(node.hostname(), new NodeMetricSnapshot(clock().instant(), - cpu, - memory, - disk, + new Load(cpu, memory, disk), generation, inService, stable, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java index 70550b0a7c3..550ecceee23 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModelTest.java @@ -37,13 +37,13 @@ public class ClusterModelTest { var model1 = new ClusterModel(application.with(new Status(0.0, 1.0)), cluster, clock, Duration.ofMinutes(10), timeseries(cluster,100, t -> t == 0 ? 10000.0 : 0.0, t -> 0.0, clock)); - assertEquals(0.131, model1.idealLoad(Resource.cpu), delta); + assertEquals(0.131, model1.idealLoad().cpu(), delta); // Almost no current traffic share: Ideal load is low but capped var model2 = new ClusterModel(application.with(new Status(0.0001, 1.0)), cluster, clock, Duration.ofMinutes(10), timeseries(cluster,100, t -> t == 0 ? 10000.0 : 0.0, t -> 0.0, clock)); - assertEquals(0.131, model2.idealLoad(Resource.cpu), delta); + assertEquals(0.131, model2.idealLoad().cpu(), delta); } @Test @@ -58,13 +58,13 @@ public class ClusterModelTest { var model1 = new ClusterModel(application, cluster, clock, Duration.ofMinutes(10), timeseries(cluster,100, t -> t == 0 ? 10000.0 : 0.0, t -> 0.0, clock)); - assertEquals(0.275, model1.idealLoad(Resource.cpu), delta); + assertEquals(0.275, model1.idealLoad().cpu(), delta); - // Almost current traffic: Ideal load is low but capped + // Almost no current traffic: Ideal load is low but capped var model2 = new ClusterModel(application.with(new Status(0.0001, 1.0)), cluster, clock, Duration.ofMinutes(10), timeseries(cluster,100, t -> t == 0 ? 10000.0 : 0.0001, t -> 0.0, clock)); - assertEquals(0.275, model1.idealLoad(Resource.cpu), delta); + assertEquals(0.040, model2.idealLoad().cpu(), delta); } private Cluster cluster(NodeResources resources) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java index ef85e228cc7..5f1a36e7b56 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcherTest.java @@ -51,14 +51,14 @@ public class MetricsV2MetricsFetcherTest { assertEquals(2, values.size()); assertEquals("host-1.yahoo.com", values.get(0).getFirst()); - assertEquals(0.162, values.get(0).getSecond().cpu(), delta); - assertEquals(0.231, values.get(0).getSecond().memory(), delta); - assertEquals(0.820, values.get(0).getSecond().disk(), delta); + assertEquals(0.162, values.get(0).getSecond().load().cpu(), delta); + assertEquals(0.231, values.get(0).getSecond().load().memory(), delta); + assertEquals(0.820, values.get(0).getSecond().load().disk(), delta); assertEquals("host-2.yahoo.com", values.get(1).getFirst()); - assertEquals(0.2, values.get(1).getSecond().cpu(), delta); - assertEquals(0.0, values.get(1).getSecond().memory(), delta); - assertEquals(0.4, values.get(1).getSecond().disk(), delta); + assertEquals(0.2, values.get(1).getSecond().load().cpu(), delta); + assertEquals(0.0, values.get(1).getSecond().load().memory(), delta); + assertEquals(0.4, values.get(1).getSecond().load().disk(), delta); assertEquals(45.0, values.get(1).getSecond().queryRate(), delta); } @@ -69,9 +69,9 @@ public class MetricsV2MetricsFetcherTest { httpClient.requestsReceived.get(1)); assertEquals(1, values.size()); assertEquals("host-3.yahoo.com", values.get(0).getFirst()); - assertEquals(0.10, values.get(0).getSecond().cpu(), delta); - assertEquals(0.15, values.get(0).getSecond().memory(), delta); - assertEquals(0.20, values.get(0).getSecond().disk(), delta); + assertEquals(0.10, values.get(0).getSecond().load().cpu(), delta); + assertEquals(0.15, values.get(0).getSecond().load().memory(), delta); + assertEquals(0.20, values.get(0).getSecond().load().disk(), delta); assertEquals(3, values.get(0).getSecond().generation(), delta); assertTrue(values.get(0).getSecond().stable()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java index 330038aa690..ae14b94e619 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java @@ -40,9 +40,7 @@ public class NodeMetricsDbTest { Collection<Pair<String, NodeMetricSnapshot>> values = new ArrayList<>(); for (int i = 0; i < 40; i++) { values.add(new Pair<>(node0, new NodeMetricSnapshot(clock.instant(), - 0.9f, - 0.6f, - 0.6f, + new Load(0.9, 0.6, 0.6), 0, true, false, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java index f465a57d76a..34243f4548f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/QuestMetricsDbTest.java @@ -54,9 +54,9 @@ public class QuestMetricsDbTest { assertEquals(1000, nodeTimeSeries1.get(0).size()); NodeMetricSnapshot snapshot = nodeTimeSeries1.get(0).asList().get(0); assertEquals(startTime.plus(Duration.ofSeconds(1)), snapshot.at()); - assertEquals(0.1, snapshot.cpu(), delta); - assertEquals(0.2, snapshot.memory(), delta); - assertEquals(0.4, snapshot.disk(), delta); + assertEquals(0.1, snapshot.load().cpu(), delta); + assertEquals(0.2, snapshot.load().memory(), delta); + assertEquals(0.4, snapshot.load().disk(), delta); assertEquals(1, snapshot.generation(), delta); assertEquals(30, snapshot.queryRate(), delta); @@ -234,10 +234,8 @@ public class QuestMetricsDbTest { for (int i = 1; i <= countPerHost; i++) { for (String host : hosts) timeseries.add(new Pair<>(host, new NodeMetricSnapshot(clock.instant(), - i * 0.1, - i * 0.2, - i * 0.4, - i % 100, + new Load(i * 0.1, i * 0.2, i * 0.4), + i % 100, true, true, 30.0))); @@ -260,11 +258,8 @@ public class QuestMetricsDbTest { Collection<Pair<String, NodeMetricSnapshot>> timeseries = new ArrayList<>(); for (int i = 1; i <= countPerHost; i++) { for (String host : hosts) - timeseries.add(new Pair<>(host, new NodeMetricSnapshot(at, - i * 0.1, - i * 0.2, - i * 0.4, - i % 100, + timeseries.add(new Pair<>(host, new NodeMetricSnapshot(at, new Load(i * 0.1, i * 0.2, i * 0.4), + i % 100, true, false, 0.0))); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java index 7313fc1797f..4ca12e4ab53 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java @@ -17,6 +17,7 @@ import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.autoscale.ClusterMetricSnapshot; +import com.yahoo.vespa.hosted.provision.autoscale.Load; import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricSnapshot; import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; @@ -74,9 +75,7 @@ public class AutoscalingMaintainerTester { for (Node node : nodes) nodeRepository().metricsDb().addNodeMetrics(List.of(new Pair<>(node.hostname(), new NodeMetricSnapshot(clock().instant(), - cpu, - mem, - disk, + new Load(cpu, mem, disk), generation, true, true, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java index 29a93c463ce..7ab21946379 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java @@ -17,10 +17,8 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Cluster; -import com.yahoo.vespa.hosted.provision.autoscale.ClusterModel; +import com.yahoo.vespa.hosted.provision.autoscale.Load; import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricSnapshot; -import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; -import com.yahoo.vespa.hosted.provision.autoscale.Resource; import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; import org.junit.Test; @@ -124,9 +122,7 @@ public class ScalingSuggestionsMaintainerTest { for (Node node : nodes) nodeRepository.metricsDb().addNodeMetrics(List.of(new Pair<>(node.hostname(), new NodeMetricSnapshot(nodeRepository.clock().instant(), - cpu, - memory, - disk, + new Load(cpu, memory, disk), generation, true, true, diff --git a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp index 7536c6c66cf..fb514063e73 100644 --- a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp +++ b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp @@ -661,7 +661,7 @@ TYPED_TEST(EnumStoreDictionaryTest, find_posting_list_works) this->update_posting_idx(value_0_idx, this->fake_pidx(), EntryRef()); } -TYPED_TEST(EnumStoreDictionaryTest, check_posting_lists_works) +TYPED_TEST(EnumStoreDictionaryTest, normalize_posting_lists_works) { auto value_0_idx = this->insert_value(0); this->update_posting_idx(value_0_idx, EntryRef(), this->fake_pidx()); @@ -674,9 +674,9 @@ TYPED_TEST(EnumStoreDictionaryTest, check_posting_lists_works) auto dummy = [](EntryRef posting_idx) { return posting_idx; }; std::vector<EntryRef> saved_refs; auto save_refs_and_clear = [&saved_refs](EntryRef posting_idx) { saved_refs.push_back(posting_idx); return EntryRef(); }; - EXPECT_FALSE(dict.check_posting_lists(dummy)); - EXPECT_TRUE(dict.check_posting_lists(save_refs_and_clear)); - EXPECT_FALSE(dict.check_posting_lists(save_refs_and_clear)); + EXPECT_FALSE(dict.normalize_posting_lists(dummy)); + EXPECT_TRUE(dict.normalize_posting_lists(save_refs_and_clear)); + EXPECT_FALSE(dict.normalize_posting_lists(save_refs_and_clear)); EXPECT_EQ((std::vector<EntryRef>{ this->fake_pidx(), EntryRef() }), saved_refs); this->store.freeze_dictionary(); root = dict.get_frozen_root(); diff --git a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp index 59313134966..1d0430cfb63 100644 --- a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp @@ -23,9 +23,9 @@ namespace search { using vespalib::btree::BTreeNode; -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> void -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::remove_unused_values(const IndexSet& unused, +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::remove_unused_values(const IndexSet& unused, const vespalib::datastore::EntryComparator& cmp) { if (unused.empty()) { @@ -36,32 +36,32 @@ EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::remove_unused_values(con } } -template <typename DictionaryT, typename UnorderedDictionaryT> -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::EnumStoreDictionary(IEnumStore& enumStore, std::unique_ptr<EntryComparator> compare) +template <typename BTreeDictionaryT, typename HashDictionaryT> +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::EnumStoreDictionary(IEnumStore& enumStore, std::unique_ptr<EntryComparator> compare) : ParentUniqueStoreDictionary(std::move(compare)), _enumStore(enumStore) { } -template <typename DictionaryT, typename UnorderedDictionaryT> -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::~EnumStoreDictionary() = default; +template <typename BTreeDictionaryT, typename HashDictionaryT> +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::~EnumStoreDictionary() = default; -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> void -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::free_unused_values(const vespalib::datastore::EntryComparator& cmp) +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::free_unused_values(const vespalib::datastore::EntryComparator& cmp) { IndexSet unused; // find unused enums - for (auto iter = this->_dict.begin(); iter.valid(); ++iter) { + for (auto iter = this->_btree_dict.begin(); iter.valid(); ++iter) { _enumStore.free_value_if_unused(iter.getKey(), unused); } remove_unused_values(unused, cmp); } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> void -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::free_unused_values(const IndexSet& to_remove, +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::free_unused_values(const IndexSet& to_remove, const vespalib::datastore::EntryComparator& cmp) { IndexSet unused; @@ -71,29 +71,29 @@ EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::free_unused_values(const remove_unused_values(unused, cmp); } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> void -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::remove(const EntryComparator &comp, EntryRef ref) +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::remove(const EntryComparator &comp, EntryRef ref) { assert(ref.valid()); - auto itr = this->_dict.lowerBound(ref, comp); + auto itr = this->_btree_dict.lowerBound(ref, comp); assert(itr.valid() && itr.getKey() == ref); - if constexpr (std::is_same_v<DictionaryT, EnumPostingTree>) { + if constexpr (std::is_same_v<BTreeDictionaryT, EnumPostingTree>) { assert(EntryRef(itr.getData()) == EntryRef()); } - this->_dict.remove(itr); - if constexpr (has_unordered_dictionary) { - auto *result = this->_unordered_dict.remove(comp, ref); + this->_btree_dict.remove(itr); + if constexpr (has_hash_dictionary) { + auto *result = this->_hash_dict.remove(comp, ref); assert(result != nullptr && result->first.load_relaxed() == ref); } } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> bool -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::find_index(const vespalib::datastore::EntryComparator& cmp, +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::find_index(const vespalib::datastore::EntryComparator& cmp, Index& idx) const { - auto itr = this->_dict.find(Index(), cmp); + auto itr = this->_btree_dict.find(Index(), cmp); if (!itr.valid()) { return false; } @@ -101,20 +101,20 @@ EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::find_index(const vespali return true; } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> bool -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::find_frozen_index(const vespalib::datastore::EntryComparator& cmp, +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::find_frozen_index(const vespalib::datastore::EntryComparator& cmp, Index& idx) const { - if constexpr (has_unordered_dictionary) { - auto find_result = this->_unordered_dict.find(cmp, EntryRef()); + if constexpr (has_hash_dictionary) { + auto find_result = this->_hash_dict.find(cmp, EntryRef()); if (find_result != nullptr) { idx = find_result->first.load_acquire(); return true; } return false; } - auto itr = this->_dict.getFrozenView().find(Index(), cmp); + auto itr = this->_btree_dict.getFrozenView().find(Index(), cmp); if (!itr.valid()) { return false; } @@ -122,12 +122,12 @@ EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::find_frozen_index(const return true; } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> std::vector<IEnumStore::EnumHandle> -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::find_matching_enums(const vespalib::datastore::EntryComparator& cmp) const +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::find_matching_enums(const vespalib::datastore::EntryComparator& cmp) const { std::vector<IEnumStore::EnumHandle> result; - auto itr = this->_dict.getFrozenView().find(Index(), cmp); + auto itr = this->_btree_dict.getFrozenView().find(Index(), cmp); while (itr.valid() && !cmp.less(Index(), itr.getKey())) { result.push_back(itr.getKey().ref()); ++itr; @@ -135,11 +135,11 @@ EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::find_matching_enums(cons return result; } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> EntryRef -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::get_frozen_root() const +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::get_frozen_root() const { - return this->_dict.getFrozenView().getRoot(); + return this->_btree_dict.getFrozenView().getRoot(); } template <> @@ -149,18 +149,18 @@ EnumStoreDictionary<EnumTree>::find_posting_list(const vespalib::datastore::Entr LOG_ABORT("should not be reached"); } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> std::pair<IEnumStore::Index, EntryRef> -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::find_posting_list(const vespalib::datastore::EntryComparator& cmp, EntryRef root) const +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::find_posting_list(const vespalib::datastore::EntryComparator& cmp, EntryRef root) const { - if constexpr (has_unordered_dictionary) { - auto find_result = this->_unordered_dict.find(cmp, EntryRef()); + if constexpr (has_hash_dictionary) { + auto find_result = this->_hash_dict.find(cmp, EntryRef()); if (find_result != nullptr) { return std::make_pair(find_result->first.load_acquire(), find_result->second.load_acquire()); } return std::make_pair(Index(), EntryRef()); } - typename DictionaryType::ConstIterator itr(vespalib::btree::BTreeNode::Ref(), this->_dict.getAllocator()); + typename BTreeDictionaryType::ConstIterator itr(vespalib::btree::BTreeNode::Ref(), this->_btree_dict.getAllocator()); itr.lower_bound(root, Index(), cmp); if (itr.valid() && !cmp.less(Index(), itr.getKey())) { return std::make_pair(itr.getKey(), EntryRef(itr.getData())); @@ -168,16 +168,16 @@ EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::find_posting_list(const return std::make_pair(Index(), EntryRef()); } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> void -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::collect_folded(Index idx, EntryRef, const std::function<void(vespalib::datastore::EntryRef)>& callback) const +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::collect_folded(Index idx, EntryRef, const std::function<void(vespalib::datastore::EntryRef)>& callback) const { callback(idx); } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> IEnumStore::Index -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::remap_index(Index idx) +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::remap_index(Index idx) { return idx; } @@ -189,11 +189,11 @@ EnumStoreDictionary<EnumTree>::clear_all_posting_lists(std::function<void(EntryR LOG_ABORT("should not be reached"); } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> void -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::clear_all_posting_lists(std::function<void(EntryRef)> clearer) +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::clear_all_posting_lists(std::function<void(EntryRef)> clearer) { - auto& dict = this->_dict; + auto& dict = this->_btree_dict; auto itr = dict.begin(); EntryRef prev; while (itr.valid()) { @@ -216,19 +216,19 @@ EnumStoreDictionary<EnumTree>::update_posting_list(Index, const vespalib::datast LOG_ABORT("should not be reached"); } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> void -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::update_posting_list(Index idx, const vespalib::datastore::EntryComparator& cmp, std::function<EntryRef(EntryRef)> updater) +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::update_posting_list(Index idx, const vespalib::datastore::EntryComparator& cmp, std::function<EntryRef(EntryRef)> updater) { - auto& dict = this->_dict; + auto& dict = this->_btree_dict; auto itr = dict.lowerBound(idx, cmp); assert(itr.valid() && itr.getKey() == idx); EntryRef old_posting_idx(itr.getData()); EntryRef new_posting_idx = updater(old_posting_idx); dict.thaw(itr); itr.writeData(new_posting_idx.ref()); - if constexpr (has_unordered_dictionary) { - auto find_result = this->_unordered_dict.find(this->_unordered_dict.get_default_comparator(), idx); + if constexpr (has_hash_dictionary) { + auto find_result = this->_hash_dict.find(this->_hash_dict.get_default_comparator(), idx); assert(find_result != nullptr && find_result->first.load_relaxed() == idx); assert(find_result->second.load_relaxed() == old_posting_idx); find_result->second.store_release(new_posting_idx); @@ -237,26 +237,26 @@ EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::update_posting_list(Inde template <> bool -EnumStoreDictionary<EnumTree>::check_posting_lists(std::function<EntryRef(EntryRef)>) +EnumStoreDictionary<EnumTree>::normalize_posting_lists(std::function<EntryRef(EntryRef)>) { LOG_ABORT("should not be reached"); } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> bool -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::check_posting_lists(std::function<EntryRef(EntryRef)> updater) +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::normalize_posting_lists(std::function<EntryRef(EntryRef)> normalize) { bool changed = false; - auto& dict = this->_dict; + auto& dict = this->_btree_dict; for (auto itr = dict.begin(); itr.valid(); ++itr) { EntryRef old_posting_idx(itr.getData()); - EntryRef new_posting_idx = updater(old_posting_idx); + EntryRef new_posting_idx = normalize(old_posting_idx); if (new_posting_idx != old_posting_idx) { changed = true; dict.thaw(itr); itr.writeData(new_posting_idx.ref()); - if constexpr (has_unordered_dictionary) { - auto find_result = this->_unordered_dict.find(this->_unordered_dict.get_default_comparator(), itr.getKey()); + if constexpr (has_hash_dictionary) { + auto find_result = this->_hash_dict.find(this->_hash_dict.get_default_comparator(), itr.getKey()); assert(find_result != nullptr && find_result->first.load_relaxed() == itr.getKey()); assert(find_result->second.load_relaxed() == old_posting_idx); find_result->second.store_release(new_posting_idx); @@ -273,11 +273,11 @@ EnumStoreDictionary<EnumTree>::get_posting_dictionary() const LOG_ABORT("should not be reached"); } -template <typename DictionaryT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename HashDictionaryT> const EnumPostingTree & -EnumStoreDictionary<DictionaryT, UnorderedDictionaryT>::get_posting_dictionary() const +EnumStoreDictionary<BTreeDictionaryT, HashDictionaryT>::get_posting_dictionary() const { - return this->_dict; + return this->_btree_dict; } EnumStoreFoldedDictionary::EnumStoreFoldedDictionary(IEnumStore& enumStore, std::unique_ptr<vespalib::datastore::EntryComparator> compare, std::unique_ptr<EntryComparator> folded_compare) @@ -291,19 +291,19 @@ EnumStoreFoldedDictionary::~EnumStoreFoldedDictionary() = default; UniqueStoreAddResult EnumStoreFoldedDictionary::add(const EntryComparator& comp, std::function<EntryRef(void)> insertEntry) { - static_assert(!has_unordered_dictionary, "Folded Dictionary does not support unordered"); - auto it = _dict.lowerBound(EntryRef(), comp); + static_assert(!has_hash_dictionary, "Folded Dictionary does not support hash dictionary"); + auto it = _btree_dict.lowerBound(EntryRef(), comp); if (it.valid() && !comp.less(EntryRef(), it.getKey())) { // Entry already exists return UniqueStoreAddResult(it.getKey(), false); } EntryRef newRef = insertEntry(); - _dict.insert(it, newRef, EntryRef().ref()); + _btree_dict.insert(it, newRef, EntryRef().ref()); // Maybe move posting list reference from next entry ++it; if (it.valid() && EntryRef(it.getData()).valid() && !_folded_compare->less(newRef, it.getKey())) { EntryRef posting_list_ref(it.getData()); - _dict.thaw(it); + _btree_dict.thaw(it); it.writeData(EntryRef().ref()); --it; assert(it.valid() && it.getKey() == newRef); @@ -315,16 +315,16 @@ EnumStoreFoldedDictionary::add(const EntryComparator& comp, std::function<EntryR void EnumStoreFoldedDictionary::remove(const EntryComparator& comp, EntryRef ref) { - static_assert(!has_unordered_dictionary, "Folded Dictionary does not support unordered"); + static_assert(!has_hash_dictionary, "Folded Dictionary does not support hash dictionary"); assert(ref.valid()); - auto it = _dict.lowerBound(ref, comp); + auto it = _btree_dict.lowerBound(ref, comp); assert(it.valid() && it.getKey() == ref); EntryRef posting_list_ref(it.getData()); - _dict.remove(it); + _btree_dict.remove(it); // Maybe copy posting list reference to next entry if (posting_list_ref.valid()) { if (it.valid() && !EntryRef(it.getData()).valid() && !_folded_compare->less(ref, it.getKey())) { - this->_dict.thaw(it); + this->_btree_dict.thaw(it); it.writeData(posting_list_ref.ref()); } else { LOG_ABORT("Posting list not cleared for removed unique value"); @@ -335,7 +335,7 @@ EnumStoreFoldedDictionary::remove(const EntryComparator& comp, EntryRef ref) void EnumStoreFoldedDictionary::collect_folded(Index idx, EntryRef root, const std::function<void(vespalib::datastore::EntryRef)>& callback) const { - DictionaryType::ConstIterator itr(vespalib::btree::BTreeNode::Ref(), _dict.getAllocator()); + BTreeDictionaryType::ConstIterator itr(vespalib::btree::BTreeNode::Ref(), _btree_dict.getAllocator()); itr.lower_bound(root, idx, *_folded_compare); while (itr.valid() && !_folded_compare->less(idx, itr.getKey())) { callback(itr.getKey()); @@ -346,7 +346,7 @@ EnumStoreFoldedDictionary::collect_folded(Index idx, EntryRef root, const std::f IEnumStore::Index EnumStoreFoldedDictionary::remap_index(Index idx) { - auto itr = _dict.find(idx, *_folded_compare); + auto itr = _btree_dict.find(idx, *_folded_compare); assert(itr.valid()); return itr.getKey(); } diff --git a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h index 6329288f874..acb85c4e7f2 100644 --- a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h +++ b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h @@ -12,20 +12,20 @@ class IEnumStore; /** * Concrete dictionary for an enum store that extends the functionality of a unique store dictionary. */ -template <typename DictionaryT, typename UnorderedDictionaryT = vespalib::datastore::NoUnorderedDictionary> -class EnumStoreDictionary : public vespalib::datastore::UniqueStoreDictionary<DictionaryT, IEnumStoreDictionary, UnorderedDictionaryT> { +template <typename BTreeDictionaryT, typename HashDictionaryT = vespalib::datastore::NoHashDictionary> +class EnumStoreDictionary : public vespalib::datastore::UniqueStoreDictionary<BTreeDictionaryT, IEnumStoreDictionary, HashDictionaryT> { protected: using EntryRef = IEnumStoreDictionary::EntryRef; using Index = IEnumStoreDictionary::Index; - using DictionaryType = DictionaryT; + using BTreeDictionaryType = BTreeDictionaryT; private: using EnumVector = IEnumStoreDictionary::EnumVector; using IndexSet = IEnumStoreDictionary::IndexSet; using IndexVector = IEnumStoreDictionary::IndexVector; - using ParentUniqueStoreDictionary = vespalib::datastore::UniqueStoreDictionary<DictionaryT, IEnumStoreDictionary, UnorderedDictionaryT>; + using ParentUniqueStoreDictionary = vespalib::datastore::UniqueStoreDictionary<BTreeDictionaryT, IEnumStoreDictionary, HashDictionaryT>; using generation_t = IEnumStoreDictionary::generation_t; protected: - using ParentUniqueStoreDictionary::has_unordered_dictionary; + using ParentUniqueStoreDictionary::has_hash_dictionary; private: IEnumStore& _enumStore; @@ -37,7 +37,7 @@ public: ~EnumStoreDictionary() override; - const DictionaryT& get_raw_dictionary() const { return this->_dict; } + const BTreeDictionaryT& get_raw_dictionary() const { return this->_btree_dict; } void free_unused_values(const vespalib::datastore::EntryComparator& cmp) override; @@ -56,7 +56,7 @@ public: Index remap_index(Index idx) override; void clear_all_posting_lists(std::function<void(EntryRef)> clearer) override; void update_posting_list(Index idx, const vespalib::datastore::EntryComparator& cmp, std::function<EntryRef(EntryRef)> updater) override; - bool check_posting_lists(std::function<EntryRef(EntryRef)> updater) override; + bool normalize_posting_lists(std::function<EntryRef(EntryRef)> normalize) override; const EnumPostingTree& get_posting_dictionary() const override; }; diff --git a/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h b/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h index f452ef01e5e..f816177b06c 100644 --- a/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h +++ b/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h @@ -52,7 +52,7 @@ public: virtual Index remap_index(Index idx) = 0; virtual void clear_all_posting_lists(std::function<void(EntryRef)> clearer) = 0; virtual void update_posting_list(Index idx, const vespalib::datastore::EntryComparator& cmp, std::function<EntryRef(EntryRef)> updater) = 0; - virtual bool check_posting_lists(std::function<EntryRef(EntryRef)> updater) = 0; + virtual bool normalize_posting_lists(std::function<EntryRef(EntryRef)> normalize) = 0; virtual const EnumPostingTree& get_posting_dictionary() const = 0; }; diff --git a/searchlib/src/vespa/searchlib/attribute/postingstore.cpp b/searchlib/src/vespa/searchlib/attribute/postingstore.cpp index d1a25396c09..fee2520b132 100644 --- a/searchlib/src/vespa/searchlib/attribute/postingstore.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postingstore.cpp @@ -124,8 +124,8 @@ PostingStore<DataT>::removeSparseBitVectors() } } if (needscan) { - res = _dictionary.check_posting_lists([this](EntryRef posting_idx) -> EntryRef - { return consider_remove_sparse_bitvector(posting_idx); }); + res = _dictionary.normalize_posting_lists([this](EntryRef posting_idx) -> EntryRef + { return consider_remove_sparse_bitvector(posting_idx); }); } return res; } diff --git a/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp b/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp index 2e1eea55fe1..25ed566f57a 100644 --- a/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp +++ b/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp @@ -13,7 +13,7 @@ #include <vespa/log/log.h> LOG_SETUP("unique_store_test"); -enum class Ordering { ORDERED, UNORDERED }; +enum class DictionaryType { BTREE, HASH, BTREE_AND_HASH }; using namespace vespalib::datastore; using vespalib::ArrayRef; @@ -27,9 +27,9 @@ struct TestBaseValues { static std::vector<ValueType> values; }; -template <typename UniqueStoreTypeAndOrder> +template <typename UniqueStoreTypeAndDictionaryType> struct TestBase : public ::testing::Test { - using UniqueStoreType = typename UniqueStoreTypeAndOrder::UniqueStoreType; + using UniqueStoreType = typename UniqueStoreTypeAndDictionaryType::UniqueStoreType; using EntryRefType = typename UniqueStoreType::RefType; using ValueType = typename UniqueStoreType::EntryType; using ValueConstRefType = typename UniqueStoreType::EntryConstRefType; @@ -142,22 +142,22 @@ struct TestBase : public ::testing::Test { } }; -template <typename UniqueStoreTypeAndOrder> -TestBase<UniqueStoreTypeAndOrder>::TestBase() +template <typename UniqueStoreTypeAndDictionaryType> +TestBase<UniqueStoreTypeAndDictionaryType>::TestBase() : store(), refStore(), generation(1) { - switch (UniqueStoreTypeAndOrder::ordering) { - case Ordering::ORDERED: + switch (UniqueStoreTypeAndDictionaryType::dictionary_type) { + case DictionaryType::BTREE: break; default: store.set_dictionary(std::make_unique<UniqueStoreDictionary<uniquestore::DefaultDictionary, IUniqueStoreDictionary, SimpleHashMap>>(std::make_unique<CompareType>(store.get_data_store()))); } } -template <typename UniqueStoreTypeAndOrder> -TestBase<UniqueStoreTypeAndOrder>::~TestBase() = default; +template <typename UniqueStoreTypeAndDictionaryType> +TestBase<UniqueStoreTypeAndDictionaryType>::~TestBase() = default; using NumberUniqueStore = UniqueStore<uint32_t>; using StringUniqueStore = UniqueStore<std::string>; @@ -177,61 +177,61 @@ std::vector<double> TestBaseValues<DoubleUniqueStore>::values{ 10.0, 20.0, 30.0, struct OrderedNumberUniqueStore { using UniqueStoreType = NumberUniqueStore; - static constexpr Ordering ordering = Ordering::ORDERED; + static constexpr DictionaryType dictionary_type = DictionaryType::BTREE; }; struct OrderedStringUniqueStore { using UniqueStoreType = StringUniqueStore; - static constexpr Ordering ordering = Ordering::ORDERED; + static constexpr DictionaryType dictionary_type = DictionaryType::BTREE; }; struct OrderedCStringUniqueStore { using UniqueStoreType = CStringUniqueStore; - static constexpr Ordering ordering = Ordering::ORDERED; + static constexpr DictionaryType dictionary_type = DictionaryType::BTREE; }; struct OrderedDoubleUniqueStore { using UniqueStoreType = DoubleUniqueStore; - static constexpr Ordering ordering = Ordering::ORDERED; + static constexpr DictionaryType dictionary_type = DictionaryType::BTREE; }; struct OrderedSmallOffsetNumberUniqueStore { using UniqueStoreType = SmallOffsetNumberUniqueStore; - static constexpr Ordering ordering = Ordering::ORDERED; + static constexpr DictionaryType dictionary_type = DictionaryType::BTREE; }; struct UnorderedNumberUniqueStore { using UniqueStoreType = NumberUniqueStore; - static constexpr Ordering ordering = Ordering::UNORDERED; + static constexpr DictionaryType dictionary_type = DictionaryType::BTREE_AND_HASH; }; struct UnorderedStringUniqueStore { using UniqueStoreType = StringUniqueStore; - static constexpr Ordering ordering = Ordering::UNORDERED; + static constexpr DictionaryType dictionary_type = DictionaryType::BTREE_AND_HASH; }; struct UnorderedCStringUniqueStore { using UniqueStoreType = CStringUniqueStore; - static constexpr Ordering ordering = Ordering::UNORDERED; + static constexpr DictionaryType dictionary_type = DictionaryType::BTREE_AND_HASH; }; struct UnorderedDoubleUniqueStore { using UniqueStoreType = DoubleUniqueStore; - static constexpr Ordering ordering = Ordering::UNORDERED; + static constexpr DictionaryType dictionary_type = DictionaryType::BTREE_AND_HASH; }; struct UnorderedSmallOffsetNumberUniqueStore { using UniqueStoreType = SmallOffsetNumberUniqueStore; - static constexpr Ordering ordering = Ordering::UNORDERED; + static constexpr DictionaryType dictionary_type = DictionaryType::BTREE_AND_HASH; }; using UniqueStoreTestTypes = ::testing::Types<OrderedNumberUniqueStore, OrderedStringUniqueStore, OrderedCStringUniqueStore, OrderedDoubleUniqueStore, UnorderedNumberUniqueStore, UnorderedStringUniqueStore, UnorderedCStringUniqueStore, UnorderedDoubleUniqueStore>; diff --git a/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h b/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h index bc686d4804a..d31139efc76 100644 --- a/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h +++ b/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h @@ -48,7 +48,7 @@ public: virtual void build(vespalib::ConstArrayRef<EntryRef> refs) = 0; virtual void build_with_payload(vespalib::ConstArrayRef<EntryRef> refs, vespalib::ConstArrayRef<uint32_t> payloads) = 0; virtual std::unique_ptr<ReadSnapshot> get_read_snapshot() const = 0; - virtual bool get_has_unordered_dictionary() const = 0; + virtual bool get_has_hash_dictionary() const = 0; }; } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h index c24f4f237c3..8fbff3f9d93 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h @@ -9,27 +9,40 @@ namespace vespalib::datastore { class EntryComparatorWrapper; -class NoUnorderedDictionary; +class NoHashDictionary; -template <typename UnorderedDictionaryT> -class UniqueStoreUnorderedDictionaryBase +template <typename BTreeDictionaryT> +class UniqueStoreBTreeDictionaryBase { protected: - UnorderedDictionaryT _unordered_dict; + BTreeDictionaryT _btree_dict; public: - static constexpr bool has_unordered_dictionary = true; - UniqueStoreUnorderedDictionaryBase(std::unique_ptr<EntryComparator> compare) - : _unordered_dict(std::move(compare)) + static constexpr bool has_btree_dictionary = true; + UniqueStoreBTreeDictionaryBase() + : _btree_dict() + { + } +}; + +template <typename HashDictionaryT> +class UniqueStoreHashDictionaryBase +{ +protected: + HashDictionaryT _hash_dict; +public: + static constexpr bool has_hash_dictionary = true; + UniqueStoreHashDictionaryBase(std::unique_ptr<EntryComparator> compare) + : _hash_dict(std::move(compare)) { } }; template <> -class UniqueStoreUnorderedDictionaryBase<NoUnorderedDictionary> +class UniqueStoreHashDictionaryBase<NoHashDictionary> { public: - static constexpr bool has_unordered_dictionary = false; - UniqueStoreUnorderedDictionaryBase(std::unique_ptr<EntryComparator>) + static constexpr bool has_hash_dictionary = false; + UniqueStoreHashDictionaryBase(std::unique_ptr<EntryComparator>) { } }; @@ -37,12 +50,12 @@ public: /** * A dictionary for unique store. Mostly accessed via base class. */ -template <typename DictionaryT, typename ParentT = IUniqueStoreDictionary, typename UnorderedDictionaryT = NoUnorderedDictionary> -class UniqueStoreDictionary : public ParentT, public UniqueStoreUnorderedDictionaryBase<UnorderedDictionaryT> { +template <typename BTreeDictionaryT, typename ParentT = IUniqueStoreDictionary, typename HashDictionaryT = NoHashDictionary> +class UniqueStoreDictionary : public ParentT, public UniqueStoreBTreeDictionaryBase<BTreeDictionaryT>, public UniqueStoreHashDictionaryBase<HashDictionaryT> { protected: - using DictionaryType = DictionaryT; - using DataType = typename DictionaryType::DataType; - using FrozenView = typename DictionaryType::FrozenView; + using BTreeDictionaryType = BTreeDictionaryT; + using DataType = typename BTreeDictionaryType::DataType; + using FrozenView = typename BTreeDictionaryType::FrozenView; using ReadSnapshot = typename ParentT::ReadSnapshot; using generation_t = typename ParentT::generation_t; @@ -57,11 +70,9 @@ protected: void foreach_key(std::function<void(EntryRef)> callback) const override; }; - DictionaryType _dict; - public: - using UniqueStoreUnorderedDictionaryBase<UnorderedDictionaryT>::has_unordered_dictionary; - static constexpr bool has_ordered_dictionary = true; + using UniqueStoreBTreeDictionaryBase<BTreeDictionaryT>::has_btree_dictionary; + using UniqueStoreHashDictionaryBase<HashDictionaryT>::has_hash_dictionary; UniqueStoreDictionary(std::unique_ptr<EntryComparator> compare); ~UniqueStoreDictionary() override; void freeze() override; @@ -77,7 +88,7 @@ public: void build(vespalib::ConstArrayRef<EntryRef> refs) override; void build_with_payload(vespalib::ConstArrayRef<EntryRef>, vespalib::ConstArrayRef<uint32_t> payloads) override; std::unique_ptr<ReadSnapshot> get_read_snapshot() const override; - bool get_has_unordered_dictionary() const override; + bool get_has_hash_dictionary() const override; }; } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp index b6378d3bd75..ae8a85985fd 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp @@ -16,16 +16,16 @@ namespace vespalib::datastore { -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>:: +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>:: ReadSnapshotImpl::ReadSnapshotImpl(FrozenView frozen_view) : _frozen_view(frozen_view) { } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> size_t -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>:: +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>:: ReadSnapshotImpl::count(const EntryComparator& comp) const { auto itr = _frozen_view.lowerBound(EntryRef(), comp); @@ -35,9 +35,9 @@ ReadSnapshotImpl::count(const EntryComparator& comp) const return 0u; } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> size_t -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>:: +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>:: ReadSnapshotImpl::count_in_range(const EntryComparator& low, const EntryComparator& high) const { @@ -49,124 +49,124 @@ ReadSnapshotImpl::count_in_range(const EntryComparator& low, return high_itr - low_itr; } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> void -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>:: +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>:: ReadSnapshotImpl::foreach_key(std::function<void(EntryRef)> callback) const { _frozen_view.foreach_key(callback); } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::UniqueStoreDictionary(std::unique_ptr<EntryComparator> compare) +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::UniqueStoreDictionary(std::unique_ptr<EntryComparator> compare) : ParentT(), - UniqueStoreUnorderedDictionaryBase<UnorderedDictionaryT>(std::move(compare)), - _dict() + UniqueStoreBTreeDictionaryBase<BTreeDictionaryT>(), + UniqueStoreHashDictionaryBase<HashDictionaryT>(std::move(compare)) { } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::~UniqueStoreDictionary() = default; +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::~UniqueStoreDictionary() = default; -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> void -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::freeze() +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::freeze() { - _dict.getAllocator().freeze(); + this->_btree_dict.getAllocator().freeze(); } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> void -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::transfer_hold_lists(generation_t generation) +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::transfer_hold_lists(generation_t generation) { - _dict.getAllocator().transferHoldLists(generation); - if constexpr (has_unordered_dictionary) { - this->_unordered_dict.transfer_hold_lists(generation); + this->_btree_dict.getAllocator().transferHoldLists(generation); + if constexpr (has_hash_dictionary) { + this->_hash_dict.transfer_hold_lists(generation); } } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> void -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::trim_hold_lists(generation_t firstUsed) +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::trim_hold_lists(generation_t firstUsed) { - _dict.getAllocator().trimHoldLists(firstUsed); - if constexpr (has_unordered_dictionary) { - this->_unordered_dict.trim_hold_lists(firstUsed); + this->_btree_dict.getAllocator().trimHoldLists(firstUsed); + if constexpr (has_hash_dictionary) { + this->_hash_dict.trim_hold_lists(firstUsed); } } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> UniqueStoreAddResult -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::add(const EntryComparator &comp, +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::add(const EntryComparator &comp, std::function<EntryRef(void)> insertEntry) { - auto itr = _dict.lowerBound(EntryRef(), comp); + auto itr = this->_btree_dict.lowerBound(EntryRef(), comp); if (itr.valid() && !comp.less(EntryRef(), itr.getKey())) { - if constexpr (has_unordered_dictionary) { - auto* result = this->_unordered_dict.find(comp, EntryRef()); + if constexpr (has_hash_dictionary) { + auto* result = this->_hash_dict.find(comp, EntryRef()); assert(result != nullptr && result->first.load_relaxed() == itr.getKey()); } return UniqueStoreAddResult(itr.getKey(), false); } else { EntryRef newRef = insertEntry(); - _dict.insert(itr, newRef, DataType()); - if constexpr (has_unordered_dictionary) { - std::function<EntryRef(void)> insert_unordered_entry([newRef]() noexcept -> EntryRef { return newRef; }); - auto& add_result = this->_unordered_dict.add(comp, newRef, insert_unordered_entry); + this->_btree_dict.insert(itr, newRef, DataType()); + if constexpr (has_hash_dictionary) { + std::function<EntryRef(void)> insert_hash_entry([newRef]() noexcept -> EntryRef { return newRef; }); + auto& add_result = this->_hash_dict.add(comp, newRef, insert_hash_entry); assert(add_result.first.load_relaxed() == newRef); } return UniqueStoreAddResult(newRef, true); } } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> EntryRef -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::find(const EntryComparator &comp) +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::find(const EntryComparator &comp) { - auto itr = _dict.lowerBound(EntryRef(), comp); + auto itr = this->_btree_dict.lowerBound(EntryRef(), comp); if (itr.valid() && !comp.less(EntryRef(), itr.getKey())) { - if constexpr (has_unordered_dictionary) { - auto* result = this->_unordered_dict.find(comp, EntryRef()); + if constexpr (has_hash_dictionary) { + auto* result = this->_hash_dict.find(comp, EntryRef()); assert(result != nullptr && result->first.load_relaxed() == itr.getKey()); } return itr.getKey(); } else { - if constexpr (has_unordered_dictionary) { - auto* result = this->_unordered_dict.find(comp, EntryRef()); + if constexpr (has_hash_dictionary) { + auto* result = this->_hash_dict.find(comp, EntryRef()); assert(result == nullptr); } return EntryRef(); } } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> void -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::remove(const EntryComparator &comp, EntryRef ref) +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::remove(const EntryComparator &comp, EntryRef ref) { assert(ref.valid()); - auto itr = _dict.lowerBound(ref, comp); + auto itr = this->_btree_dict.lowerBound(ref, comp); assert(itr.valid() && itr.getKey() == ref); - _dict.remove(itr); - if constexpr (has_unordered_dictionary) { - auto *result = this->_unordered_dict.remove(comp, ref); + this->_btree_dict.remove(itr); + if constexpr (has_hash_dictionary) { + auto *result = this->_hash_dict.remove(comp, ref); assert(result != nullptr && result->first.load_relaxed() == ref); } } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> void -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::move_entries(ICompactable &compactable) +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::move_entries(ICompactable &compactable) { - auto itr = _dict.begin(); + auto itr = this->_btree_dict.begin(); while (itr.valid()) { EntryRef oldRef(itr.getKey()); EntryRef newRef(compactable.move(oldRef)); if (newRef != oldRef) { - _dict.thaw(itr); + this->_btree_dict.thaw(itr); itr.writeKey(newRef); - if constexpr (has_unordered_dictionary) { - auto result = this->_unordered_dict.find(this->_unordered_dict.get_default_comparator(), oldRef); + if constexpr (has_hash_dictionary) { + auto result = this->_hash_dict.find(this->_hash_dict.get_default_comparator(), oldRef); assert(result != nullptr && result->first.load_relaxed() == oldRef); result->first.store_release(newRef); } @@ -175,29 +175,29 @@ UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::move_entries( } } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> uint32_t -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::get_num_uniques() const +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::get_num_uniques() const { - return _dict.size(); + return this->_btree_dict.size(); } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> vespalib::MemoryUsage -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::get_memory_usage() const +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::get_memory_usage() const { - return _dict.getMemoryUsage(); + return this->_btree_dict.getMemoryUsage(); } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> void -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::build(vespalib::ConstArrayRef<EntryRef> refs, +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::build(vespalib::ConstArrayRef<EntryRef> refs, vespalib::ConstArrayRef<uint32_t> ref_counts, std::function<void(EntryRef)> hold) { assert(refs.size() == ref_counts.size()); assert(!refs.empty()); - typename DictionaryType::Builder builder(_dict.getAllocator()); + typename BTreeDictionaryType::Builder builder(this->_btree_dict.getAllocator()); for (size_t i = 1; i < refs.size(); ++i) { if (ref_counts[i] != 0u) { builder.insert(refs[i], DataType()); @@ -205,44 +205,44 @@ UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::build(vespali hold(refs[i]); } } - _dict.assign(builder); - if constexpr (has_unordered_dictionary) { + this->_btree_dict.assign(builder); + if constexpr (has_hash_dictionary) { for (size_t i = 1; i < refs.size(); ++i) { if (ref_counts[i] != 0u) { EntryRef ref = refs[i]; - std::function<EntryRef(void)> insert_unordered_entry([ref]() noexcept -> EntryRef { return ref; }); - auto& add_result = this->_unordered_dict.add(this->_unordered_dict.get_default_comparator(), ref, insert_unordered_entry); + std::function<EntryRef(void)> insert_hash_entry([ref]() noexcept -> EntryRef { return ref; }); + auto& add_result = this->_hash_dict.add(this->_hash_dict.get_default_comparator(), ref, insert_hash_entry); assert(add_result.first.load_relaxed() == ref); } } } } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> void -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::build(vespalib::ConstArrayRef<EntryRef> refs) +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::build(vespalib::ConstArrayRef<EntryRef> refs) { - typename DictionaryType::Builder builder(_dict.getAllocator()); + typename BTreeDictionaryType::Builder builder(this->_btree_dict.getAllocator()); for (const auto& ref : refs) { builder.insert(ref, DataType()); } - _dict.assign(builder); - if constexpr (has_unordered_dictionary) { + this->_btree_dict.assign(builder); + if constexpr (has_hash_dictionary) { for (const auto& ref : refs) { - std::function<EntryRef(void)> insert_unordered_entry([ref]() noexcept -> EntryRef { return ref; }); - auto& add_result = this->_unordered_dict.add(this->_unordered_dict.get_default_comparator(), ref, insert_unordered_entry); + std::function<EntryRef(void)> insert_hash_entry([ref]() noexcept -> EntryRef { return ref; }); + auto& add_result = this->_hash_dict.add(this->_hash_dict.get_default_comparator(), ref, insert_hash_entry); assert(add_result.first.load_relaxed() == ref); } } } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> void -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::build_with_payload(vespalib::ConstArrayRef<EntryRef> refs, +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::build_with_payload(vespalib::ConstArrayRef<EntryRef> refs, vespalib::ConstArrayRef<uint32_t> payloads) { assert(refs.size() == payloads.size()); - typename DictionaryType::Builder builder(_dict.getAllocator()); + typename BTreeDictionaryType::Builder builder(this->_btree_dict.getAllocator()); for (size_t i = 0; i < refs.size(); ++i) { if constexpr (std::is_same_v<DataType, uint32_t>) { builder.insert(refs[i], payloads[i]); @@ -250,12 +250,12 @@ UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::build_with_pa builder.insert(refs[i], DataType()); } } - _dict.assign(builder); - if constexpr (has_unordered_dictionary) { + this->_btree_dict.assign(builder); + if constexpr (has_hash_dictionary) { for (size_t i = 0; i < refs.size(); ++i) { EntryRef ref = refs[i]; - std::function<EntryRef(void)> insert_unordered_entry([ref]() noexcept -> EntryRef { return ref; }); - auto& add_result = this->_unordered_dict.add(this->_unordered_dict.get_default_comparator(), ref, insert_unordered_entry); + std::function<EntryRef(void)> insert_hash_entry([ref]() noexcept -> EntryRef { return ref; }); + auto& add_result = this->_hash_dict.add(this->_hash_dict.get_default_comparator(), ref, insert_hash_entry); assert(add_result.first.load_relaxed() == refs[i]); if constexpr (std::is_same_v<DataType, uint32_t>) { add_result.second.store_relaxed(EntryRef(payloads[i])); @@ -264,18 +264,18 @@ UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::build_with_pa } } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> std::unique_ptr<typename ParentT::ReadSnapshot> -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::get_read_snapshot() const +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::get_read_snapshot() const { - return std::make_unique<ReadSnapshotImpl>(_dict.getFrozenView()); + return std::make_unique<ReadSnapshotImpl>(this->_btree_dict.getFrozenView()); } -template <typename DictionaryT, typename ParentT, typename UnorderedDictionaryT> +template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT> bool -UniqueStoreDictionary<DictionaryT, ParentT, UnorderedDictionaryT>::get_has_unordered_dictionary() const +UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::get_has_hash_dictionary() const { - return has_unordered_dictionary; + return has_hash_dictionary; } } diff --git a/zkfacade/pom.xml b/zkfacade/pom.xml index 7563ef068a9..25cc678fe66 100644 --- a/zkfacade/pom.xml +++ b/zkfacade/pom.xml @@ -64,7 +64,7 @@ <!-- Needed to have the same version as slf4j-api --> <groupId>org.slf4j</groupId> <artifactId>slf4j-log4j12</artifactId> - <version>1.7.5</version> + <version>${slf4j.version}</version> </dependency> <dependency> <groupId>org.apache.zookeeper</groupId> diff --git a/zookeeper-command-line-client/pom.xml b/zookeeper-command-line-client/pom.xml index c186b377eb6..50301914e41 100644 --- a/zookeeper-command-line-client/pom.xml +++ b/zookeeper-command-line-client/pom.xml @@ -36,7 +36,7 @@ <dependency> <groupId>org.slf4j</groupId> <artifactId>slf4j-api</artifactId> - <version>1.7.25</version> + <scope>compile</scope> </dependency> </dependencies> <build> diff --git a/zookeeper-server/zookeeper-server-3.6.2/pom.xml b/zookeeper-server/zookeeper-server-3.6.2/pom.xml index 04f8b012b09..2f0029169ea 100644 --- a/zookeeper-server/zookeeper-server-3.6.2/pom.xml +++ b/zookeeper-server/zookeeper-server-3.6.2/pom.xml @@ -41,7 +41,7 @@ <dependency> <groupId>org.slf4j</groupId> <artifactId>slf4j-log4j12</artifactId> - <version>1.7.5</version> + <version>${slf4j.version}</version> </dependency> <!-- snappy-java and metrics-core are included here to be able to work with ZooKeeper 3.6.2 due to |