diff options
90 files changed, 1250 insertions, 831 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java index ca3169ede6f..918f01eef16 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 @@ -334,13 +334,13 @@ public class NodeStateChangeChecker { for (DistributorNodeInfo distributorNodeInfo : clusterInfo.getDistributorNodeInfo()) { Integer distributorClusterStateVersion = distributorNodeInfo.getHostInfo().getClusterStateVersionOrNull(); if (distributorClusterStateVersion == null) { - return Result.createDisallowed("Distributor node (" + distributorNodeInfo.getNodeIndex() - + ") has not reported any cluster state version yet."); + return Result.createDisallowed("Distributor node " + distributorNodeInfo.getNodeIndex() + + " has not reported any cluster state version yet."); } else if (distributorClusterStateVersion != clusterStateVersion) { - return Result.createDisallowed("Distributor node (" + distributorNodeInfo.getNodeIndex() - + ") does not report same version (" - + distributorNodeInfo.getHostInfo().getClusterStateVersionOrNull() - + ") as fleetcontroller has (" + clusterStateVersion + ")"); + return Result.createDisallowed("Distributor node " + distributorNodeInfo.getNodeIndex() + + " does not report same version (" + + distributorNodeInfo.getHostInfo().getClusterStateVersionOrNull() + + ") as fleetcontroller (" + clusterStateVersion + ")"); } List<StorageNode> storageNodes = distributorNodeInfo.getHostInfo().getDistributor().getStorageNodes(); 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 3f654d36246..41284cc95d0 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 @@ -273,7 +273,7 @@ public class NodeStateChangeCheckerTest { nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); assertFalse(result.settingWantedStateIsAllowed()); assertFalse(result.wantedStateAlreadySet()); - assertThat(result.getReason(), is("Distributor node (0) has not reported any cluster state version yet.")); + assertThat(result.getReason(), is("Distributor node 0 has not reported any cluster state version yet.")); } private NodeStateChangeChecker.Result transitionToSameState(State state, String oldDescription, String newDescription) { diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index 53ecc5892b6..91ecb981e12 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -82,10 +82,10 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"baldersheim"}) default double feedConcurrency() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useBucketExecutorForLidSpaceCompact() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useBucketExecutorForBucketMove() { throw new UnsupportedOperationException("TODO specify default value"); } - @ModelFeatureFlag(owners = {"musum", "mpolden"}, comment = "Revisit in February 2021") default boolean reconfigurableZookeeperServer() { return false; } + @ModelFeatureFlag(owners = {"musum", "mpolden"}, comment = "Revisit in February 2021", removeAfter = "7.370") default boolean reconfigurableZookeeperServer() { return true; } @ModelFeatureFlag(owners = {"geirst"}) default boolean enableFeedBlockInDistributor() { return false; } @ModelFeatureFlag(owners = {"baldersheim", "geirst", "toregge"}) default double maxDeadBytesRatio() { return 0.2; } - @ModelFeatureFlag(owners = {"hmusum"}) default int clusterControllerMaxHeapSizeInMb() { return 512; } + @ModelFeatureFlag(owners = {"hmusum"}) default int clusterControllerMaxHeapSizeInMb() { return 256; } @ModelFeatureFlag(owners = {"hmusum"}) default int metricsProxyMaxHeapSizeInMb(ClusterSpec.Type type) { return 512; } @ModelFeatureFlag(owners = {"bjorncs", "tokle"}) default List<String> allowedAthenzProxyIdentities() { return List.of(); } @ModelFeatureFlag(owners = {"tokle"}) default boolean tenantIamRole() { return false; } diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index be15abc5ac8..a95e924cb57 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -51,12 +51,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private boolean useAccessControlTlsHandshakeClientAuth; private boolean useAsyncMessageHandlingOnSchedule = false; private double feedConcurrency = 0.5; - private boolean reconfigurableZookeeperServer = false; private boolean useBucketExecutorForLidSpaceCompact; private boolean useBucketExecutorForBucketMove; private boolean enableFeedBlockInDistributor = false; private double maxDeadBytesRatio = 0.2; - private int clusterControllerMaxHeapSizeInMb = 512; + private int clusterControllerMaxHeapSizeInMb = 256; private int metricsProxyMaxHeapSizeInMb = 512; private int maxActivationInhibitedOutOfSyncGroups = 0; @@ -91,7 +90,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public boolean useAccessControlTlsHandshakeClientAuth() { return useAccessControlTlsHandshakeClientAuth; } @Override public boolean useAsyncMessageHandlingOnSchedule() { return useAsyncMessageHandlingOnSchedule; } @Override public double feedConcurrency() { return feedConcurrency; } - @Override public boolean reconfigurableZookeeperServer() { return reconfigurableZookeeperServer; } @Override public boolean useBucketExecutorForLidSpaceCompact() { return useBucketExecutorForLidSpaceCompact; } @Override public boolean useBucketExecutorForBucketMove() { return useBucketExecutorForBucketMove; } @Override public boolean enableFeedBlockInDistributor() { return enableFeedBlockInDistributor; } @@ -200,11 +198,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea return this; } - public TestProperties reconfigurableZookeeperServer(boolean enabled) { - this.reconfigurableZookeeperServer = enabled; - return this; - } - public TestProperties useBucketExecutorForLidSpaceCompact(boolean enabled) { useBucketExecutorForLidSpaceCompact = enabled; return this; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java index 916c65e5f82..e0886fb8b24 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java @@ -1,4 +1,4 @@ -// Copyright 2019 Oath Inc. 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.model.admin; import com.yahoo.config.model.deploy.DeployState; @@ -27,7 +27,7 @@ public class LogserverContainerCluster extends ContainerCluster<LogserverContain public void getConfig(QrStartConfig.Builder builder) { super.getConfig(builder); builder.jvm.heapsize(384) - .verbosegc(true); + .verbosegc(true); } protected boolean messageBusEnabled() { return false; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java index f55d179b184..f1717625d10 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.model.admin.clustercontroller; import com.yahoo.cloud.config.ZookeeperServerConfig; import com.yahoo.component.ComponentSpecification; -import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.api.container.ContainerServiceType; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; @@ -41,16 +40,13 @@ public class ClusterControllerContainer extends Container implements private static final ComponentSpecification REINDEXING_CONTROLLER_BUNDLE = new ComponentSpecification("clustercontroller-reindexer"); private final Set<String> bundles = new TreeSet<>(); - private final ModelContext.FeatureFlags featureFlags; - - public ClusterControllerContainer( - AbstractConfigProducer<?> parent, - int index, - boolean runStandaloneZooKeeper, - DeployState deployState, - boolean retired) { + + public ClusterControllerContainer(AbstractConfigProducer<?> parent, + int index, + boolean runStandaloneZooKeeper, + DeployState deployState, + boolean retired) { super(parent, "" + index, retired, index, deployState.isHosted()); - this.featureFlags = deployState.featureFlags(); addHandler("clustercontroller-status", "com.yahoo.vespa.clustercontroller.apps.clustercontroller.StatusHandler", "/clustercontroller-status/*", @@ -70,7 +66,7 @@ public class ClusterControllerContainer extends Container implements addFileBundle("clustercontroller-utils"); addFileBundle("zookeeper-server"); configureReindexing(); - configureZooKeeperServer(runStandaloneZooKeeper, deployState.featureFlags().reconfigurableZookeeperServer()); + configureZooKeeperServer(runStandaloneZooKeeper); } @Override @@ -88,16 +84,13 @@ public class ClusterControllerContainer extends Container implements return ContainerServiceType.CLUSTERCONTROLLER_CONTAINER; } - private void configureZooKeeperServer(boolean runStandaloneZooKeeper, boolean reconfigurable) { - if (reconfigurable && runStandaloneZooKeeper) { + private void configureZooKeeperServer(boolean runStandaloneZooKeeper) { + if (runStandaloneZooKeeper) ContainerModelBuilder.addReconfigurableZooKeeperServerComponents(this); - } else { + else addComponent("clustercontroller-zookeeper-server", - runStandaloneZooKeeper - ? "com.yahoo.vespa.zookeeper.VespaZooKeeperServerImpl" - : "com.yahoo.vespa.zookeeper.DummyVespaZooKeeperServer", + "com.yahoo.vespa.zookeeper.DummyVespaZooKeeperServer", ZOOKEEPER_SERVER_BUNDLE); - } } private void addHandler(Handler<?> h, String path) { @@ -148,7 +141,7 @@ public class ClusterControllerContainer extends Container implements @Override public void getConfig(ZookeeperServerConfig.Builder builder) { builder.myid(index()); - builder.dynamicReconfiguration(featureFlags.reconfigurableZookeeperServer()); + builder.dynamicReconfiguration(true); } @Override diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java index 4fc73de3b48..abdbb3c90a3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java @@ -35,11 +35,10 @@ public class ClusterControllerContainerCluster extends ContainerCluster<ClusterC @Override public void getConfig(QrStartConfig.Builder builder) { super.getConfig(builder); - int maxHeapSize = featureFlags.clusterControllerMaxHeapSizeInMb(); - boolean verboseGc = (maxHeapSize < 512); + builder.jvm - .verbosegc(verboseGc) - .heapsize(maxHeapSize); + .verbosegc(true) + .heapsize(featureFlags.clusterControllerMaxHeapSizeInMb()); } public ReindexingContext reindexingContext() { return reindexingContext; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java index 690900865ad..45af078f2a3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java @@ -15,7 +15,6 @@ import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.api.container.ContainerServiceType; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterMembership; -import com.yahoo.config.provision.ClusterSpec; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.HostResource; import com.yahoo.vespa.model.PortAllocBridge; @@ -150,9 +149,8 @@ public class MetricsProxyContainer extends Container implements if (clusterMembership.isPresent()) { int maxHeapSize = featureFlags.metricsProxyMaxHeapSizeInMb(clusterMembership.get().cluster().type()); - boolean verboseGc = (maxHeapSize < 512); builder.jvm - .verbosegc(verboseGc) + .verbosegc(true) .heapsize(maxHeapSize); } } @@ -173,4 +171,5 @@ public class MetricsProxyContainer extends Container implements protected String defaultPreload() { return ""; } + } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java index dafce877ddd..f0c62664988 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java @@ -81,7 +81,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat private ContainerModelEvaluation modelEvaluation; - private Optional<String> tlsClientAuthority; + private final Optional<String> tlsClientAuthority; private MbusParams mbusParams; private boolean messageBusEnabled = true; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java index c3591d9ebdd..7c268a92d99 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java @@ -355,7 +355,7 @@ public class ContentCluster extends AbstractConfigProducer implements private ClusterControllerContainerCluster getDedicatedSharedControllers(ModelElement contentElement, Admin admin, ConfigModelContext context, DeployState deployState) { if (admin.getClusterControllers() == null) { - NodesSpecification spec = NodesSpecification.requiredFromSharedParents(deployState.zone().environment().isTest() ? 1 : 3, + NodesSpecification spec = NodesSpecification.requiredFromSharedParents(deployState.zone().environment().isProduction() ? 3 : 1, deployState.featureFlags().dedicatedClusterControllerFlavor().orElse(clusterControllerResources), contentElement, context); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/ClusterControllerTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/ClusterControllerTestCase.java index 0a99797a53f..4ce52aaf4d3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/ClusterControllerTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/ClusterControllerTestCase.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.model.admin; import com.google.common.collect.Collections2; @@ -389,10 +389,10 @@ public class ClusterControllerTestCase extends DomBuilderTest { model.getConfig(qrBuilder, "admin/cluster-controllers/0/components/clustercontroller-bar-configurer"); QrStartConfig qrStartConfig = new QrStartConfig(qrBuilder); assertEquals(32, qrStartConfig.jvm().minHeapsize()); - assertEquals(512, qrStartConfig.jvm().heapsize()); + assertEquals(256, qrStartConfig.jvm().heapsize()); assertEquals(0, qrStartConfig.jvm().heapSizeAsPercentageOfPhysicalMemory()); assertEquals(2, qrStartConfig.jvm().availableProcessors()); - assertFalse(qrStartConfig.jvm().verbosegc()); + assertTrue(qrStartConfig.jvm().verbosegc()); assertEquals("-XX:+UseG1GC -XX:MaxTenuringThreshold=15", qrStartConfig.jvm().gcopts()); assertEquals(512, qrStartConfig.jvm().stacksize()); assertEquals(0, qrStartConfig.jvm().directMemorySizeCache()); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java index 30e3767e3dc..23c37666257 100755 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java @@ -107,7 +107,7 @@ public class ContainerClusterTest { private void verifyHeapSizeAsPercentageOfPhysicalMemory(boolean isHosted, boolean isCombinedCluster, Integer explicitMemoryPercentage, int expectedMemoryPercentage) { - ContainerCluster cluster = createContainerCluster(createRoot(isHosted), isCombinedCluster, explicitMemoryPercentage); + ContainerCluster<?> cluster = createContainerCluster(createRoot(isHosted), isCombinedCluster, explicitMemoryPercentage); QrStartConfig.Builder qsB = new QrStartConfig.Builder(); cluster.getConfig(qsB); QrStartConfig qsC= new QrStartConfig(qsB); @@ -171,7 +171,7 @@ public class ContainerClusterTest { cluster.getConfig(qrBuilder); QrStartConfig qrStartConfig = new QrStartConfig(qrBuilder); assertEquals(32, qrStartConfig.jvm().minHeapsize()); - assertEquals(512, qrStartConfig.jvm().heapsize()); + assertEquals(256, qrStartConfig.jvm().heapsize()); assertEquals(32, qrStartConfig.jvm().compressedClassSpaceSize()); assertEquals(0, qrStartConfig.jvm().heapSizeAsPercentageOfPhysicalMemory()); root.freezeModelTopology(); @@ -398,7 +398,7 @@ public class ContainerClusterTest { return cluster; } - private static ClusterInfoConfig getClusterInfoConfig(ContainerCluster cluster) { + private static ClusterInfoConfig getClusterInfoConfig(ContainerCluster<?> cluster) { ClusterInfoConfig.Builder builder = new ClusterInfoConfig.Builder(); cluster.getConfig(builder); return new ClusterInfoConfig(builder); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java index afaea7c0200..ea76578ef04 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java @@ -1,7 +1,6 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.content; -import com.yahoo.cloud.config.ZookeeperServerConfig; import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; @@ -46,11 +45,8 @@ import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.function.Function; -import java.util.stream.Collectors; import static java.util.stream.Collectors.toList; -import static java.util.stream.Collectors.toSet; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -255,7 +251,7 @@ public class ContentClusterTest extends ContentBaseTest { List<String> sds = ApplicationPackageUtils.generateSchemas("type1", "type2"); VespaModel model = new VespaModelCreatorWithMockPkg(null, xml, sds).create(); assertEquals(2, model.getContentClusters().get("bar").getDocumentDefinitions().size()); - ContainerCluster cluster = model.getAdmin().getClusterControllers(); + ContainerCluster<?> cluster = model.getAdmin().getClusterControllers(); assertEquals(3, cluster.getContainers().size()); } @@ -305,7 +301,7 @@ public class ContentClusterTest extends ContentBaseTest { VespaModel model = createEnd2EndOneNode(new TestProperties()); assertEquals(1, model.getContentClusters().get("storage").getDocumentDefinitions().size()); - ContainerCluster cluster = model.getAdmin().getClusterControllers(); + ContainerCluster<?> cluster = model.getAdmin().getClusterControllers(); assertEquals(1, cluster.getContainers().size()); } @@ -399,8 +395,7 @@ public class ContentClusterTest extends ContentBaseTest { "</content>" ); fail("no exception thrown"); - } catch (Exception e) { - } + } catch (Exception e) { /* ignore */ } } @Test @@ -420,8 +415,7 @@ public class ContentClusterTest extends ContentBaseTest { "</content>" ); fail("no exception thrown"); - } catch (Exception e) { - } + } catch (Exception e) { /* ignore */ } } FleetcontrollerConfig getFleetControllerConfig(String xml) { @@ -1027,11 +1021,8 @@ public class ContentClusterTest extends ContentBaseTest { assertEquals(0.1, resolveMaxDeadBytesRatio(0.1), 1e-5); } - void assertZookeeperServerImplementation(boolean reconfigurable, String expectedClassName) { - VespaModel model = createEnd2EndOneNode( - new TestProperties() - .reconfigurableZookeeperServer(reconfigurable) - .setMultitenant(true)); + void assertZookeeperServerImplementation(String expectedClassName) { + VespaModel model = createEnd2EndOneNode(new TestProperties().setMultitenant(true)); ContentCluster cc = model.getContentClusters().get("storage"); for (ClusterControllerContainer c : cc.getClusterControllers().getContainers()) { @@ -1040,19 +1031,14 @@ public class ContentClusterTest extends ContentBaseTest { assertEquals(1, new ComponentsConfig(builder).components().stream() .filter(component -> component.classId().equals(expectedClassName)) .count()); - - var zBuilder = new ZookeeperServerConfig.Builder(); - c.getConfig(zBuilder); - assertEquals(reconfigurable, new ZookeeperServerConfig(zBuilder).dynamicReconfiguration()); } } @Test - public void reconfigurableZookeeperServerForClusterController() { - assertZookeeperServerImplementation(false, "com.yahoo.vespa.zookeeper.VespaZooKeeperServerImpl"); - assertZookeeperServerImplementation(true, "com.yahoo.vespa.zookeeper.ReconfigurableVespaZooKeeperServer"); - assertZookeeperServerImplementation(true, "com.yahoo.vespa.zookeeper.Reconfigurer"); - assertZookeeperServerImplementation(true, "com.yahoo.vespa.zookeeper.VespaZooKeeperAdminImpl"); + public void reconfigurableZookeeperServerComponentsForClusterController() { + assertZookeeperServerImplementation("com.yahoo.vespa.zookeeper.ReconfigurableVespaZooKeeperServer"); + assertZookeeperServerImplementation("com.yahoo.vespa.zookeeper.Reconfigurer"); + assertZookeeperServerImplementation("com.yahoo.vespa.zookeeper.VespaZooKeeperAdminImpl"); } private int resolveMaxInhibitedGroupsConfigWithFeatureFlag(int maxGroups) { diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java index e356ee06ac6..56882f8676f 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java @@ -49,6 +49,11 @@ public enum NodeType { return !childNodeTypes.isEmpty(); } + /** Returns whether this supports host sharing */ + public boolean isSharable() { + return this == NodeType.host; + } + public String description() { return description; } @@ -75,4 +80,16 @@ public enum NodeType { public boolean canRun(NodeType type) { return childNodeTypes.contains(type); } + + /** Returns the host type of this */ + public NodeType hostType() { + if (isHost()) return this; + for (NodeType nodeType : values()) { + if (nodeType.childNodeTypes.size() == 1 && nodeType.canRun(this)) { + return nodeType; + } + } + throw new IllegalArgumentException("No host of " + this + " exists"); + } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index 2568ff23f9e..af2f91b62d9 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -21,7 +21,6 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.DockerImage; -import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.TenantName; @@ -29,7 +28,6 @@ import com.yahoo.config.provision.Zone; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.IntFlag; import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.flags.UnboundFlag; @@ -167,7 +165,6 @@ public class ModelContextImpl implements ModelContext { private final boolean useAccessControlTlsHandshakeClientAuth; private final boolean useAsyncMessageHandlingOnSchedule; private final double feedConcurrency; - private final boolean reconfigurableZookeeperServer; private final boolean useBucketExecutorForLidSpaceCompact; private final boolean useBucketExecutorForBucketMove; private final boolean enableFeedBlockInDistributor; @@ -192,7 +189,6 @@ public class ModelContextImpl implements ModelContext { this.useAccessControlTlsHandshakeClientAuth = flagValue(source, appId, Flags.USE_ACCESS_CONTROL_CLIENT_AUTHENTICATION); this.useAsyncMessageHandlingOnSchedule = flagValue(source, appId, Flags.USE_ASYNC_MESSAGE_HANDLING_ON_SCHEDULE); this.feedConcurrency = flagValue(source, appId, Flags.FEED_CONCURRENCY); - this.reconfigurableZookeeperServer = flagValue(source, appId, Flags.RECONFIGURABLE_ZOOKEEPER_SERVER_FOR_CLUSTER_CONTROLLER); this.useBucketExecutorForLidSpaceCompact = flagValue(source, appId, Flags.USE_BUCKET_EXECUTOR_FOR_LID_SPACE_COMPACT); this.useBucketExecutorForBucketMove = flagValue(source, appId, Flags.USE_BUCKET_EXECUTOR_FOR_BUCKET_MOVE); this.enableFeedBlockInDistributor = flagValue(source, appId, Flags.ENABLE_FEED_BLOCK_IN_DISTRIBUTOR); @@ -217,7 +213,6 @@ public class ModelContextImpl implements ModelContext { @Override public boolean useAccessControlTlsHandshakeClientAuth() { return useAccessControlTlsHandshakeClientAuth; } @Override public boolean useAsyncMessageHandlingOnSchedule() { return useAsyncMessageHandlingOnSchedule; } @Override public double feedConcurrency() { return feedConcurrency; } - @Override public boolean reconfigurableZookeeperServer() { return reconfigurableZookeeperServer; } @Override public boolean useBucketExecutorForLidSpaceCompact() { return useBucketExecutorForLidSpaceCompact; } @Override public boolean useBucketExecutorForBucketMove() { return useBucketExecutorForBucketMove; } @Override public boolean enableFeedBlockInDistributor() { return enableFeedBlockInDistributor; } @@ -292,7 +287,7 @@ public class ModelContextImpl implements ModelContext { this.athenzDomain = athenzDomain; this.applicationRoles = applicationRoles; this.quota = maybeQuota.orElseGet(Quota::unlimited); - this.dedicatedClusterControllerCluster = zoneHasRedundancyOrIsCD(zone) && dedicatedClusterControllerCluster; + this.dedicatedClusterControllerCluster = dedicatedClusterControllerCluster; jvmGcOptions = flagValue(flagSource, applicationId, PermanentFlags.JVM_GC_OPTIONS); } @@ -360,11 +355,6 @@ public class ModelContextImpl implements ModelContext { } } - private static boolean zoneHasRedundancyOrIsCD(Zone zone) { - return zone.system().isCd() && zone.environment() == Environment.dev - || List.of(Environment.staging, Environment.perf, Environment.prod).contains(zone.environment()); - } - private static NodeResources parseDedicatedClusterControllerFlavor(String flagValue) { String[] parts = flagValue.split("-"); if (parts.length != 3) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java index ecf3d29bc1a..ad739d16ff8 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java @@ -102,6 +102,11 @@ enum Policy { .on(PathGroup.tenantKeys, PathGroup.applicationKeys) .in(SystemName.all())), + /** Access to revoke keys from the tenant */ + keyRevokal(Privilege.grant(Action.delete) + .on(PathGroup.tenantKeys, PathGroup.applicationKeys) + .in(SystemName.all())), + /** Full access to application development deployments. */ developmentDeployment(Privilege.grant(Action.all()) .on(PathGroup.developmentDeployment, PathGroup.developmentRestart) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java index 3b861c607b1..40903b02465 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java @@ -63,6 +63,7 @@ public enum RoleDefinition { Policy.tenantManager, Policy.tenantDelete, Policy.applicationManager, + Policy.keyRevokal, Policy.paymentInstrumentRead, Policy.paymentInstrumentUpdate, Policy.paymentInstrumentDelete, diff --git a/eval/src/tests/eval/fast_value/fast_value_test.cpp b/eval/src/tests/eval/fast_value/fast_value_test.cpp index bb268a386ac..70c534b2010 100644 --- a/eval/src/tests/eval/fast_value/fast_value_test.cpp +++ b/eval/src/tests/eval/fast_value/fast_value_test.cpp @@ -159,9 +159,8 @@ const std::vector<GenSpec> layouts = { TEST(FastValueBuilderFactoryTest, fast_values_can_be_copied) { auto factory = FastValueBuilderFactory::get(); for (const auto &layout: layouts) { - for (TensorSpec expect : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec expect = layout.cpy().cells(ct); std::unique_ptr<Value> value = value_from_spec(expect, factory); std::unique_ptr<Value> copy = factory.copy(*value); TensorSpec actual = spec_from_value(*copy); diff --git a/eval/src/tests/eval/simple_value/simple_value_test.cpp b/eval/src/tests/eval/simple_value/simple_value_test.cpp index 1f0df71d306..af3b6e9bc43 100644 --- a/eval/src/tests/eval/simple_value/simple_value_test.cpp +++ b/eval/src/tests/eval/simple_value/simple_value_test.cpp @@ -67,9 +67,8 @@ TensorSpec simple_value_join(const TensorSpec &a, const TensorSpec &b, join_fun_ TEST(SimpleValueTest, simple_values_can_be_converted_from_and_to_tensor_spec) { for (const auto &layout: layouts) { - for (TensorSpec expect : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec expect = layout.cpy().cells(ct); std::unique_ptr<Value> value = value_from_spec(expect, SimpleValueBuilderFactory::get()); TensorSpec actual = spec_from_value(*value); EXPECT_EQ(actual, expect); @@ -79,9 +78,8 @@ TEST(SimpleValueTest, simple_values_can_be_converted_from_and_to_tensor_spec) { TEST(SimpleValueTest, simple_values_can_be_copied) { for (const auto &layout: layouts) { - for (TensorSpec expect : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec expect = layout.cpy().cells(ct); std::unique_ptr<Value> value = value_from_spec(expect, SimpleValueBuilderFactory::get()); std::unique_ptr<Value> copy = SimpleValueBuilderFactory::get().copy(*value); TensorSpec actual = spec_from_value(*copy); @@ -131,12 +129,10 @@ TEST(SimpleValueTest, new_generic_join_works_for_simple_values) { for (size_t i = 0; i < join_layouts.size(); i += 2) { const auto l = join_layouts[i].cpy().seq(N_16ths); const auto r = join_layouts[i + 1].cpy().seq(N_16ths); - for (TensorSpec lhs : { l.cpy().cells_float(), - l.cpy().cells_double() }) - { - for (TensorSpec rhs : { r.cpy().cells_float(), - r.cpy().cells_double() }) - { + for (CellType lct : CellTypeUtils::list_types()) { + TensorSpec lhs = l.cpy().cells(lct); + for (CellType rct : CellTypeUtils::list_types()) { + TensorSpec rhs = r.cpy().cells(rct); for (auto fun: {operation::Add::f, operation::Sub::f, operation::Mul::f, operation::Div::f}) { SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); auto expect = ReferenceOperations::join(lhs, rhs, fun); diff --git a/eval/src/tests/eval/value_codec/value_codec_test.cpp b/eval/src/tests/eval/value_codec/value_codec_test.cpp index 30de76abb7d..434ad0b2a53 100644 --- a/eval/src/tests/eval/value_codec/value_codec_test.cpp +++ b/eval/src/tests/eval/value_codec/value_codec_test.cpp @@ -32,9 +32,8 @@ const std::vector<GenSpec> layouts = { TEST(ValueCodecTest, simple_values_can_be_converted_from_and_to_tensor_spec) { for (const auto &layout: layouts) { - for (TensorSpec expect : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec expect = layout.cpy().cells(ct); std::unique_ptr<Value> value = value_from_spec(expect, factory); TensorSpec actual = spec_from_value(*value); EXPECT_EQ(actual, expect); diff --git a/eval/src/tests/instruction/generic_cell_cast/generic_cell_cast_test.cpp b/eval/src/tests/instruction/generic_cell_cast/generic_cell_cast_test.cpp index 6a0b63bd562..06b74eff4d5 100644 --- a/eval/src/tests/instruction/generic_cell_cast/generic_cell_cast_test.cpp +++ b/eval/src/tests/instruction/generic_cell_cast/generic_cell_cast_test.cpp @@ -44,13 +44,12 @@ TensorSpec perform_generic_cell_cast(const TensorSpec &a, CellType to, const Val void test_generic_cell_cast_with(const ValueBuilderFactory &factory) { for (const auto &layout : layouts) { - for (TensorSpec lhs : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { - for (CellType ct : { CellType::FLOAT, CellType::DOUBLE }) { + for (CellType in_type: CellTypeUtils::list_types()) { + for (CellType out_type: CellTypeUtils::list_types()) { + TensorSpec lhs = layout.cpy().cells(in_type); SCOPED_TRACE(fmt("\n===\nLHS: %s\n===\n", lhs.to_string().c_str())); - auto expect = ReferenceOperations::cell_cast(lhs, ct); - auto actual = perform_generic_cell_cast(lhs, ct, factory); + auto expect = ReferenceOperations::cell_cast(lhs, out_type); + auto actual = perform_generic_cell_cast(lhs, out_type, factory); EXPECT_EQ(actual, expect); } } diff --git a/eval/src/tests/instruction/generic_concat/generic_concat_test.cpp b/eval/src/tests/instruction/generic_concat/generic_concat_test.cpp index 09ac2e413c1..8bf6751cd73 100644 --- a/eval/src/tests/instruction/generic_concat/generic_concat_test.cpp +++ b/eval/src/tests/instruction/generic_concat/generic_concat_test.cpp @@ -78,12 +78,10 @@ void test_generic_concat_with(const ValueBuilderFactory &factory) { for (size_t i = 0; i < concat_layouts.size(); i += 2) { const auto l = concat_layouts[i]; const auto r = concat_layouts[i+1].cpy().seq(N_16ths); - for (TensorSpec lhs : { l.cpy().cells_float(), - l.cpy().cells_double() }) - { - for (TensorSpec rhs : { r.cpy().cells_float(), - r.cpy().cells_double() }) - { + for (CellType lct : CellTypeUtils::list_types()) { + TensorSpec lhs = l.cpy().cells(lct); + for (CellType rct : CellTypeUtils::list_types()) { + TensorSpec rhs = r.cpy().cells(rct); SCOPED_TRACE(fmt("\n===\nin LHS: %s\nin RHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); auto actual = perform_generic_concat(lhs, rhs, "y", factory); auto expect = ReferenceOperations::concat(lhs, rhs, "y"); diff --git a/eval/src/tests/instruction/generic_create/generic_create_test.cpp b/eval/src/tests/instruction/generic_create/generic_create_test.cpp index 3677159b64e..c387dad780e 100644 --- a/eval/src/tests/instruction/generic_create/generic_create_test.cpp +++ b/eval/src/tests/instruction/generic_create/generic_create_test.cpp @@ -91,9 +91,8 @@ TensorSpec perform_generic_create(const TensorSpec &a, const ValueBuilderFactory void test_generic_create_with(const ValueBuilderFactory &factory) { for (const auto &layout : create_layouts) { - for (TensorSpec full : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec full = layout.cpy().cells(ct); auto actual = perform_generic_create(full, factory); auto expect = reference_create(full).normalize(); EXPECT_EQ(actual, expect); diff --git a/eval/src/tests/instruction/generic_join/generic_join_test.cpp b/eval/src/tests/instruction/generic_join/generic_join_test.cpp index 181f44d0f2e..2f619bcaa54 100644 --- a/eval/src/tests/instruction/generic_join/generic_join_test.cpp +++ b/eval/src/tests/instruction/generic_join/generic_join_test.cpp @@ -107,12 +107,10 @@ TEST(GenericJoinTest, generic_join_works_for_simple_and_fast_values) { for (size_t i = 0; i < join_layouts.size(); i += 2) { const auto &l = join_layouts[i]; const auto &r = join_layouts[i+1]; - for (TensorSpec lhs : { l.cpy().cells_float(), - l.cpy().cells_double() }) - { - for (TensorSpec rhs : { r.cpy().cells_float(), - r.cpy().cells_double() }) - { + for (CellType lct : CellTypeUtils::list_types()) { + TensorSpec lhs = l.cpy().cells(lct); + for (CellType rct : CellTypeUtils::list_types()) { + TensorSpec rhs = r.cpy().cells(rct); for (auto fun: {operation::Add::f, operation::Sub::f, operation::Mul::f, operation::Div::f}) { SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); auto expect = ReferenceOperations::join(lhs, rhs, fun); diff --git a/eval/src/tests/instruction/generic_map/generic_map_test.cpp b/eval/src/tests/instruction/generic_map/generic_map_test.cpp index 9f30efe135e..cf4b09914c3 100644 --- a/eval/src/tests/instruction/generic_map/generic_map_test.cpp +++ b/eval/src/tests/instruction/generic_map/generic_map_test.cpp @@ -43,9 +43,8 @@ TensorSpec perform_generic_map(const TensorSpec &a, map_fun_t func, const ValueB void test_generic_map_with(const ValueBuilderFactory &factory) { for (const auto &layout : map_layouts) { - for (TensorSpec lhs : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec lhs = layout.cpy().cells(ct); for (auto func : {operation::Floor::f, operation::Fabs::f, operation::Square::f, operation::Inv::f}) { SCOPED_TRACE(fmt("\n===\nLHS: %s\n===\n", lhs.to_string().c_str())); auto expect = ReferenceOperations::map(lhs, func); diff --git a/eval/src/tests/instruction/generic_merge/generic_merge_test.cpp b/eval/src/tests/instruction/generic_merge/generic_merge_test.cpp index bb14d869440..025ab7f857e 100644 --- a/eval/src/tests/instruction/generic_merge/generic_merge_test.cpp +++ b/eval/src/tests/instruction/generic_merge/generic_merge_test.cpp @@ -50,12 +50,10 @@ void test_generic_merge_with(const ValueBuilderFactory &factory) { for (size_t i = 0; i < merge_layouts.size(); i += 2) { const auto l = merge_layouts[i]; const auto r = merge_layouts[i+1].cpy().seq(N_16ths); - for (TensorSpec lhs : { l.cpy().cells_float(), - l.cpy().cells_double() }) - { - for (TensorSpec rhs : { r.cpy().cells_float(), - r.cpy().cells_double() }) - { + for (CellType lct : CellTypeUtils::list_types()) { + TensorSpec lhs = l.cpy().cells(lct); + for (CellType rct : CellTypeUtils::list_types()) { + TensorSpec rhs = r.cpy().cells(rct); SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); for (auto fun: {operation::Add::f, operation::Mul::f, operation::Sub::f, operation::Max::f}) { auto expect = ReferenceOperations::merge(lhs, rhs, fun); diff --git a/eval/src/tests/instruction/generic_peek/generic_peek_test.cpp b/eval/src/tests/instruction/generic_peek/generic_peek_test.cpp index cafaf5837ec..972e763f018 100644 --- a/eval/src/tests/instruction/generic_peek/generic_peek_test.cpp +++ b/eval/src/tests/instruction/generic_peek/generic_peek_test.cpp @@ -194,9 +194,8 @@ void fill_dims_and_check(const TensorSpec &input, void test_generic_peek_with(const ValueBuilderFactory &factory) { for (const auto &layout : peek_layouts) { - for (TensorSpec input : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec input = layout.cpy().cells(ct); ValueType input_type = ValueType::from_spec(input.type()); const auto &dims = input_type.dimensions(); PeekSpec spec; diff --git a/eval/src/tests/instruction/generic_reduce/generic_reduce_test.cpp b/eval/src/tests/instruction/generic_reduce/generic_reduce_test.cpp index 95236d15299..8eb7b5bd057 100644 --- a/eval/src/tests/instruction/generic_reduce/generic_reduce_test.cpp +++ b/eval/src/tests/instruction/generic_reduce/generic_reduce_test.cpp @@ -70,9 +70,8 @@ TEST(GenericReduceTest, sparse_reduce_plan_can_be_created) { void test_generic_reduce_with(const ValueBuilderFactory &factory) { for (const auto &layout: layouts) { - for (TensorSpec input : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec input = layout.cpy().cells(ct); SCOPED_TRACE(fmt("tensor type: %s, num_cells: %zu", input.type().c_str(), input.cells().size())); for (Aggr aggr: {Aggr::SUM, Aggr::AVG, Aggr::MIN, Aggr::MAX}) { SCOPED_TRACE(fmt("aggregator: %s", AggrNames::name_of(aggr)->c_str())); diff --git a/eval/src/tests/instruction/generic_rename/generic_rename_test.cpp b/eval/src/tests/instruction/generic_rename/generic_rename_test.cpp index 9c17dce972f..7b67f1bbd22 100644 --- a/eval/src/tests/instruction/generic_rename/generic_rename_test.cpp +++ b/eval/src/tests/instruction/generic_rename/generic_rename_test.cpp @@ -110,9 +110,8 @@ TensorSpec perform_generic_rename(const TensorSpec &a, void test_generic_rename_with(const ValueBuilderFactory &factory) { for (const auto &layout : rename_layouts) { - for (TensorSpec lhs : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec lhs = layout.cpy().cells(ct); ValueType lhs_type = ValueType::from_spec(lhs.type()); for (const auto & from_to : rename_from_to) { ValueType renamed_type = lhs_type.rename(from_to.from, from_to.to); diff --git a/eval/src/tests/streamed/value/streamed_value_test.cpp b/eval/src/tests/streamed/value/streamed_value_test.cpp index 44c30d226bd..e5f4088d908 100644 --- a/eval/src/tests/streamed/value/streamed_value_test.cpp +++ b/eval/src/tests/streamed/value/streamed_value_test.cpp @@ -67,9 +67,8 @@ TensorSpec streamed_value_join(const TensorSpec &a, const TensorSpec &b, join_fu TEST(StreamedValueTest, streamed_values_can_be_converted_from_and_to_tensor_spec) { for (const auto &layout: layouts) { - for (TensorSpec expect : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec expect = layout.cpy().cells(ct); std::unique_ptr<Value> value = value_from_spec(expect, StreamedValueBuilderFactory::get()); TensorSpec actual = spec_from_value(*value); EXPECT_EQ(actual, expect); @@ -79,9 +78,8 @@ TEST(StreamedValueTest, streamed_values_can_be_converted_from_and_to_tensor_spec TEST(StreamedValueTest, streamed_values_can_be_copied) { for (const auto &layout: layouts) { - for (TensorSpec expect : { layout.cpy().cells_float(), - layout.cpy().cells_double() }) - { + for (CellType ct : CellTypeUtils::list_types()) { + TensorSpec expect = layout.cpy().cells(ct); std::unique_ptr<Value> value = value_from_spec(expect, StreamedValueBuilderFactory::get()); std::unique_ptr<Value> copy = StreamedValueBuilderFactory::get().copy(*value); TensorSpec actual = spec_from_value(*copy); @@ -131,12 +129,10 @@ TEST(StreamedValueTest, new_generic_join_works_for_streamed_values) { for (size_t i = 0; i < join_layouts.size(); i += 2) { const auto l = join_layouts[i].cpy().seq(N_16ths); const auto r = join_layouts[i + 1].cpy().seq(N_16ths); - for (TensorSpec lhs : { l.cpy().cells_float(), - l.cpy().cells_double() }) - { - for (TensorSpec rhs : { r.cpy().cells_float(), - r.cpy().cells_double() }) - { + for (CellType lct : CellTypeUtils::list_types()) { + TensorSpec lhs = l.cpy().cells(lct); + for (CellType rct : CellTypeUtils::list_types()) { + TensorSpec rhs = r.cpy().cells(rct); for (auto fun: {operation::Add::f, operation::Sub::f, operation::Mul::f, operation::Max::f}) { SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); auto expect = ReferenceOperations::join(lhs, rhs, fun); diff --git a/eval/src/vespa/eval/eval/value_type_spec.cpp b/eval/src/vespa/eval/eval/value_type_spec.cpp index 1eabbd0a9fc..470da4f63a3 100644 --- a/eval/src/vespa/eval/eval/value_type_spec.cpp +++ b/eval/src/vespa/eval/eval/value_type_spec.cpp @@ -8,16 +8,6 @@ namespace vespalib::eval::value_type { -std::optional<CellType> cell_type_from_name(const vespalib::string &name) { - if (name == "double") { - return CellType::DOUBLE; - } else if (name == "float") { - return CellType::FLOAT; - } else { - return std::nullopt; - } -} - vespalib::string cell_type_to_name(CellType cell_type) { switch (cell_type) { case CellType::DOUBLE: return "double"; @@ -26,6 +16,15 @@ vespalib::string cell_type_to_name(CellType cell_type) { abort(); } +std::optional<CellType> cell_type_from_name(const vespalib::string &name) { + for (CellType t : CellTypeUtils::list_types()) { + if (name == cell_type_to_name(t)) { + return t; + } + } + return std::nullopt; +} + namespace { class ParseContext @@ -171,12 +170,13 @@ CellType parse_cell_type(ParseContext &ctx) { ctx.eat('>'); if (ctx.failed()) { ctx.revert(mark); - return CellType::DOUBLE; - } - if (cell_type == "float") { - return CellType::FLOAT; - } else if (cell_type != "double") { - ctx.fail(); + } else { + auto result = cell_type_from_name(cell_type); + if (result.has_value()) { + return result.value(); + } else { + ctx.fail(); + } } return CellType::DOUBLE; } 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 c9a4006667f..dd3762e7234 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -206,13 +206,6 @@ public class Flags { "Takes effect on the next suspension request to the Orchestrator.", APPLICATION_ID); - public static final UnboundBooleanFlag RECONFIGURABLE_ZOOKEEPER_SERVER_FOR_CLUSTER_CONTROLLER = defineFeatureFlag( - "reconfigurable-zookeeper-server-for-cluster-controller", true, - List.of("musum", "mpolden"), "2020-12-16", "2021-03-16", - "Whether to use reconfigurable zookeeper server for cluster controller", - "Takes effect on (re)redeployment", - APPLICATION_ID); - public static final UnboundBooleanFlag ENABLE_FEED_BLOCK_IN_DISTRIBUTOR = defineFeatureFlag( "enable-feed-block-in-distributor", false, List.of("geirst"), "2021-01-27", "2021-04-01", @@ -234,7 +227,7 @@ public class Flags { TENANT_ID, ZONE_ID); public static final UnboundIntFlag CLUSTER_CONTROLLER_MAX_HEAP_SIZE_IN_MB = defineIntFlag( - "cluster-controller-max-heap-size-in-mb", 512, + "cluster-controller-max-heap-size-in-mb", 256, List.of("hmusum"), "2021-02-10", "2021-04-10", "JVM max heap size for cluster controller in Mb", "Takes effect when restarting cluster controller"); @@ -279,6 +272,12 @@ public class Flags { "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); + public static final UnboundBooleanFlag DYNAMIC_CONFIG_SERVER_PROVISIONING = defineFeatureFlag( + "dynamic-config-server-provisioning", false, + List.of("mpolden", "hakon"), "2021-03-03", "2021-05-01", + "Enable dynamic provisioning of config servers", + "Takes effect immediately, for subsequent provisioning"); + /** 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-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java index c8763f7154e..2044844e9ef 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java @@ -103,10 +103,10 @@ public final class Node implements Nodelike { } if (type != NodeType.host && reservedTo.isPresent()) - throw new IllegalArgumentException("Only hosts can be reserved to a tenant"); + throw new IllegalArgumentException("Only tenant hosts can be reserved to a tenant"); if (type != NodeType.host && exclusiveTo.isPresent()) - throw new IllegalArgumentException("Only hosts can be exclusive to an application"); + throw new IllegalArgumentException("Only tenant hosts can be exclusive to an application"); } /** Returns the IP config of this node */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java index 204d4eea1c4..2979940ee22 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java @@ -90,13 +90,13 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer { /** Resume provisioning of already provisioned hosts and their children */ private void resumeProvisioning(NodeList nodes, Mutex lock) { - Map<String, Set<Node>> nodesByProvisionedParentHostname = nodes.nodeType(NodeType.tenant).asList().stream() + Map<String, Set<Node>> nodesByProvisionedParentHostname = nodes.nodeType(NodeType.tenant, NodeType.config).asList().stream() .filter(node -> node.parentHostname().isPresent()) .collect(Collectors.groupingBy( node -> node.parentHostname().get(), Collectors.toSet())); - nodes.state(Node.State.provisioned).hosts().forEach(host -> { + nodes.state(Node.State.provisioned).nodeType(NodeType.host, NodeType.confighost).forEach(host -> { Set<Node> children = nodesByProvisionedParentHostname.getOrDefault(host.hostname(), Set.of()); try { List<Node> updatedNodes = hostProvisioner.provision(host, children); @@ -197,10 +197,10 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer { .collect(Collectors.toMap(Node::hostname, Function.identity()))); nodes.stream() - .filter(node -> node.allocation().isPresent()) - .flatMap(node -> node.parentHostname().stream()) - .distinct() - .forEach(hostsByHostname::remove); + .filter(node -> node.allocation().isPresent()) + .flatMap(node -> node.parentHostname().stream()) + .distinct() + .forEach(hostsByHostname::remove); return List.copyOf(hostsByHostname.values()); } @@ -246,8 +246,8 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer { private List<Node> provisionHosts(int count, NodeResources nodeResources) { try { Version osVersion = nodeRepository().osVersions().targetFor(NodeType.host).orElse(Version.emptyVersion); - List<Integer> provisionIndexes = nodeRepository().database().getProvisionIndexes(count); - List<Node> hosts = hostProvisioner.provisionHosts(provisionIndexes, nodeResources, + List<Integer> provisionIndices = nodeRepository().database().readProvisionIndices(count); + List<Node> hosts = hostProvisioner.provisionHosts(provisionIndices, NodeType.host, nodeResources, ApplicationId.defaultId(), osVersion, HostSharing.shared) .stream() .map(ProvisionedHost::generateHost) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java index 5b9cd6a69e1..b720bf004ff 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Expirer.java @@ -55,6 +55,10 @@ public abstract class Expirer extends NodeRepositoryMaintainer { } protected boolean isExpired(Node node) { + return isExpired(node, expiryTime); + } + + protected final boolean isExpired(Node node, Duration expiryTime) { return node.history().hasEventBefore(eventType, clock().instant().minus(expiryTime)); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveExpirer.java index ae6e716bffe..238f89fc448 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveExpirer.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.maintenance; +import com.yahoo.config.provision.NodeType; import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -10,6 +11,7 @@ import com.yahoo.vespa.hosted.provision.node.Status; import java.time.Duration; import java.util.List; +import java.util.Map; /** * Maintenance job which moves inactive nodes to dirty or parked after timeout. @@ -30,10 +32,15 @@ import java.util.List; public class InactiveExpirer extends Expirer { private final NodeRepository nodeRepository; + private final Duration defaultTimeout; + private final Map<NodeType, Duration> inactiveTimeouts; - InactiveExpirer(NodeRepository nodeRepository, Duration inactiveTimeout, Metric metric) { - super(Node.State.inactive, History.Event.Type.deactivated, nodeRepository, inactiveTimeout, metric); + InactiveExpirer(NodeRepository nodeRepository, Duration defaultTimeout, Map<NodeType, Duration> inactiveTimeouts, + Metric metric) { + super(Node.State.inactive, History.Event.Type.deactivated, nodeRepository, defaultTimeout, metric); this.nodeRepository = nodeRepository; + this.defaultTimeout = defaultTimeout; + this.inactiveTimeouts = Map.copyOf(inactiveTimeouts); } @Override @@ -45,8 +52,12 @@ public class InactiveExpirer extends Expirer { @Override protected boolean isExpired(Node node) { - return super.isExpired(node) - || node.allocation().get().owner().instance().isTester(); + return super.isExpired(node, timeout(node)) || + node.allocation().get().owner().instance().isTester(); + } + + private Duration timeout(Node node) { + return inactiveTimeouts.getOrDefault(node.type(), defaultTimeout); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index 0154b030baa..edbdd982a60 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -8,19 +8,20 @@ import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostLivenessTracker; import com.yahoo.config.provision.InfraDeployer; +import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.Zone; import com.yahoo.jdisc.Metric; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.hosted.provision.NodeRepository; -import com.yahoo.vespa.hosted.provision.autoscale.MetricsFetcher; import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; +import com.yahoo.vespa.hosted.provision.autoscale.MetricsFetcher; import com.yahoo.vespa.hosted.provision.provisioning.ProvisionServiceProvider; import com.yahoo.vespa.orchestrator.Orchestrator; import com.yahoo.vespa.service.monitor.ServiceMonitor; import java.time.Duration; import java.util.List; -import java.util.Set; +import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; /** @@ -52,7 +53,9 @@ public class NodeRepositoryMaintenance extends AbstractComponent { maintainers.add(new OperatorChangeApplicationMaintainer(deployer, metric, nodeRepository, defaults.operatorChangeRedeployInterval)); maintainers.add(new ReservationExpirer(nodeRepository, defaults.reservationExpiry, metric)); maintainers.add(new RetiredExpirer(nodeRepository, orchestrator, deployer, metric, defaults.retiredInterval, defaults.retiredExpiry)); - maintainers.add(new InactiveExpirer(nodeRepository, defaults.inactiveExpiry, metric)); + maintainers.add(new InactiveExpirer(nodeRepository, defaults.inactiveExpiry, Map.of(NodeType.config, defaults.inactiveConfigServerExpiry, + NodeType.controller, defaults.inactiveControllerExpiry), + metric)); maintainers.add(new FailedExpirer(nodeRepository, zone, defaults.failedExpirerInterval, metric)); maintainers.add(new DirtyExpirer(nodeRepository, defaults.dirtyExpiry, metric)); maintainers.add(new ProvisionedExpirer(nodeRepository, defaults.provisionedExpiry, metric)); @@ -65,9 +68,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { maintainers.add(new AutoscalingMaintainer(nodeRepository, metricsDb, deployer, metric, defaults.autoscalingInterval)); maintainers.add(new ScalingSuggestionsMaintainer(nodeRepository, metricsDb, defaults.scalingSuggestionsInterval, metric)); maintainers.add(new SwitchRebalancer(nodeRepository, defaults.switchRebalancerInterval, metric, deployer)); - if (Set.of(Environment.staging, Environment.perf, Environment.prod).contains(zone.environment()) - || (zone.system().isCd() && zone.environment() == Environment.dev)) // TODO: Temporarily when testing the feature - maintainers.add(new DedicatedClusterControllerClusterMigrator(deployer, metric, nodeRepository, defaults.dedicatedClusterControllerMigratorInterval, flagSource, orchestrator)); + maintainers.add(new DedicatedClusterControllerClusterMigrator(deployer, metric, nodeRepository, defaults.dedicatedClusterControllerMigratorInterval, flagSource, orchestrator)); provisionServiceProvider.getLoadBalancerService(nodeRepository) .map(lbService -> new LoadBalancerExpirer(nodeRepository, defaults.loadBalancerExpirerInterval, lbService, metric)) @@ -99,6 +100,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final Duration reservationExpiry; private final Duration inactiveExpiry; + private final Duration inactiveConfigServerExpiry; + private final Duration inactiveControllerExpiry; private final Duration retiredExpiry; private final Duration failedExpirerInterval; private final Duration dirtyExpiry; @@ -148,6 +151,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent { retiredExpiry = Duration.ofDays(4); // give up migrating data after 4 days dedicatedClusterControllerMigratorInterval = zone.environment() == Environment.staging || zone.system().isCd() ? Duration.ofMinutes(3) : Duration.ofHours(2); + inactiveConfigServerExpiry = Duration.ofMinutes(5); + inactiveControllerExpiry = Duration.ofMinutes(5); if (zone.environment() == Environment.prod && ! zone.system().isCd()) { inactiveExpiry = Duration.ofHours(4); // enough time for the application owner to discover and redeploy diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index d637236e1b8..534115342f3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -560,6 +560,22 @@ public class Nodes { return performOn(filter, (node, lock) -> write(node.withWantToRetire(true, agent, instant), lock)); } + /** Retire and deprovision given host and all of its children */ + public List<Node> deprovision(Node host, Agent agent, Instant instant) { + if (!host.type().isHost()) throw new IllegalArgumentException("Cannot deprovision non-host " + host); + Optional<NodeMutex> nodeMutex = lockAndGet(host); + if (nodeMutex.isEmpty()) return List.of(); + List<Node> result; + try (NodeMutex lock = nodeMutex.get(); Mutex allocationLock = lockUnallocated()) { + // This takes allocationLock to prevent any further allocation of nodes on this host + host = lock.node(); + NodeList children = list(allocationLock).childrenOf(host); + result = retire(NodeListFilter.from(children.asList()), agent, instant); + result.add(write(host.withWantToRetire(true, true, agent, instant), lock)); + } + return result; + } + /** * Writes this node after it has changed some internal state but NOT changed its state field. * This does NOT lock the node repository implicitly, but callers are expected to already hold the lock. diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringUpgrader.java index 8118556f4c1..72967cca98a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringUpgrader.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringUpgrader.java @@ -3,10 +3,8 @@ package com.yahoo.vespa.hosted.provision.os; import com.yahoo.component.Version; import com.yahoo.config.provision.NodeType; -import com.yahoo.transaction.Mutex; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; -import com.yahoo.vespa.hosted.provision.NodeMutex; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.filter.NodeListFilter; @@ -52,7 +50,7 @@ public class RetiringUpgrader implements Upgrader { .not().deprovisioning() .byIncreasingOsVersion() .first(1) - .forEach(node -> deprovision(node, target.version(), now, allNodes)); + .forEach(node -> upgrade(node, target.version(), now)); } @Override @@ -60,26 +58,15 @@ public class RetiringUpgrader implements Upgrader { // No action needed in this implementation. } - /** Retire and deprovision given host and its children */ - private void deprovision(Node host, Version target, Instant now, NodeList allNodes) { - if (!host.type().isHost()) throw new IllegalArgumentException("Cannot retire non-host " + host); - Optional<NodeMutex> nodeMutex = nodeRepository.nodes().lockAndGet(host); - if (nodeMutex.isEmpty()) return; - // Take allocationLock to prevent any further allocation of nodes on this host - try (NodeMutex lock = nodeMutex.get(); Mutex allocationLock = nodeRepository.nodes().lockUnallocated()) { - host = lock.node(); - NodeType nodeType = host.type(); - - LOG.info("Retiring and deprovisioning " + host + ": On stale OS version " + - host.status().osVersion().current().map(Version::toFullString).orElse("<unset>") + - ", want " + target); - NodeList children = allNodes.childrenOf(host); - nodeRepository.nodes().retire(NodeListFilter.from(children.asList()), Agent.RetiringUpgrader, now); - host = host.withWantToRetire(true, true, Agent.RetiringUpgrader, now); - host = host.with(host.status().withOsVersion(host.status().osVersion().withWanted(Optional.of(target)))); - nodeRepository.nodes().write(host, lock); - nodeRepository.osVersions().writeChange((change) -> change.withRetirementAt(now, nodeType)); - } + /** Upgrade given host by retiring and deprovisioning it */ + private void upgrade(Node host, Version target, Instant now) { + LOG.info("Retiring and deprovisioning " + host + ": On stale OS version " + + host.status().osVersion().current().map(Version::toFullString).orElse("<unset>") + + ", want " + target); + nodeRepository.nodes().deprovision(host, Agent.RetiringUpgrader, now); + nodeRepository.nodes().upgradeOs(NodeListFilter.from(host), Optional.of(target)); + NodeType nodeType = host.type(); + nodeRepository.osVersions().writeChange((change) -> change.withRetirementAt(now, nodeType)); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index c2fe063dae6..3ed29e14527 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -533,15 +533,15 @@ public class CuratorDatabaseClient { .collect(Collectors.toUnmodifiableList()); } - /** Returns a given number of unique provision indexes */ - public List<Integer> getProvisionIndexes(int numIndexes) { - if (numIndexes < 1) - throw new IllegalArgumentException("numIndexes must be a positive integer, was " + numIndexes); - - int firstProvisionIndex = (int) provisionIndexCounter.add(numIndexes) - numIndexes; - return IntStream.range(0, numIndexes) - .mapToObj(i -> firstProvisionIndex + i) - .collect(Collectors.toList()); + /** Returns a given number of unique provision indices */ + public List<Integer> readProvisionIndices(int count) { + if (count < 1) + throw new IllegalArgumentException("count must be a positive integer, was " + count); + + int firstIndex = (int) provisionIndexCounter.add(count) - count; + return IntStream.range(0, count) + .mapToObj(i -> firstIndex + i) + .collect(Collectors.toList()); } public CacheStats cacheStats() { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index 0a22cc1cc58..7e4dc8d5108 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -6,8 +6,8 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.OutOfCapacityException; -import com.yahoo.lang.MutableInteger; import com.yahoo.transaction.Mutex; +import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.Flags; @@ -35,6 +35,7 @@ public class GroupPreparer { private final NodeRepository nodeRepository; private final Optional<HostProvisioner> hostProvisioner; private final StringFlag allocateOsRequirementFlag; + private final BooleanFlag provisionConfigServerDynamically; public GroupPreparer(NodeRepository nodeRepository, Optional<HostProvisioner> hostProvisioner, @@ -42,6 +43,7 @@ public class GroupPreparer { this.nodeRepository = nodeRepository; this.hostProvisioner = hostProvisioner; this.allocateOsRequirementFlag = Flags.ALLOCATE_OS_REQUIREMENT.bindTo(flagSource); + this.provisionConfigServerDynamically = Flags.DYNAMIC_CONFIG_SERVER_PROVISIONING.bindTo(flagSource); } /** @@ -88,21 +90,27 @@ public class GroupPreparer { NodeAllocation allocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, indices::next, wantedGroups, allocationLock, allocateOsRequirement); - - if (nodeRepository.zone().getCloud().dynamicProvisioning()) { + NodeType hostType = allocation.nodeType().hostType(); + boolean hostTypeSupportsDynamicProvisioning = hostType == NodeType.host || + (hostType == NodeType.confighost && + provisionConfigServerDynamically.value()); + if (nodeRepository.zone().getCloud().dynamicProvisioning() && hostTypeSupportsDynamicProvisioning) { final Version osVersion; if (allocateOsRequirement.equals("rhel8")) { osVersion = new Version(8, Integer.MAX_VALUE /* always use latest 8 version */, 0); } else { - osVersion = nodeRepository.osVersions().targetFor(NodeType.host).orElse(Version.emptyVersion); + osVersion = nodeRepository.osVersions().targetFor(hostType).orElse(Version.emptyVersion); } - - List<ProvisionedHost> provisionedHosts = allocation.getFulfilledDockerDeficit() - .map(deficit -> hostProvisioner.get().provisionHosts(nodeRepository.database().getProvisionIndexes(deficit.getCount()), - deficit.getFlavor(), - application, - osVersion, - requestedNodes.isExclusive() ? HostSharing.exclusive : HostSharing.any)) + HostSharing sharing = hostSharing(requestedNodes, hostType); + List<ProvisionedHost> provisionedHosts = allocation.nodeDeficit() + .map(deficit -> { + return hostProvisioner.get().provisionHosts(allocation.provisionIndices(deficit.getCount()), + hostType, + deficit.getFlavor(), + application, + osVersion, + sharing); + }) .orElseGet(List::of); // At this point we have started provisioning of the hosts, the first priority is to make sure that @@ -147,4 +155,12 @@ public class GroupPreparer { return allocation; } + private static HostSharing hostSharing(NodeSpec spec, NodeType hostType) { + HostSharing sharing = spec.isExclusive() ? HostSharing.exclusive : HostSharing.any; + if (!hostType.isSharable() && sharing != HostSharing.any) { + throw new IllegalArgumentException(hostType + " does not support sharing requirement"); + } + return sharing; + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java index ae8c6757b5a..bfb526a518f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.provision.provisioning; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; import java.util.List; @@ -32,8 +33,9 @@ public interface HostProvisioner { /** * Schedule provisioning of a given number of hosts. * - * @param provisionIndexes list of unique provision indexes which will be used to generate the node hostnames + * @param provisionIndices list of unique provision indices which will be used to generate the node hostnames * on the form of <code>[prefix][index].[domain]</code> + * @param hostType The host type to provision * @param resources the resources needed per node - the provisioned host may be significantly larger * @param applicationId id of the application that will own the provisioned host * @param osVersion the OS version to use. If this version does not exist, implementations may choose a suitable @@ -41,8 +43,11 @@ public interface HostProvisioner { * @param sharing puts requirements on sharing or exclusivity of the host to be provisioned. * @return list of {@link ProvisionedHost} describing the provisioned nodes */ - List<ProvisionedHost> provisionHosts(List<Integer> provisionIndexes, NodeResources resources, - ApplicationId applicationId, Version osVersion, + List<ProvisionedHost> provisionHosts(List<Integer> provisionIndices, + NodeType hostType, + NodeResources resources, + ApplicationId applicationId, + Version osVersion, HostSharing sharing); /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index 0eb933a7dcc..f6e0ede4e7d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -281,7 +281,7 @@ class NodeAllocation { /** Returns true if the content of this list is sufficient to meet the request */ boolean fulfilled() { - return requestedNodes.fulfilledBy(accepted); + return requestedNodes.fulfilledBy(accepted()); } /** Returns true if this allocation was already fulfilled and resulted in no new changes */ @@ -290,17 +290,49 @@ class NodeAllocation { } /** - * Returns {@link FlavorCount} describing the docker node deficit for the given {@link NodeSpec}. + * Returns {@link FlavorCount} describing the node deficit for the given {@link NodeSpec}. * - * @return empty if the requested spec is not count based or the requested flavor type is not docker or - * the request is already fulfilled. Otherwise returns {@link FlavorCount} containing the required flavor - * and node count to cover the deficit. + * @return empty if the requested spec is already fulfilled. Otherwise returns {@link FlavorCount} containing the + * flavor and node count required to cover the deficit. */ - Optional<FlavorCount> getFulfilledDockerDeficit() { - return Optional.of(requestedNodes) - .filter(NodeSpec.CountNodeSpec.class::isInstance) - .map(spec -> new FlavorCount(spec.resources().get(), spec.fulfilledDeficitCount(accepted))) - .filter(flavorCount -> flavorCount.getCount() > 0); + Optional<FlavorCount> nodeDeficit() { + if (nodeType() != NodeType.config && nodeType() != NodeType.tenant) { + return Optional.empty(); // Requests for these node types never have a deficit + } + return Optional.of(new FlavorCount(requestedNodes.resources().orElseGet(NodeResources::unspecified), + requestedNodes.fulfilledDeficitCount(accepted()))) + .filter(flavorCount -> flavorCount.getCount() > 0); + } + + /** Returns the indices to use when provisioning hosts for this */ + List<Integer> provisionIndices(int count) { + if (count < 1) throw new IllegalArgumentException("Count must be positive"); + NodeType hostType = requestedNodes.type().hostType(); + + // Tenant hosts have a continuously increasing index + if (hostType == NodeType.host) return nodeRepository.database().readProvisionIndices(count); + + // Infrastructure hosts have fixed indices, starting at 1 + Set<Integer> currentIndices = allNodes.nodeType(hostType) + .stream() + .map(Node::hostname) + // TODO(mpolden): Use cluster index instead of parsing hostname, once all + // config servers have been replaced once and have switched + // to compact indices + .map(NodeAllocation::parseIndex) + .collect(Collectors.toSet()); + List<Integer> indices = new ArrayList<>(count); + for (int i = 1; indices.size() < count; i++) { + if (!currentIndices.contains(i)) { + indices.add(i); + } + } + return indices; + } + + /** The node type this is allocating */ + NodeType nodeType() { + return requestedNodes.type(); } /** @@ -367,6 +399,14 @@ class NodeAllocation { .collect(Collectors.toList()); } + /** Returns the number of nodes accepted this far */ + private int accepted() { + if (nodeType() == NodeType.tenant) return accepted; + // Infrastructure nodes are always allocated by type. Count all nodes as accepted so that we never exceed + // the wanted number of nodes for the type. + return allNodes.nodeType(nodeType()).size(); + } + /** Prefer to retire nodes we want the least */ private List<NodeCandidate> byRetiringPriority(Collection<NodeCandidate> candidates) { return candidates.stream().sorted(Comparator.reverseOrder()).collect(Collectors.toList()); @@ -395,6 +435,15 @@ class NodeAllocation { return ": Not enough nodes available due to " + String.join(", ", reasons); } + private static Integer parseIndex(String hostname) { + // Node index is the first number appearing in the hostname, before the first dot + try { + return Integer.parseInt(hostname.replaceFirst("^\\D+(\\d+)\\..*", "$1")); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Could not parse index from hostname '" + hostname + "'", e); + } + } + static class FlavorCount { private final NodeResources flavor; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java index b37c7b92ea4..c3cb805499c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; +import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -170,6 +171,8 @@ public interface NodeSpec { /** A node spec specifying a node type. This will accept all nodes of this type. */ class TypeNodeSpec implements NodeSpec { + private static final Map<NodeType, Integer> WANTED_NODE_COUNT = Map.of(NodeType.config, 3); + private final NodeType type; public TypeNodeSpec(NodeType type) { @@ -204,14 +207,17 @@ public interface NodeSpec { @Override public int fulfilledDeficitCount(int count) { - return 0; + // If no wanted count is specified for this node type, then any count fulfills the deficit + return Math.max(0, WANTED_NODE_COUNT.getOrDefault(type, 0) - count); } @Override public NodeSpec fraction(int divisor) { return this; } @Override - public Optional<NodeResources> resources() { return Optional.empty(); } + public Optional<NodeResources> resources() { + return Optional.empty(); + } @Override public boolean needsResize(Node node) { return false; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java index 6ff4d4ca5f8..caaea1167b5 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java @@ -27,20 +27,23 @@ public class ProvisionedHost { private final String id; private final String hostHostname; private final Flavor hostFlavor; + private final NodeType hostType; private final Optional<ApplicationId> exclusiveTo; private final List<Address> nodeAddresses; private final NodeResources nodeResources; private final Version osVersion; - public ProvisionedHost(String id, String hostHostname, Flavor hostFlavor, Optional<ApplicationId> exclusiveTo, + public ProvisionedHost(String id, String hostHostname, Flavor hostFlavor, NodeType hostType, Optional<ApplicationId> exclusiveTo, List<Address> nodeAddresses, NodeResources nodeResources, Version osVersion) { this.id = Objects.requireNonNull(id, "Host id must be set"); this.hostHostname = Objects.requireNonNull(hostHostname, "Host hostname must be set"); this.hostFlavor = Objects.requireNonNull(hostFlavor, "Host flavor must be set"); + this.hostType = Objects.requireNonNull(hostType, "Host type must be set"); this.exclusiveTo = Objects.requireNonNull(exclusiveTo, "exclusiveTo must be set"); this.nodeAddresses = validateNodeAddresses(nodeAddresses); this.nodeResources = Objects.requireNonNull(nodeResources, "Node resources must be set"); this.osVersion = Objects.requireNonNull(osVersion, "OS version must be set"); + if (!hostType.isHost()) throw new IllegalArgumentException(hostType + " is not a host"); } private static List<Address> validateNodeAddresses(List<Address> nodeAddresses) { @@ -54,7 +57,7 @@ public class ProvisionedHost { /** Generate {@link Node} instance representing the provisioned physical host */ public Node generateHost() { Node.Builder builder = Node - .create(id, IP.Config.of(Set.of(), Set.of(), nodeAddresses), hostHostname, hostFlavor, NodeType.host) + .create(id, IP.Config.of(Set.of(), Set.of(), nodeAddresses), hostHostname, hostFlavor, hostType) .status(Status.initial().withOsVersion(OsVersion.EMPTY.withCurrent(Optional.of(osVersion)))); exclusiveTo.ifPresent(builder::exclusiveTo); return builder.build(); @@ -62,7 +65,7 @@ public class ProvisionedHost { /** Generate {@link Node} instance representing the node running on this physical host */ public Node generateNode() { - return Node.reserve(Set.of(), nodeHostname(), hostHostname, nodeResources, NodeType.tenant).build(); + return Node.reserve(Set.of(), nodeHostname(), hostHostname, nodeResources, hostType.childNodeType()).build(); } public String getId() { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java new file mode 100644 index 00000000000..25e74df677b --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java @@ -0,0 +1,182 @@ +// 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.testutils; + +import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.Flavor; +import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.OutOfCapacityException; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.node.Address; +import com.yahoo.vespa.hosted.provision.node.IP; +import com.yahoo.vespa.hosted.provision.provisioning.FatalProvisioningException; +import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner; +import com.yahoo.vespa.hosted.provision.provisioning.ProvisionedHost; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +/** + * @author mpolden + */ +public class MockHostProvisioner implements HostProvisioner { + + private final List<ProvisionedHost> provisionedHosts = new ArrayList<>(); + private final List<Flavor> flavors; + private final MockNameResolver nameResolver; + private final int memoryTaxGb; + + private int deprovisionedHosts = 0; + private EnumSet<Behaviour> behaviours = EnumSet.noneOf(Behaviour.class); + private Optional<Flavor> hostFlavor = Optional.empty(); + + public MockHostProvisioner(List<Flavor> flavors, MockNameResolver nameResolver, int memoryTaxGb) { + this.flavors = List.copyOf(flavors); + this.nameResolver = nameResolver; + this.memoryTaxGb = memoryTaxGb; + } + + public MockHostProvisioner(List<Flavor> flavors) { + this(flavors, 0); + } + + public MockHostProvisioner(List<Flavor> flavors, int memoryTaxGb) { + this(flavors, new MockNameResolver().mockAnyLookup(), memoryTaxGb); + } + + @Override + public List<ProvisionedHost> provisionHosts(List<Integer> provisionIndices, NodeType hostType, NodeResources resources, + ApplicationId applicationId, Version osVersion, HostSharing sharing) { + Flavor hostFlavor = this.hostFlavor.orElseGet(() -> flavors.stream().filter(f -> compatible(f, resources)) + .findFirst() + .orElseThrow(() -> new OutOfCapacityException("No host flavor matches " + resources))); + List<ProvisionedHost> hosts = new ArrayList<>(); + for (int index : provisionIndices) { + String hostHostname = hostType == NodeType.host ? "hostname" + index : hostType.name() + index; + hosts.add(new ProvisionedHost("id-of-" + hostType.name() + index, + hostHostname, + hostFlavor, + hostType, + Optional.empty(), + createAddressesForHost(hostType, hostFlavor, index), + resources, + osVersion)); + } + provisionedHosts.addAll(hosts); + return hosts; + } + + @Override + public List<Node> provision(Node host, Set<Node> children) throws FatalProvisioningException { + if (behaviours.contains(Behaviour.failProvisioning)) throw new FatalProvisioningException("Failed to provision node(s)"); + if (host.state() != Node.State.provisioned) throw new IllegalStateException("Host to provision must be in " + Node.State.provisioned); + List<Node> result = new ArrayList<>(); + result.add(withIpAssigned(host)); + for (var child : children) { + if (child.state() != Node.State.reserved) throw new IllegalStateException("Child to provisioned must be in " + Node.State.reserved); + result.add(withIpAssigned(child)); + } + return result; + } + + @Override + public void deprovision(Node host) { + if (behaviours.contains(Behaviour.failDeprovisioning)) throw new FatalProvisioningException("Failed to deprovision node"); + provisionedHosts.removeIf(provisionedHost -> provisionedHost.hostHostname().equals(host.hostname())); + deprovisionedHosts++; + } + + /** Returns the hosts that have been provisioned by this */ + public List<ProvisionedHost> provisionedHosts() { + return Collections.unmodifiableList(provisionedHosts); + } + + /** Returns the number of hosts deprovisioned by this */ + public int deprovisionedHosts() { + return deprovisionedHosts; + } + + public MockHostProvisioner with(Behaviour first, Behaviour... rest) { + this.behaviours = EnumSet.of(first, rest); + return this; + } + + public MockHostProvisioner without(Behaviour first, Behaviour... rest) { + Set<Behaviour> behaviours = new HashSet<>(this.behaviours); + behaviours.removeAll(EnumSet.of(first, rest)); + this.behaviours = behaviours.isEmpty() ? EnumSet.noneOf(Behaviour.class) : EnumSet.copyOf(behaviours); + return this; + } + + public MockHostProvisioner overrideHostFlavor(String flavorName) { + Flavor flavor = flavors.stream().filter(f -> f.name().equals(flavorName)) + .findFirst() + .orElseThrow(() -> new IllegalArgumentException("No such flavor '" + flavorName + "'")); + hostFlavor = Optional.of(flavor); + return this; + } + + public boolean compatible(Flavor flavor, NodeResources resources) { + NodeResources resourcesToVerify = resources.withMemoryGb(resources.memoryGb() - memoryTaxGb); + + if (flavor.resources().storageType() == NodeResources.StorageType.remote + && flavor.resources().diskGb() >= resources.diskGb()) + resourcesToVerify = resourcesToVerify.withDiskGb(flavor.resources().diskGb()); + if (flavor.resources().bandwidthGbps() >= resources.bandwidthGbps()) + resourcesToVerify = resourcesToVerify.withBandwidthGbps(flavor.resources().bandwidthGbps()); + return flavor.resources().compatibleWith(resourcesToVerify); + } + + private List<Address> createAddressesForHost(NodeType hostType, Flavor flavor, int hostIndex) { + long numAddresses = Math.max(1, Math.round(flavor.resources().bandwidthGbps())); + return IntStream.range(0, (int) numAddresses) + .mapToObj(i -> { + String hostname = hostType == NodeType.host + ? "nodename" + hostIndex + "_" + i + : hostType.childNodeType().name() + i; + return new Address(hostname); + }) + .collect(Collectors.toList()); + } + + private Node withIpAssigned(Node node) { + if (!node.type().isHost()) { + return node.with(node.ipConfig().withPrimary(nameResolver.resolveAll(node.hostname()))); + } + int hostIndex = Integer.parseInt(node.hostname().replaceAll("^[a-z]+|-\\d+$", "")); + Set<String> addresses = Set.of("::" + hostIndex + ":0"); + Set<String> ipAddressPool = new HashSet<>(); + if (!behaviours.contains(Behaviour.failDnsUpdate)) { + nameResolver.addRecord(node.hostname(), addresses.iterator().next()); + for (int i = 1; i <= 2; i++) { + String ip = "::" + hostIndex + ":" + i; + ipAddressPool.add(ip); + nameResolver.addRecord(node.hostname() + "-" + i, ip); + } + } + IP.Pool pool = node.ipConfig().pool().withIpAddresses(ipAddressPool); + return node.with(node.ipConfig().withPrimary(addresses).withPool(pool)); + } + + public enum Behaviour { + + /** Fail all calls to {@link MockHostProvisioner#provision(com.yahoo.vespa.hosted.provision.Node, java.util.Set)} */ + failProvisioning, + + /** Fail all calls to {@link MockHostProvisioner#deprovision(com.yahoo.vespa.hosted.provision.Node)} */ + failDeprovisioning, + + /** Fail DNS updates of provisioned hosts */ + failDnsUpdate, + + } + +} 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 0619b0ad645..77c3a5209e2 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 @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.provision.autoscale; import com.yahoo.collections.Pair; -import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterResources; @@ -21,17 +20,12 @@ import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.Nodelike; import com.yahoo.vespa.hosted.provision.applications.Application; -import com.yahoo.vespa.hosted.provision.node.Address; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.IP; -import com.yahoo.vespa.hosted.provision.provisioning.FatalProvisioningException; -import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner; import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; -import com.yahoo.vespa.hosted.provision.provisioning.ProvisionedHost; import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; import java.time.Duration; -import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.Set; @@ -39,6 +33,9 @@ import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +/** + * @author bratseth + */ class AutoscalingTester { private final ProvisioningTester provisioningTester; @@ -296,45 +293,14 @@ class AutoscalingTester { } - private class MockHostProvisioner implements HostProvisioner { + private class MockHostProvisioner extends com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner { - private final List<Flavor> hostFlavors; - - public MockHostProvisioner(List<Flavor> hostFlavors) { - this.hostFlavors = hostFlavors; - } - - @Override - public List<ProvisionedHost> provisionHosts(List<Integer> provisionIndexes, NodeResources resources, - ApplicationId applicationId, Version osVersion, - HostSharing sharing) { - Flavor hostFlavor = hostFlavors.stream().filter(f -> matches(f, resources)).findAny() - .orElseThrow(() -> new RuntimeException("No flavor matching " + resources + ". Flavors: " + hostFlavors)); - - List<ProvisionedHost> hosts = new ArrayList<>(); - for (int index : provisionIndexes) { - hosts.add(new ProvisionedHost("host" + index, - "hostname" + index, - hostFlavor, - Optional.empty(), - List.of(new Address("nodename" + index)), - resources, - osVersion)); - } - return hosts; + public MockHostProvisioner(List<Flavor> flavors) { + super(flavors); } @Override - public List<Node> provision(Node host, Set<Node> children) throws FatalProvisioningException { - throw new RuntimeException("Not implemented"); - } - - @Override - public void deprovision(Node host) { - throw new RuntimeException("Not implemented"); - } - - private boolean matches(Flavor flavor, NodeResources resources) { + public boolean compatible(Flavor flavor, NodeResources resources) { NodeResources flavorResources = hostResourcesCalculator.advertisedResourcesOf(flavor); if (flavorResources.storageType() == NodeResources.StorageType.remote && resources.diskGb() <= flavorResources.diskGb()) diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java index 56cf8d02149..f47bcd0d550 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java @@ -12,9 +12,11 @@ import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ParentHostUnavailableException; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.flags.custom.ClusterCapacity; @@ -27,11 +29,10 @@ import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.Allocation; import com.yahoo.vespa.hosted.provision.node.Generation; import com.yahoo.vespa.hosted.provision.node.IP; -import com.yahoo.vespa.hosted.provision.provisioning.FatalProvisioningException; import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; -import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner; import com.yahoo.vespa.hosted.provision.provisioning.ProvisionedHost; import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; +import com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner; import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; import com.yahoo.vespa.service.duper.ConfigServerApplication; import com.yahoo.vespa.service.duper.ConfigServerHostApplication; @@ -39,21 +40,19 @@ import org.junit.Test; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; -import java.util.EnumSet; -import java.util.HashSet; +import java.util.Comparator; import java.util.List; import java.util.Optional; import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; -import java.util.stream.IntStream; import java.util.stream.Stream; -import static com.yahoo.vespa.hosted.provision.maintenance.DynamicProvisioningMaintainerTest.MockHostProvisioner.Behaviour; +import static com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner.Behaviour; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author freva @@ -115,7 +114,7 @@ public class DynamicProvisioningMaintainerTest { tester.maintainer.maintain(); assertTrue("Failed host is deprovisioned", tester.nodeRepository.nodes().node(failedHost.get().hostname()).isEmpty()); - assertEquals(1, tester.hostProvisioner.deprovisionedHosts); + assertEquals(1, tester.hostProvisioner.deprovisionedHosts()); } @Test @@ -126,7 +125,7 @@ public class DynamicProvisioningMaintainerTest { new ClusterCapacity(1, 16, 24, 100, 1.0)), ClusterCapacity.class); - assertEquals(0, tester.hostProvisioner.provisionedHosts.size()); + assertEquals(0, tester.hostProvisioner.provisionedHosts().size()); assertEquals(11, tester.nodeRepository.nodes().list().size()); assertTrue(tester.nodeRepository.nodes().node("host2").isPresent()); assertTrue(tester.nodeRepository.nodes().node("host2-1").isPresent()); @@ -136,7 +135,7 @@ public class DynamicProvisioningMaintainerTest { tester.maintainer.maintain(); - assertEquals(2, tester.hostProvisioner.provisionedHosts.size()); + assertEquals(2, tester.hostProvisioner.provisionedHosts().size()); assertEquals(2, tester.provisionedHostsMatching(new NodeResources(48, 128, 1000, 10))); NodeList nodesAfter = tester.nodeRepository.nodes().list(); assertEquals(11, nodesAfter.size()); // 2 removed, 2 added @@ -151,13 +150,13 @@ public class DynamicProvisioningMaintainerTest { public void preprovision_with_shared_host() { var tester = new DynamicProvisioningTester().addInitialNodes(); // Makes provisioned hosts 48-128-1000-10 - tester.hostProvisioner.provisionSharedHost("host4"); + tester.hostProvisioner.overrideHostFlavor("host4"); tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), List.of(new ClusterCapacity(2, 1, 30, 20, 3.0)), ClusterCapacity.class); - assertEquals(0, tester.hostProvisioner.provisionedHosts.size()); + assertEquals(0, tester.hostProvisioner.provisionedHosts().size()); assertEquals(11, tester.nodeRepository.nodes().list().size()); assertTrue(tester.nodeRepository.nodes().node("host2").isPresent()); assertTrue(tester.nodeRepository.nodes().node("host2-1").isPresent()); @@ -196,7 +195,7 @@ public class DynamicProvisioningMaintainerTest { tester.maintainer.maintain(); - assertEquals(2, tester.hostProvisioner.provisionedHosts.size()); + assertEquals(2, tester.hostProvisioner.provisionedHosts().size()); assertEquals(2, tester.provisionedHostsMatching(new NodeResources(48, 128, 1000, 10))); assertEquals(10, tester.nodeRepository.nodes().list().size()); // 3 removed, 2 added assertTrue("preprovision capacity is prefered on shared hosts", tester.nodeRepository.nodes().node("host3").isEmpty()); @@ -212,7 +211,7 @@ public class DynamicProvisioningMaintainerTest { tester.maintainer.maintain(); assertEquals("one provisioned host has been deprovisioned, so there are 2 -> 1 provisioned hosts", - 1, tester.hostProvisioner.provisionedHosts.size()); + 1, tester.hostProvisioner.provisionedHosts().size()); assertEquals(1, tester.provisionedHostsMatching(new NodeResources(48, 128, 1000, 10))); assertEquals(9, tester.nodeRepository.nodes().list().size()); // 4 removed, 2 added if (tester.nodeRepository.nodes().node("hostname100").isPresent()) { @@ -226,7 +225,7 @@ public class DynamicProvisioningMaintainerTest { } private void verifyFirstMaintain(DynamicProvisioningTester tester) { - assertEquals(1, tester.hostProvisioner.provisionedHosts.size()); + assertEquals(1, tester.hostProvisioner.provisionedHosts().size()); assertEquals(1, tester.provisionedHostsMatching(new NodeResources(48, 128, 1000, 10))); assertEquals(10, tester.nodeRepository.nodes().list().size()); // 2 removed, 1 added assertTrue("Failed host 'host2' is deprovisioned", tester.nodeRepository.nodes().node("host2").isEmpty()); @@ -266,17 +265,17 @@ public class DynamicProvisioningMaintainerTest { private void assertWithMinCount(int minCount, int provisionCount, int deprovisionCount) { var tester = new DynamicProvisioningTester().addInitialNodes(); - tester.hostProvisioner.provisionSharedHost("host4"); + tester.hostProvisioner.overrideHostFlavor("host4"); tester.flagSource.withJacksonFlag(PermanentFlags.SHARED_HOST.id(), new SharedHost(null, minCount), SharedHost.class); tester.maintainer.maintain(); - assertEquals(provisionCount, tester.hostProvisioner.provisionedHosts.size()); - assertEquals(deprovisionCount, tester.hostProvisioner.deprovisionedHosts); + assertEquals(provisionCount, tester.hostProvisioner.provisionedHosts().size()); + assertEquals(deprovisionCount, tester.hostProvisioner.deprovisionedHosts()); // Verify next maintain is a no-op tester.maintainer.maintain(); - assertEquals(provisionCount, tester.hostProvisioner.provisionedHosts.size()); - assertEquals(deprovisionCount, tester.hostProvisioner.deprovisionedHosts); + assertEquals(provisionCount, tester.hostProvisioner.provisionedHosts().size()); + assertEquals(deprovisionCount, tester.hostProvisioner.deprovisionedHosts()); } @Test @@ -300,14 +299,14 @@ public class DynamicProvisioningMaintainerTest { // Hosts are provisioned assertEquals(2, tester.provisionedHostsMatching(resources1)); - assertEquals(0, tester.hostProvisioner.deprovisionedHosts); + assertEquals(0, tester.hostProvisioner.deprovisionedHosts()); // Next maintenance run does nothing tester.assertNodesUnchanged(); // Pretend shared-host flag has been set to host4's flavor var sharedHostNodeResources = new NodeResources(48, 128, 1000, 10, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote); - tester.hostProvisioner.provisionSharedHost("host4"); + tester.hostProvisioner.overrideHostFlavor("host4"); // Next maintenance run does nothing tester.assertNodesUnchanged(); @@ -421,6 +420,94 @@ public class DynamicProvisioningMaintainerTest { assertCfghost3IsDeprovisioned(tester); } + @Test + public void replace_config_server() { + Cloud cloud = Cloud.builder().dynamicProvisioning(true).build(); + DynamicProvisioningTester dynamicProvisioningTester = new DynamicProvisioningTester(cloud, new MockNameResolver().mockAnyLookup()); + ProvisioningTester tester = dynamicProvisioningTester.provisioningTester; + dynamicProvisioningTester.hostProvisioner.overrideHostFlavor("default"); + dynamicProvisioningTester.flagSource.withBooleanFlag(Flags.DYNAMIC_CONFIG_SERVER_PROVISIONING.id(), true); + + // Initial config server hosts are provisioned manually + ApplicationId hostApp = ApplicationId.from("hosted-vespa", "configserver-host", "default"); + List<Node> provisionedHosts = tester.makeReadyNodes(3, "default", NodeType.confighost).stream() + .sorted(Comparator.comparing(Node::hostname)) + .collect(Collectors.toList()); + tester.prepareAndActivateInfraApplication(hostApp, NodeType.confighost); + + // Provision config servers + ApplicationId configSrvApp = ApplicationId.from("hosted-vespa", "zone-config-servers", "default"); + for (int i = 0; i < provisionedHosts.size(); i++) { + tester.makeReadyChildren(1, i + 1, NodeResources.unspecified(), NodeType.config, + provisionedHosts.get(i).hostname(), (nodeIndex) -> "cfg" + nodeIndex); + } + tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + + // Expected number of hosts and children are provisioned + NodeList allNodes = tester.nodeRepository().nodes().list(); + NodeList configHosts = allNodes.nodeType(NodeType.confighost); + NodeList configNodes = allNodes.nodeType(NodeType.config); + assertEquals(3, configHosts.size()); + assertEquals(3, configNodes.size()); + String hostnameToRemove = provisionedHosts.get(1).hostname(); + Supplier<Node> hostToRemove = () -> tester.nodeRepository().nodes().node(hostnameToRemove).get(); + Supplier<Node> nodeToRemove = () -> tester.nodeRepository().nodes().node(configNodes.childrenOf(hostnameToRemove).first().get().hostname()).get(); + + // Retire and deprovision host + tester.nodeRepository().nodes().deprovision(hostToRemove.get(), Agent.system, tester.clock().instant()); + tester.nodeRepository().nodes().deallocate(hostToRemove.get(), Agent.system, getClass().getSimpleName()); + assertSame("Host moves to parked", Node.State.parked, hostToRemove.get().state()); + assertSame("Node remains active", Node.State.active, nodeToRemove.get().state()); + assertTrue("Node wants to retire", nodeToRemove.get().status().wantToRetire()); + + // Redeployment of config server application retires node + tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + assertTrue("Redeployment retires node", nodeToRemove.get().allocation().get().membership().retired()); + + // Config server becomes removable (done by RetiredExpirer in a real system) and redeployment moves it + // to inactive + tester.nodeRepository().nodes().setRemovable(configSrvApp, List.of(nodeToRemove.get())); + tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + assertEquals("Node moves to inactive", Node.State.inactive, nodeToRemove.get().state()); + + // Node is completely removed (done by InactiveExpirer and host-admin in a real system) + Node inactiveConfigServer = nodeToRemove.get(); + int removedIndex = inactiveConfigServer.allocation().get().membership().index(); + tester.nodeRepository().nodes().removeRecursively(inactiveConfigServer, true); + assertEquals(2, tester.nodeRepository().nodes().list().nodeType(NodeType.config).size()); + + // Host is removed + dynamicProvisioningTester.maintainer.maintain(); + assertEquals(2, tester.nodeRepository().nodes().list().nodeType(NodeType.confighost).size()); + + // Next deployment starts provisioning a new host and child + try { + tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + fail("Expected provisioning to fail"); + } catch (ParentHostUnavailableException ignored) {} + Node newNode = tester.nodeRepository().nodes().list(Node.State.reserved).nodeType(NodeType.config).first().get(); + + // Resume provisioning and activate host + dynamicProvisioningTester.maintainer.maintain(); + List<ProvisionedHost> newHosts = dynamicProvisioningTester.hostProvisioner.provisionedHosts(); + assertEquals(1, newHosts.size()); + tester.nodeRepository().nodes().setReady(newHosts.get(0).hostHostname(), Agent.operator, getClass().getSimpleName()); + tester.prepareAndActivateInfraApplication(hostApp, NodeType.confighost); + assertEquals(3, tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.confighost).size()); + + // Redeployment of config server app actives new node + tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + newNode = tester.nodeRepository().nodes().node(newNode.hostname()).get(); + assertSame(Node.State.active, newNode.state()); + assertEquals("Removed index is reused", removedIndex, newNode.allocation().get().membership().index()); + + // Next redeployment does nothing + NodeList nodesBefore = tester.nodeRepository().nodes().list().nodeType(NodeType.config); + tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + NodeList nodesAfter = tester.nodeRepository().nodes().list().nodeType(NodeType.config); + assertEquals(nodesBefore, nodesAfter); + } + private void assertCfghost3IsActive(DynamicProvisioningTester tester) { assertEquals(5, tester.nodeRepository.nodes().list(Node.State.active).size()); assertEquals(3, tester.nodeRepository.nodes().list(Node.State.active).nodeType(NodeType.confighost).size()); @@ -451,17 +538,17 @@ public class DynamicProvisioningMaintainerTest { private final ProvisioningTester provisioningTester; public DynamicProvisioningTester() { - this(Cloud.builder().dynamicProvisioning(true).build()); + this(Cloud.builder().dynamicProvisioning(true).build(), new MockNameResolver()); } - public DynamicProvisioningTester(Cloud cloud) { - MockNameResolver nameResolver = new MockNameResolver(); - this.hostProvisioner = new MockHostProvisioner(flavors, nameResolver); + public DynamicProvisioningTester(Cloud cloud, MockNameResolver nameResolver) { + this.hostProvisioner = new MockHostProvisioner(flavors.getFlavors(), nameResolver, 0); this.provisioningTester = new ProvisioningTester.Builder().zone(new Zone(cloud, SystemName.defaultSystem(), Environment.defaultEnvironment(), RegionName.defaultName())) .flavors(flavors.getFlavors()) .nameResolver(nameResolver) + .flagSource(flagSource) .hostProvisioner(hostProvisioner) .build(); this.nodeRepository = provisioningTester.nodeRepository(); @@ -529,9 +616,9 @@ public class DynamicProvisioningMaintainerTest { } private long provisionedHostsMatching(NodeResources resources) { - return hostProvisioner.provisionedHosts.stream() - .filter(host -> host.generateHost().resources().compatibleWith(resources)) - .count(); + return hostProvisioner.provisionedHosts().stream() + .filter(host -> host.generateHost().resources().compatibleWith(resources)) + .count(); } private void assertNodesUnchanged() { @@ -542,113 +629,5 @@ public class DynamicProvisioningMaintainerTest { } - static class MockHostProvisioner implements HostProvisioner { - - private final List<ProvisionedHost> provisionedHosts = new ArrayList<>(); - private final NodeFlavors flavors; - private final MockNameResolver nameResolver; - - private int deprovisionedHosts = 0; - private EnumSet<Behaviour> behaviours = EnumSet.noneOf(Behaviour.class); - private Optional<Flavor> provisionHostFlavor = Optional.empty(); - - public MockHostProvisioner(NodeFlavors flavors, MockNameResolver nameResolver) { - this.flavors = flavors; - this.nameResolver = nameResolver; - } - - public MockHostProvisioner provisionSharedHost(String flavorName) { - provisionHostFlavor = Optional.of(flavors.getFlavorOrThrow(flavorName)); - return this; - } - - @Override - public List<ProvisionedHost> provisionHosts(List<Integer> provisionIndexes, NodeResources resources, - ApplicationId applicationId, Version osVersion, HostSharing sharing) { - Flavor hostFlavor = provisionHostFlavor - .orElseGet(() -> flavors.getFlavors().stream() - .filter(f -> !f.isDocker()) - .filter(f -> f.resources().compatibleWith(resources)) - .findFirst() - .orElseThrow(() -> new IllegalArgumentException("No host flavor found satisfying " + resources))); - - List<ProvisionedHost> hosts = new ArrayList<>(); - for (int index : provisionIndexes) { - hosts.add(new ProvisionedHost("host" + index, - "hostname" + index, - hostFlavor, - Optional.empty(), - createAddressesForHost(hostFlavor, index), - resources, - osVersion)); - } - provisionedHosts.addAll(hosts); - return hosts; - } - - private List<Address> createAddressesForHost(Flavor flavor, int hostIndex) { - long numAddresses = Math.max(1, Math.round(flavor.resources().bandwidthGbps())); - return IntStream.range(0, (int) numAddresses) - .mapToObj(i -> new Address("nodename" + hostIndex + "_" + i)) - .collect(Collectors.toList()); - } - - @Override - public List<Node> provision(Node host, Set<Node> children) throws FatalProvisioningException { - if (behaviours.contains(Behaviour.failProvisioning)) throw new FatalProvisioningException("Failed to provision node(s)"); - assertSame(Node.State.provisioned, host.state()); - List<Node> result = new ArrayList<>(); - result.add(withIpAssigned(host)); - for (var child : children) { - assertSame(Node.State.reserved, child.state()); - result.add(withIpAssigned(child)); - } - return result; - } - - @Override - public void deprovision(Node host) { - if (behaviours.contains(Behaviour.failDeprovisioning)) throw new FatalProvisioningException("Failed to deprovision node"); - provisionedHosts.removeIf(provisionedHost -> provisionedHost.hostHostname().equals(host.hostname())); - deprovisionedHosts++; - } - - private MockHostProvisioner with(Behaviour first, Behaviour... rest) { - this.behaviours = EnumSet.of(first, rest); - return this; - } - - private MockHostProvisioner without(Behaviour first, Behaviour... rest) { - Set<Behaviour> behaviours = new HashSet<>(this.behaviours); - behaviours.removeAll(EnumSet.of(first, rest)); - this.behaviours = behaviours.isEmpty() ? EnumSet.noneOf(Behaviour.class) : EnumSet.copyOf(behaviours); - return this; - } - - private Node withIpAssigned(Node node) { - if (node.parentHostname().isPresent()) return node; - int hostIndex = Integer.parseInt(node.hostname().replaceAll("^[a-z]+|-\\d+$", "")); - Set<String> addresses = Set.of("::" + hostIndex + ":0"); - Set<String> ipAddressPool = new HashSet<>(); - if (!behaviours.contains(Behaviour.failDnsUpdate)) { - nameResolver.addRecord(node.hostname(), addresses.iterator().next()); - for (int i = 1; i <= 2; i++) { - String ip = "::" + hostIndex + ":" + i; - ipAddressPool.add(ip); - nameResolver.addRecord(node.hostname() + "-" + i, ip); - } - } - - IP.Pool pool = node.ipConfig().pool().withIpAddresses(ipAddressPool); - return node.with(node.ipConfig().withPrimary(addresses).withPool(pool)); - } - - enum Behaviour { - failProvisioning, - failDeprovisioning, - failDnsUpdate, - } - - } - } + diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java index eda744e9ee1..d5699f0cffe 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InactiveAndFailedExpirerTest.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.maintenance; +import com.yahoo.component.Vtag; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.Capacity; @@ -18,6 +19,7 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.History; +import com.yahoo.vespa.hosted.provision.node.filter.NodeListFilter; import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; import com.yahoo.vespa.hosted.provision.testutils.MockDeployer; import com.yahoo.vespa.orchestrator.OrchestrationException; @@ -28,9 +30,12 @@ import java.time.Duration; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.function.Supplier; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; @@ -64,7 +69,7 @@ public class InactiveAndFailedExpirerTest { // Inactive times out tester.advanceTime(Duration.ofMinutes(14)); - new InactiveExpirer(tester.nodeRepository(), Duration.ofMinutes(10), new TestMetric()).run(); + new InactiveExpirer(tester.nodeRepository(), Duration.ofMinutes(10), Map.of(), new TestMetric()).run(); assertEquals(0, tester.nodeRepository().nodes().list(Node.State.inactive).size()); NodeList dirty = tester.nodeRepository().nodes().list(Node.State.dirty); assertEquals(2, dirty.size()); @@ -105,7 +110,7 @@ public class InactiveAndFailedExpirerTest { // Inactive times out and node is moved to dirty tester.advanceTime(Duration.ofMinutes(14)); - new InactiveExpirer(tester.nodeRepository(), Duration.ofMinutes(10), new TestMetric()).run(); + new InactiveExpirer(tester.nodeRepository(), Duration.ofMinutes(10), Map.of(), new TestMetric()).run(); NodeList dirty = tester.nodeRepository().nodes().list(Node.State.dirty); assertEquals(2, dirty.size()); @@ -156,7 +161,7 @@ public class InactiveAndFailedExpirerTest { // Inactive times out and one node is moved to parked tester.advanceTime(Duration.ofMinutes(11)); // Trigger InactiveExpirer - new InactiveExpirer(tester.nodeRepository(), Duration.ofMinutes(10), new TestMetric()).run(); + new InactiveExpirer(tester.nodeRepository(), Duration.ofMinutes(10), Map.of(), new TestMetric()).run(); assertEquals(1, tester.nodeRepository().nodes().list(Node.State.parked).size()); } @@ -178,7 +183,7 @@ public class InactiveAndFailedExpirerTest { assertEquals(1, inactiveNodes.size()); // See that nodes are moved to dirty immediately. - new InactiveExpirer(tester.nodeRepository(), Duration.ofMinutes(10), new TestMetric()).run(); + new InactiveExpirer(tester.nodeRepository(), Duration.ofMinutes(10), Map.of(), new TestMetric()).run(); assertEquals(0, tester.nodeRepository().nodes().list(Node.State.inactive).size()); NodeList dirty = tester.nodeRepository().nodes().list(Node.State.dirty); assertEquals(1, dirty.size()); @@ -202,8 +207,31 @@ public class InactiveAndFailedExpirerTest { // Nodes marked for deprovisioning are moved to parked tester.patchNodes(inactiveNodes, (node) -> node.withWantToRetire(true, true, Agent.system, tester.clock().instant())); tester.advanceTime(Duration.ofMinutes(11)); - new InactiveExpirer(tester.nodeRepository(), Duration.ofMinutes(10), new TestMetric()).run(); + new InactiveExpirer(tester.nodeRepository(), Duration.ofMinutes(10), Map.of(), new TestMetric()).run(); assertEquals(2, tester.nodeRepository().nodes().list(Node.State.parked).size()); } + @Test + public void inactive_config_server_expires_according_to_custom_timeout() { + ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build(); + InactiveExpirer expirer = new InactiveExpirer(tester.nodeRepository(), Duration.ofHours(1), + Map.of(NodeType.config, Duration.ofMinutes(5)), + new TestMetric()); + NodeList nodes = tester.makeConfigServers(3, "default", Vtag.currentVersion); + Supplier<Node> firstNode = () -> tester.nodeRepository().nodes().node(nodes.first().get().hostname()).get(); + ApplicationId application = firstNode.get().allocation().get().owner(); + + // Retired config server is moved to inactive + tester.nodeRepository().nodes().retire(NodeListFilter.from(firstNode.get()), Agent.system, tester.clock().instant()); + tester.prepareAndActivateInfraApplication(application, NodeType.config); + assertSame(Node.State.inactive, firstNode.get().state()); + expirer.maintain(); + assertSame(Node.State.inactive, firstNode.get().state()); + + // Config server expires + tester.clock().advance(Duration.ofMinutes(5).plus(Duration.ofSeconds(1))); + expirer.maintain(); + assertSame(Node.State.dirty, firstNode.get().state()); + } + } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java index 131c02015a1..4db1b86419b 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java @@ -14,22 +14,20 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeResources.DiskSpeed; import com.yahoo.config.provision.NodeResources.StorageType; import com.yahoo.config.provision.NodeType; -import com.yahoo.config.provision.OutOfCapacityException; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; -import com.yahoo.vespa.hosted.provision.node.Address; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner.HostSharing; +import com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner; import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; import org.junit.Test; import java.time.Instant; import java.util.List; -import java.util.Optional; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -73,7 +71,7 @@ public class DynamicDockerProvisionTest { mockHostProvisioner(hostProvisioner, "large", 3, null); // Provision shared hosts prepareAndActivate(application1, clusterSpec("mycluster"), 4, 1, resources); - verify(hostProvisioner).provisionHosts(List.of(100, 101, 102, 103), resources, application1, + verify(hostProvisioner).provisionHosts(List.of(100, 101, 102, 103), NodeType.host, resources, application1, Version.emptyVersion, HostSharing.any); // Total of 8 nodes should now be in node-repo, 4 active hosts and 4 active nodes @@ -99,7 +97,7 @@ public class DynamicDockerProvisionTest { ApplicationId application3 = ProvisioningTester.applicationId(); mockHostProvisioner(hostProvisioner, "large", 3, application3); prepareAndActivate(application3, clusterSpec("mycluster", true), 4, 1, resources); - verify(hostProvisioner).provisionHosts(List.of(104, 105, 106, 107), resources, application3, + verify(hostProvisioner).provisionHosts(List.of(104, 105, 106, 107), NodeType.host, resources, application3, Version.emptyVersion, HostSharing.exclusive); // Total of 20 nodes should now be in node-repo, 8 active hosts and 12 active nodes @@ -429,7 +427,7 @@ public class DynamicDockerProvisionTest { doAnswer(invocation -> { Flavor hostFlavor = tester.nodeRepository().flavors().getFlavorOrThrow(hostFlavorName); List<Integer> provisionIndexes = (List<Integer>) invocation.getArguments()[0]; - NodeResources nodeResources = (NodeResources) invocation.getArguments()[1]; + NodeResources nodeResources = (NodeResources) invocation.getArguments()[2]; return provisionIndexes.stream() .map(hostIndex -> { @@ -451,52 +449,7 @@ public class DynamicDockerProvisionTest { return provisionedHost; }) .collect(Collectors.toList()); - }).when(hostProvisioner).provisionHosts(any(), any(), any(), any(), any()); - } - - private static class MockHostProvisioner implements HostProvisioner { - - private final List<Flavor> hostFlavors; - private final int memoryTaxGb; - - public MockHostProvisioner(List<Flavor> hostFlavors, int memoryTaxGb) { - this.hostFlavors = List.copyOf(hostFlavors); - this.memoryTaxGb = memoryTaxGb; - } - - @Override - public List<ProvisionedHost> provisionHosts(List<Integer> provisionIndexes, NodeResources resources, - ApplicationId applicationId, Version osVersion, HostSharing sharing) { - Optional<Flavor> hostFlavor = hostFlavors.stream().filter(f -> compatible(f, resources)).findFirst(); - if (hostFlavor.isEmpty()) - throw new OutOfCapacityException("No host flavor matches " + resources); - return provisionIndexes.stream() - .map(i -> new ProvisionedHost("id-" + i, "host-" + i, hostFlavor.get(), Optional.empty(), - List.of(new Address("host-" + i + "-1")), resources, osVersion)) - .collect(Collectors.toList()); - } - - private boolean compatible(Flavor hostFlavor, NodeResources resources) { - NodeResources resourcesToVerify = resources.withMemoryGb(resources.memoryGb() - memoryTaxGb); - - if (hostFlavor.resources().storageType() == NodeResources.StorageType.remote - && hostFlavor.resources().diskGb() >= resources.diskGb()) - resourcesToVerify = resourcesToVerify.withDiskGb(hostFlavor.resources().diskGb()); - if (hostFlavor.resources().bandwidthGbps() >= resources.bandwidthGbps()) - resourcesToVerify = resourcesToVerify.withBandwidthGbps(hostFlavor.resources().bandwidthGbps()); - return hostFlavor.resources().compatibleWith(resourcesToVerify); - } - - @Override - public List<Node> provision(Node host, Set<Node> children) throws FatalProvisioningException { - throw new RuntimeException("Not implemented: provision"); - } - - @Override - public void deprovision(Node host) { - throw new RuntimeException("Not implemented: deprovision"); - } - + }).when(hostProvisioner).provisionHosts(any(), any(), any(), any(), any(), any()); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java index 0986f2954a7..c269b4642ea 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java @@ -33,7 +33,6 @@ import org.junit.Test; import java.time.Duration; import java.util.Collection; -import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -1017,7 +1016,7 @@ public class ProvisioningTest { private Set<HostSpec> prepare(ApplicationId application, ProvisioningTester tester, ClusterSpec cluster, int nodeCount, int groups, boolean required, NodeResources nodeResources) { - if (nodeCount == 0) return Collections.emptySet(); // this is a shady practice + if (nodeCount == 0) return Set.of(); // this is a shady practice return new HashSet<>(tester.prepare(application, cluster, nodeCount, groups, required, nodeResources)); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index 97baddf93fa..eefbd03ce4e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -227,7 +227,10 @@ public class ProvisioningTester { } public void prepareAndActivateInfraApplication(ApplicationId application, NodeType nodeType, Version version) { - ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(nodeType.toString())).vespaVersion(version).build(); + ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(nodeType.toString())) + .vespaVersion(version) + .stateful(nodeType == NodeType.config || nodeType == NodeType.controller) + .build(); Capacity capacity = Capacity.fromRequiredNodeType(nodeType); List<HostSpec> hostSpecs = prepare(application, cluster, capacity); activate(application, hostSpecs); @@ -510,17 +513,18 @@ public class ProvisioningTester { index -> UUID.randomUUID().toString()); } - /** Creates a set of virtual nodes on a single parent host */ - public List<Node> makeReadyChildren(int count, int startIndex, NodeResources resources, String parentHostname, - Function<Integer, String> nodeNamer) { + /** Create one or more child nodes on given parent host */ + public List<Node> makeReadyChildren(int count, int startIndex, NodeResources resources, NodeType nodeType, + String parentHostname, Function<Integer, String> nodeNamer) { + if (nodeType.isHost()) throw new IllegalArgumentException("Non-child node type: " + nodeType); List<Node> nodes = new ArrayList<>(count); for (int i = startIndex; i < count + startIndex; i++) { String hostname = nodeNamer.apply(i); IP.Config ipConfig = new IP.Config(nodeRepository.nameResolver().resolveAll(hostname), Set.of()); - - Node.Builder builder = Node.create("node-id", ipConfig, hostname, new Flavor(resources), NodeType.tenant); - builder.parentHostname(parentHostname); - nodes.add(builder.build()); + Node node = Node.create("node-id", ipConfig, hostname, new Flavor(resources), nodeType) + .parentHostname(parentHostname) + .build(); + nodes.add(node); } nodes = nodeRepository.nodes().addNodes(nodes, Agent.system); nodes = nodeRepository.nodes().deallocate(nodes, Agent.system, getClass().getSimpleName()); @@ -528,6 +532,12 @@ public class ProvisioningTester { return nodes; } + /** Create one or more child nodes on given parent host */ + public List<Node> makeReadyChildren(int count, int startIndex, NodeResources resources, String parentHostname, + Function<Integer, String> nodeNamer) { + return makeReadyChildren(count, startIndex, resources, NodeType.tenant, parentHostname, nodeNamer); + } + public void activateTenantHosts() { prepareAndActivateInfraApplication(applicationId(), NodeType.host); } diff --git a/searchcore/src/vespa/searchcore/config/proton.def b/searchcore/src/vespa/searchcore/config/proton.def index a634ccd0a78..9ae15bd919e 100644 --- a/searchcore/src/vespa/searchcore/config/proton.def +++ b/searchcore/src/vespa/searchcore/config/proton.def @@ -363,7 +363,7 @@ lidspacecompaction.interval double default=10.0 ## ## When considering compaction the lid bloat is calculated as (docIdLimit - numDocs). ## The lid bloat must be >= allowedlidbloat before considering compaction. -lidspacecompaction.allowedlidbloat int default=1000 +lidspacecompaction.allowedlidbloat int default=1 ## The allowed lid bloat factor (relative) before considering lid space compaction. ## diff --git a/searchlib/src/tests/attribute/postinglistattribute/postinglistattribute_test.cpp b/searchlib/src/tests/attribute/postinglistattribute/postinglistattribute_test.cpp index 46157385f44..bde75edcd03 100644 --- a/searchlib/src/tests/attribute/postinglistattribute/postinglistattribute_test.cpp +++ b/searchlib/src/tests/attribute/postinglistattribute/postinglistattribute_test.cpp @@ -453,8 +453,7 @@ PostingListAttributeTest::checkPostingList(const VectorType & vec, const std::ve const uint32_t docBegin = range.getBegin(i); const uint32_t docEnd = range.getEnd(i); - auto itr = dict.find(typename VectorType::EnumIndex(), - enumStore.make_comparator(values[i])); + auto itr = dict.find(enumstore::Index(), enumStore.make_comparator(values[i])); ASSERT_TRUE(itr.valid()); typename VectorType::PostingList::Iterator postings; diff --git a/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp b/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp index a4b218e73e0..a3a43e09ae5 100644 --- a/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp +++ b/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp @@ -15,47 +15,24 @@ #include <vespa/log/log.h> LOG_SETUP("stringattribute_test"); -namespace search { - -using attribute::CollectionType; -using attribute::IAttributeVector; +using search::attribute::CollectionType; +using search::attribute::IAttributeVector; using vespalib::datastore::EntryRef; +using namespace search; -class StringAttributeTest : public vespalib::TestApp -{ -private: - typedef ArrayStringAttribute ArrayStr; - typedef WeightedSetStringAttribute WeightedSetStr; - typedef ArrayStringPostingAttribute ArrayStrPosting; - typedef WeightedSetStringPostingAttribute WeightedSetStrPosting; - typedef attribute::Config Config; - typedef attribute::BasicType BasicType; - - template <typename Attribute> - void addDocs(Attribute & vec, uint32_t numDocs); - template <typename Attribute> - void checkCount(Attribute & vec, uint32_t doc, uint32_t valueCount, - uint32_t numValues, const vespalib::string & value); - void testMultiValue(); - template <typename Attribute> - void testMultiValue(Attribute & attr, uint32_t numDocs); - void testMultiValueMultipleClearDocBetweenCommit(); - void testMultiValueRemove(); - void testSingleValue(); - void testDefaultValueOnAddDoc(AttributeVector & v); - template <typename Attribute> - void testSingleValue(Attribute & svsa, Config &cfg); - -public: - int Main() override; -}; +typedef ArrayStringAttribute ArrayStr; +typedef WeightedSetStringAttribute WeightedSetStr; +typedef ArrayStringPostingAttribute ArrayStrPosting; +typedef WeightedSetStringPostingAttribute WeightedSetStrPosting; +typedef attribute::Config Config; +typedef attribute::BasicType BasicType; template <typename Attribute> void -StringAttributeTest::addDocs(Attribute & vec, uint32_t numDocs) +addDocs(Attribute & vec, uint32_t numDocs) { for (uint32_t i = 0; i < numDocs; ++i) { - typename Attribute::DocId doc; + IAttributeVector::DocId doc; EXPECT_TRUE(vec.addDoc(doc)); EXPECT_TRUE(doc == i); EXPECT_TRUE(vec.getNumDocs() == i + 1); @@ -66,7 +43,7 @@ StringAttributeTest::addDocs(Attribute & vec, uint32_t numDocs) template <typename Attribute> void -StringAttributeTest::checkCount(Attribute & vec, uint32_t doc, uint32_t valueCount, +checkCount(Attribute & vec, uint32_t doc, uint32_t valueCount, uint32_t numValues, const vespalib::string & value) { std::vector<vespalib::string> buffer(valueCount); @@ -75,36 +52,6 @@ StringAttributeTest::checkCount(Attribute & vec, uint32_t doc, uint32_t valueCou EXPECT_TRUE(std::count(buffer.begin(), buffer.end(), value) == numValues); } - -void -StringAttributeTest::testMultiValue() -{ - uint32_t numDocs = 16; - - { // Array String Attribute - ArrayStr attr("a-string"); - testMultiValue(attr, numDocs); - } - { // Weighted Set String Attribute - WeightedSetStr attr("ws-string", - Config(BasicType::STRING, CollectionType::WSET)); - testMultiValue(attr, numDocs); - } - { // Array String Posting Attribute - Config cfg(BasicType::STRING, CollectionType::ARRAY); - cfg.setFastSearch(true); - ArrayStrPosting attr("a-fs-string", cfg); - testMultiValue(attr, numDocs); - } - { // Weighted Set String Posting Attribute - Config cfg(BasicType::STRING, CollectionType::WSET); - cfg.setFastSearch(true); - WeightedSetStrPosting attr("ws-fs-string", cfg); - testMultiValue(attr, numDocs); - } - -} - namespace { template <typename T0, typename T1> @@ -124,7 +71,7 @@ auto zipped_and_sorted_by_first(const std::vector<T0>& a, const std::vector<T1>& template <typename Attribute> void -StringAttributeTest::testMultiValue(Attribute & attr, uint32_t numDocs) +testMultiValue(Attribute & attr, uint32_t numDocs) { EXPECT_TRUE(attr.getNumDocs() == 0); @@ -194,7 +141,7 @@ StringAttributeTest::testMultiValue(Attribute & attr, uint32_t numDocs) // check for correct refcounts for (uint32_t i = 0; i < uniqueStrings.size(); ++i) { - typename Attribute::EnumStore::Index idx; + enumstore::Index idx; EXPECT_TRUE(attr.getEnumStore().find_index(uniqueStrings[i].c_str(), idx)); uint32_t expectedUsers = numDocs - 1 - i; EXPECT_EQUAL(expectedUsers, attr.getEnumStore().get_ref_count(idx)); @@ -242,15 +189,41 @@ StringAttributeTest::testMultiValue(Attribute & attr, uint32_t numDocs) // check for correct refcounts for (uint32_t i = 0; i < newUniques.size(); ++i) { - typename Attribute::EnumStore::Index idx; + enumstore::Index idx; EXPECT_TRUE(attr.getEnumStore().find_index(newUniques[i].c_str(), idx)); uint32_t expectedUsers = numDocs - 1 - i; EXPECT_EQUAL(expectedUsers, attr.getEnumStore().get_ref_count(idx)); } } -void -StringAttributeTest::testMultiValueMultipleClearDocBetweenCommit() +TEST("testMultiValue") +{ + uint32_t numDocs = 16; + + { // Array String Attribute + ArrayStr attr("a-string"); + testMultiValue(attr, numDocs); + } + { // Weighted Set String Attribute + WeightedSetStr attr("ws-string", + Config(BasicType::STRING, CollectionType::WSET)); + testMultiValue(attr, numDocs); + } + { // Array String Posting Attribute + Config cfg(BasicType::STRING, CollectionType::ARRAY); + cfg.setFastSearch(true); + ArrayStrPosting attr("a-fs-string", cfg); + testMultiValue(attr, numDocs); + } + { // Weighted Set String Posting Attribute + Config cfg(BasicType::STRING, CollectionType::WSET); + cfg.setFastSearch(true); + WeightedSetStrPosting attr("ws-fs-string", cfg); + testMultiValue(attr, numDocs); + } +} + +TEST("testMultiValueMultipleClearDocBetweenCommit") { // This is also tested for all array attributes in attribute unit test ArrayStr mvsa("a-string"); @@ -276,8 +249,7 @@ StringAttributeTest::testMultiValueMultipleClearDocBetweenCommit() } -void -StringAttributeTest::testMultiValueRemove() +TEST("testMultiValueRemove") { // This is also tested for all array attributes in attribute unit test ArrayStr mvsa("a-string"); @@ -320,30 +292,7 @@ StringAttributeTest::testMultiValueRemove() } void -StringAttributeTest::testSingleValue() -{ - { - Config cfg(BasicType::STRING, CollectionType::SINGLE); - SingleValueStringAttribute svsa("svsa", cfg); - const IAttributeVector * ia = &svsa; - EXPECT_TRUE(dynamic_cast<const SingleValueEnumAttributeBase *>(ia) != nullptr); - testSingleValue(svsa, cfg); - - SingleValueStringAttribute svsb("svsa", cfg); - testDefaultValueOnAddDoc(svsb); - } - { - Config cfg(BasicType::STRING, CollectionType::SINGLE); - cfg.setFastSearch(true); - SingleValueStringPostingAttribute svsa("svspb", cfg); - testSingleValue(svsa, cfg); - - SingleValueStringPostingAttribute svsb("svspb", cfg); - testDefaultValueOnAddDoc(svsb); - } -} - -void StringAttributeTest::testDefaultValueOnAddDoc(AttributeVector & v) +testDefaultValueOnAddDoc(AttributeVector & v) { EXPECT_EQUAL(0u, v.getNumDocs()); v.addReservedDoc(); @@ -359,7 +308,7 @@ void StringAttributeTest::testDefaultValueOnAddDoc(AttributeVector & v) template <typename Attribute> void -StringAttributeTest::testSingleValue(Attribute & svsa, Config &cfg) +testSingleValue(Attribute & svsa, Config &cfg) { StringAttribute & v = svsa; const char * t = "not defined"; @@ -434,21 +383,29 @@ StringAttributeTest::testSingleValue(Attribute & svsa, Config &cfg) load.load(); } - - -int -StringAttributeTest::Main() +TEST("testSingleValue") { - TEST_INIT("stringattribute_test"); - - testMultiValue(); - testMultiValueMultipleClearDocBetweenCommit(); - testMultiValueRemove(); - testSingleValue(); + EXPECT_EQUAL(24u, sizeof(AttributeVector::SearchContext)); + EXPECT_EQUAL(80u, sizeof(SingleValueStringAttribute::StringSingleImplSearchContext)); + { + Config cfg(BasicType::STRING, CollectionType::SINGLE); + SingleValueStringAttribute svsa("svsa", cfg); + const IAttributeVector * ia = &svsa; + EXPECT_TRUE(dynamic_cast<const SingleValueEnumAttributeBase *>(ia) != nullptr); + testSingleValue(svsa, cfg); - TEST_DONE(); -} + SingleValueStringAttribute svsb("svsa", cfg); + testDefaultValueOnAddDoc(svsb); + } + { + Config cfg(BasicType::STRING, CollectionType::SINGLE); + cfg.setFastSearch(true); + SingleValueStringPostingAttribute svsa("svspb", cfg); + testSingleValue(svsa, cfg); + SingleValueStringPostingAttribute svsb("svspb", cfg); + testDefaultValueOnAddDoc(svsb); + } } -TEST_APPHOOK(search::StringAttributeTest); +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/attribute/enumattribute.h b/searchlib/src/vespa/searchlib/attribute/enumattribute.h index 82a0df6e157..552528894fa 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/enumattribute.h @@ -56,9 +56,6 @@ protected: EnumStore _enumStore; - EnumStore & getEnumStore() { return _enumStore; } - const EnumStore & getEnumStore() const { return _enumStore; } - const IEnumStore* getEnumStoreBase() const override { return &_enumStore; } IEnumStore* getEnumStoreBase() override { return &_enumStore; } EnumEntryType getFromEnum(EnumHandle e) const override { return _enumStore.get_value(e); } @@ -81,6 +78,9 @@ public: EnumAttribute(const vespalib::string & baseFileName, const AttributeVector::Config & cfg); ~EnumAttribute(); bool findEnum(EnumEntryType v, EnumHandle & e) const override { return _enumStore.find_enum(v, e); } + const EnumStore & getEnumStore() const { return _enumStore; } + EnumStore & getEnumStore() { return _enumStore; } + }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/multistringattribute.h b/searchlib/src/vespa/searchlib/attribute/multistringattribute.h index 781fe106d87..ce1c36681f8 100644 --- a/searchlib/src/vespa/searchlib/attribute/multistringattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/multistringattribute.h @@ -33,13 +33,11 @@ protected: using QueryTermSimpleUP = AttributeVector::QueryTermSimpleUP; using WeightedIndex = typename MultiValueAttribute<B, M>::MultiValueType; using WeightedIndexArrayRef = typename MultiValueAttribute<B, M>::MultiValueArrayRef; - using WeightedIndexVector = typename MultiValueAttribute<B, M>::ValueVector; using Change = StringAttribute::Change; using ChangeVector = StringAttribute::ChangeVector; using DocId = StringAttribute::DocId; using EnumHandle = StringAttribute::EnumHandle; - using EnumModifier = StringAttribute::EnumModifier; using LoadedVector = StringAttribute::LoadedVector; using SearchContext = StringAttribute::SearchContext; using ValueModifier = StringAttribute::ValueModifier; @@ -48,9 +46,6 @@ protected: using WeightedString = StringAttribute::WeightedString; using generation_t = StringAttribute::generation_t; -private: - friend class StringAttributeTest; - public: MultiValueStringAttributeT(const vespalib::string & name, const AttributeVector::Config & c = AttributeVector::Config(AttributeVector::BasicType::STRING, @@ -159,7 +154,6 @@ public: { using BT::queryTerm; using AttrType = MultiValueStringAttributeT<B, M>; - using FoldedComparatorType = typename EnumStore::FoldedComparatorType; public: StringTemplSearchContext(SearchContext::QueryTermSimpleUP qTerm, const AttrType & toBeSearched); }; diff --git a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.h b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.h index 142879f4578..c324ecdf125 100644 --- a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.h @@ -41,11 +41,6 @@ private: }; DocumentWeightAttributeAdapter _document_weight_attribute_adapter; - friend class PostingListAttributeTest; - template <typename, typename, typename> - friend class attribute::PostingSearchContext; // getEnumStore() - friend class StringAttributeTest; - using LoadedVector = typename B::LoadedVector; using PostingParent = PostingListAttributeSubBase<AttributeWeightPosting, LoadedVector, @@ -53,16 +48,10 @@ private: typename B::EnumStore>; using ComparatorType = typename EnumStore::ComparatorType; - using Dictionary = EnumPostingTree; - using DictionaryConstIterator = typename Dictionary::ConstIterator; using DocId = typename MultiValueStringAttributeT<B, T>::DocId; using DocIndices = typename MultiValueStringAttributeT<B, T>::DocIndices; - using EnumIndex = typename EnumStore::Index; - using FoldedComparatorType = typename EnumStore::FoldedComparatorType; - using FrozenDictionary = typename Dictionary::FrozenView; using LoadedEnumAttributeVector = attribute::LoadedEnumAttributeVector; using Posting = typename PostingParent::Posting; - using PostingList = typename PostingParent::PostingList; using PostingMap = typename PostingParent::PostingMap; using QueryTermSimpleUP = AttributeVector::QueryTermSimpleUP; using SelfType = MultiValueStringPostingAttributeT<B, T>; @@ -84,6 +73,10 @@ private: void applyValueChanges(const DocIndices& docIndices, EnumStoreBatchUpdater& updater) override ; public: + using PostingParent::getPostingList; + using Dictionary = EnumPostingTree; + using PostingList = typename PostingParent::PostingList; + MultiValueStringPostingAttributeT(const vespalib::string & name, const AttributeVector::Config & c = AttributeVector::Config(AttributeVector::BasicType::STRING, attribute::CollectionType::ARRAY)); diff --git a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp index 4263eacfa52..25d7858ea81 100644 --- a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp @@ -111,11 +111,11 @@ IDocumentWeightAttribute::LookupResult MultiValueStringPostingAttributeT<B, T>::DocumentWeightAttributeAdapter::lookup(const vespalib::string &term, vespalib::datastore::EntryRef dictionary_snapshot) const { const Dictionary &dictionary = self._enumStore.get_posting_dictionary(); - DictionaryConstIterator dictItr(vespalib::btree::BTreeNode::Ref(), dictionary.getAllocator()); + Dictionary::ConstIterator dictItr(vespalib::btree::BTreeNode::Ref(), dictionary.getAllocator()); auto comp = self._enumStore.make_folded_comparator(term.c_str()); - dictItr.lower_bound(dictionary_snapshot, EnumIndex(), comp); - if (dictItr.valid() && !comp(EnumIndex(), dictItr.getKey())) { + dictItr.lower_bound(dictionary_snapshot, enumstore::Index(), comp); + if (dictItr.valid() && !comp(enumstore::Index(), dictItr.getKey())) { vespalib::datastore::EntryRef pidx(dictItr.getData()); if (pidx.valid()) { const PostingList &plist = self.getPostingList(); @@ -131,7 +131,7 @@ void MultiValueStringPostingAttributeT<B, T>::DocumentWeightAttributeAdapter::collect_folded(vespalib::datastore::EntryRef enum_idx, vespalib::datastore::EntryRef dictionary_snapshot, const std::function<void(vespalib::datastore::EntryRef)>& callback) const { const Dictionary &dictionary = self._enumStore.get_posting_dictionary(); - DictionaryConstIterator dictItr(vespalib::btree::BTreeNode::Ref(), dictionary.getAllocator()); + Dictionary::ConstIterator dictItr(vespalib::btree::BTreeNode::Ref(), dictionary.getAllocator()); auto comp = self._enumStore.make_folded_comparator(); dictItr.lower_bound(dictionary_snapshot, enum_idx, comp); while (dictItr.valid() && !comp(enum_idx, dictItr.getKey())) { diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp index ed764dd9ffc..f5b4accfedc 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp @@ -17,8 +17,7 @@ PostingListAttributeBase(AttributeVector &attr, _postingList(enumStore.get_dictionary().get_posting_dictionary(), attr.getStatus(), attr.getConfig()), _attr(attr), - _dict(enumStore.get_dictionary().get_posting_dictionary()), - _esb(enumStore) + _dict(enumStore.get_dictionary().get_posting_dictionary()) { } template <typename P> diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.h b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.h index 0d4a4982793..5f2eb02ecd2 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.h @@ -50,7 +50,6 @@ protected: PostingList _postingList; AttributeVector &_attr; EnumPostingTree &_dict; - IEnumStore &_esb; PostingListAttributeBase(AttributeVector &attr, IEnumStore &enumStore); virtual ~PostingListAttributeBase(); diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp index 0dc063a5165..972baa267ce 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp @@ -15,7 +15,6 @@ PostingListSearchContext(const Dictionary &dictionary, uint32_t docIdLimit, uint64_t numValues, bool hasWeight, - const IEnumStore &esb, uint32_t minBvDocFreq, bool useBitVector, const ISearchContext &baseSearchCtx) @@ -32,7 +31,6 @@ PostingListSearchContext(const Dictionary &dictionary, _frozenRoot(), _FSTC(0.0), _PLSTC(0.0), - _esb(esb), _minBvDocFreq(minBvDocFreq), _gbv(nullptr), _baseSearchCtx(baseSearchCtx) diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h index 8cd7a7064f6..d8fcfaff958 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h @@ -42,14 +42,13 @@ protected: vespalib::datastore::EntryRef _frozenRoot; // Posting list in tree form float _FSTC; // Filtering Search Time Constant float _PLSTC; // Posting List Search Time Constant - const IEnumStore &_esb; uint32_t _minBvDocFreq; const GrowableBitVector *_gbv; // bitvector if _useBitVector has been set const ISearchContext &_baseSearchCtx; PostingListSearchContext(const Dictionary &dictionary, uint32_t docIdLimit, uint64_t numValues, bool hasWeight, - const IEnumStore &esb, uint32_t minBvDocFreq, bool useBitVector, const ISearchContext &baseSearchCtx); + uint32_t minBvDocFreq, bool useBitVector, const ISearchContext &baseSearchCtx); ~PostingListSearchContext(); @@ -98,7 +97,6 @@ protected: using Traits = PostingListTraits<DataType>; using PostingList = typename Traits::PostingList; using Posting = typename Traits::Posting; - using PostingVector = std::vector<Posting>; using EntryRef = vespalib::datastore::EntryRef; using FrozenView = typename PostingList::BTreeType::FrozenView; @@ -113,8 +111,8 @@ protected: static const long MIN_APPROXHITS_TO_NUMDOCS_RATIO_BEFORE_APPROXIMATION = 10; PostingListSearchContextT(const Dictionary &dictionary, uint32_t docIdLimit, uint64_t numValues, - bool hasWeight, const PostingList &postingList, const IEnumStore &esb, - uint32_t minBvCocFreq, bool useBitVector, const ISearchContext &baseSearchCtx); + bool hasWeight, const PostingList &postingList, uint32_t minBvDocFreq, + bool useBitVector, const ISearchContext &baseSearchCtx); ~PostingListSearchContextT() override; void lookupSingle(); @@ -151,8 +149,8 @@ protected: using Parent::singleHits; PostingListFoldedSearchContextT(const Dictionary &dictionary, uint32_t docIdLimit, uint64_t numValues, - bool hasWeight, const PostingList &postingList, const IEnumStore &esb, - uint32_t minBvCocFreq, bool useBitVector, const ISearchContext &baseSearchCtx); + bool hasWeight, const PostingList &postingList, uint32_t minBvDocFreq, + bool useBitVector, const ISearchContext &baseSearchCtx); unsigned int approximateHits() const override; }; @@ -181,7 +179,6 @@ private: using AggregationTraits = PostingListTraits<DataT>; using PostingList = typename AggregationTraits::PostingList; using Parent = PostingSearchContext<BaseSC, PostingListFoldedSearchContextT<DataT>, AttrT>; - using FoldedComparatorType = typename Parent::EnumStore::FoldedComparatorType; using RegexpUtil = vespalib::RegexpUtil; using QueryTermSimpleUP = typename Parent::QueryTermSimpleUP; using Parent::_toBeSearched; @@ -189,7 +186,7 @@ private: using Parent::isRegex; using Parent::getRegex; bool useThis(const PostingListSearchContext::DictionaryConstIterator & it) const override { - return isRegex() ? (getRegex() ? getRegex()->partial_match(_enumStore.get_value(it.getKey())) : false ) : true; + return isRegex() ? (getRegex().valid() ? getRegex().partial_match(_enumStore.get_value(it.getKey())) : false ) : true; } public: StringPostingSearchContext(QueryTermSimpleUP qTerm, bool useBitVector, const AttrT &toBeSearched); @@ -255,8 +252,7 @@ PostingSearchContext(QueryTermSimpleUP qTerm, bool useBitVector, const AttrT &to toBeSearched.getStatus().getNumValues(), toBeSearched.hasWeightedSetType(), toBeSearched.getPostingList(), - toBeSearched.getEnumStore(), - toBeSearched._postingList._minBvDocFreq, + toBeSearched.getPostingList()._minBvDocFreq, useBitVector, *this), _toBeSearched(toBeSearched), diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp index 09e5a9da5bc..072b52ff47a 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp @@ -21,9 +21,9 @@ namespace search::attribute { template <typename DataT> PostingListSearchContextT<DataT>:: PostingListSearchContextT(const Dictionary &dictionary, uint32_t docIdLimit, uint64_t numValues, bool hasWeight, - const PostingList &postingList, const IEnumStore &esb, - uint32_t minBvDocFreq, bool useBitVector, const ISearchContext &searchContext) - : PostingListSearchContext(dictionary, docIdLimit, numValues, hasWeight, esb, minBvDocFreq, useBitVector, searchContext), + const PostingList &postingList, uint32_t minBvDocFreq, + bool useBitVector, const ISearchContext &searchContext) + : PostingListSearchContext(dictionary, docIdLimit, numValues, hasWeight, minBvDocFreq, useBitVector, searchContext), _postingList(postingList), _merger(docIdLimit) { @@ -283,9 +283,9 @@ PostingListSearchContextT<DataT>::applyRangeLimit(int rangeLimit) template <typename DataT> PostingListFoldedSearchContextT<DataT>:: PostingListFoldedSearchContextT(const Dictionary &dictionary, uint32_t docIdLimit, uint64_t numValues, - bool hasWeight, const PostingList &postingList, const IEnumStore &esb, - uint32_t minBvDocFreq, bool useBitVector, const ISearchContext &searchContext) - : Parent(dictionary, docIdLimit, numValues, hasWeight, postingList, esb, minBvDocFreq, useBitVector, searchContext) + bool hasWeight, const PostingList &postingList, uint32_t minBvDocFreq, + bool useBitVector, const ISearchContext &searchContext) + : Parent(dictionary, docIdLimit, numValues, hasWeight, postingList, minBvDocFreq, useBitVector, searchContext) { } diff --git a/searchlib/src/vespa/searchlib/attribute/singlestringattribute.h b/searchlib/src/vespa/searchlib/attribute/singlestringattribute.h index 5d0c636dc91..11d4911bf09 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlestringattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/singlestringattribute.h @@ -25,7 +25,6 @@ protected: using EnumHintSearchContext = attribute::EnumHintSearchContext; using EnumIndex = typename SingleValueEnumAttributeBase::EnumIndex; using EnumIndexVector = typename SingleValueEnumAttributeBase::EnumIndexVector; - using EnumModifier = StringAttribute::EnumModifier; using EnumStore = typename SingleValueEnumAttribute<B>::EnumStore; using LoadedVector = StringAttribute::LoadedVector; using QueryTermSimpleUP = AttributeVector::QueryTermSimpleUP; @@ -107,7 +106,6 @@ public: public EnumHintSearchContext { using AttrType = SingleValueStringAttributeT<B>; - using FoldedComparatorType = typename EnumStore::FoldedComparatorType; using StringSingleImplSearchContext::queryTerm; public: StringTemplSearchContext(QueryTermSimpleUP qTerm, const AttrType & toBeSearched); diff --git a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.h b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.h index 3ac49ab6921..60a4bc022be 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.h @@ -26,11 +26,6 @@ public: using EnumStoreBatchUpdater = typename EnumStore::BatchUpdater; private: - friend class PostingListAttributeTest; - template <typename, typename, typename> - friend class attribute::PostingSearchContext; // getEnumStore() - friend class StringAttributeTest; - using LoadedVector = typename B::LoadedVector; using PostingParent = PostingListAttributeSubBase<AttributePosting, LoadedVector, @@ -40,12 +35,9 @@ private: using Change = StringAttribute::Change; using ChangeVector = StringAttribute::ChangeVector; using ComparatorType = typename EnumStore::ComparatorType; - using Dictionary = EnumPostingTree; using DocId = typename SingleValueStringAttributeT<B>::DocId; using EnumIndex = typename SingleValueStringAttributeT<B>::EnumIndex; - using FoldedComparatorType = typename EnumStore::FoldedComparatorType; using LoadedEnumAttributeVector = attribute::LoadedEnumAttributeVector; - using PostingList = typename PostingParent::PostingList; using PostingMap = typename PostingParent::PostingMap; using QueryTermSimpleUP = AttributeVector::QueryTermSimpleUP; using SelfType = SingleValueStringPostingAttributeT<B>; @@ -62,6 +54,8 @@ private: using PostingParent::handle_load_posting_lists_and_update_enum_store; using PostingParent::forwardedOnAddDoc; public: + using PostingList = typename PostingParent::PostingList; + using Dictionary = EnumPostingTree; using PostingParent::getPostingList; private: diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp index f113b99357f..78225a59b66 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp @@ -21,10 +21,9 @@ using attribute::LoadedEnumAttribute; using attribute::LoadedEnumAttributeVector; AttributeVector::SearchContext::UP -StringAttribute::getSearch(QueryTermSimple::UP term, const attribute::SearchContextParams & params) const +StringAttribute::getSearch(QueryTermSimple::UP term, const attribute::SearchContextParams &) const { - (void) params; - return SearchContext::UP(new StringSearchContext(std::move(term), *this)); + return std::make_unique<StringSearchContext>(std::move(term), *this); } class SortDataChar { @@ -94,7 +93,8 @@ public: } }; -size_t StringAttribute::countZero(const char * bt, size_t sz) +size_t +StringAttribute::countZero(const char * bt, size_t sz) { size_t size(0); for(size_t i(0); i < sz; i++) { @@ -105,7 +105,8 @@ size_t StringAttribute::countZero(const char * bt, size_t sz) return size; } -void StringAttribute::generateOffsets(const char * bt, size_t sz, OffsetVector & offsets) +void +StringAttribute::generateOffsets(const char * bt, size_t sz, OffsetVector & offsets) { offsets.clear(); uint32_t start(0); @@ -131,25 +132,27 @@ StringAttribute::StringAttribute(const vespalib::string & name, const Config & c { } -StringAttribute::~StringAttribute() {} +StringAttribute::~StringAttribute() = default; -uint32_t StringAttribute::get(DocId doc, WeightedInt * v, uint32_t sz) const +uint32_t +StringAttribute::get(DocId doc, WeightedInt * v, uint32_t sz) const { WeightedConstChar * s = new WeightedConstChar[sz]; uint32_t n = static_cast<const AttributeVector *>(this)->get(doc, s, sz); for(uint32_t i(0),m(std::min(n,sz)); i<m; i++) { - v[i] = WeightedInt(strtoll(s[i].getValue(), NULL, 0), s[i].getWeight()); + v[i] = WeightedInt(strtoll(s[i].getValue(), nullptr, 0), s[i].getWeight()); } delete [] s; return n; } -uint32_t StringAttribute::get(DocId doc, WeightedFloat * v, uint32_t sz) const +uint32_t +StringAttribute::get(DocId doc, WeightedFloat * v, uint32_t sz) const { WeightedConstChar * s = new WeightedConstChar[sz]; uint32_t n = static_cast<const AttributeVector *>(this)->get(doc, s, sz); for(uint32_t i(0),m(std::min(n,sz)); i<m; i++) { - v[i] = WeightedFloat(vespalib::locale::c::strtod(s[i].getValue(), NULL), s[i].getWeight()); + v[i] = WeightedFloat(vespalib::locale::c::strtod(s[i].getValue(), nullptr), s[i].getWeight()); } delete [] s; return n; @@ -157,32 +160,35 @@ uint32_t StringAttribute::get(DocId doc, WeightedFloat * v, uint32_t sz) const double StringAttribute::getFloat(DocId doc) const { - return vespalib::locale::c::strtod(get(doc), NULL); + return vespalib::locale::c::strtod(get(doc), nullptr); } -uint32_t StringAttribute::get(DocId doc, double * v, uint32_t sz) const +uint32_t +StringAttribute::get(DocId doc, double * v, uint32_t sz) const { const char ** s = new const char *[sz]; uint32_t n = static_cast<const AttributeVector *>(this)->get(doc, s, sz); for(uint32_t i(0),m(std::min(n,sz)); i<m; i++) { - v[i] = vespalib::locale::c::strtod(s[i], NULL); + v[i] = vespalib::locale::c::strtod(s[i], nullptr); } delete [] s; return n; } -uint32_t StringAttribute::get(DocId doc, largeint_t * v, uint32_t sz) const +uint32_t +StringAttribute::get(DocId doc, largeint_t * v, uint32_t sz) const { const char ** s = new const char *[sz]; uint32_t n = static_cast<const AttributeVector *>(this)->get(doc, s, sz); for(uint32_t i(0),m(std::min(n,sz)); i<m; i++) { - v[i] = strtoll(s[i], NULL, 0); + v[i] = strtoll(s[i], nullptr, 0); } delete [] s; return n; } -long StringAttribute::onSerializeForAscendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const +long +StringAttribute::onSerializeForAscendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const { unsigned char *dst = static_cast<unsigned char *>(serTo); const char *value(get(doc)); @@ -199,7 +205,8 @@ long StringAttribute::onSerializeForAscendingSort(DocId doc, void * serTo, long return buf.size(); } -long StringAttribute::onSerializeForDescendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const +long +StringAttribute::onSerializeForDescendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const { (void) bc; unsigned char *dst = static_cast<unsigned char *>(serTo); @@ -223,13 +230,13 @@ long StringAttribute::onSerializeForDescendingSort(DocId doc, void * serTo, long StringAttribute::StringSearchContext::StringSearchContext(QueryTermSimple::UP qTerm, const StringAttribute & toBeSearched) : SearchContext(toBeSearched), - _isPrefix(qTerm->isPrefix()), - _isRegex(qTerm->isRegex()), - _queryTerm(std::move(qTerm)), + _queryTerm(static_cast<QueryTermUCS4 *>(qTerm.release())), _termUCS4(queryTerm()->getUCS4Term()), - _bufferLen(toBeSearched.getMaxValueCount()), _buffer(nullptr), - _regex() + _regex(), + _bufferLen(toBeSearched.getMaxValueCount()), + _isPrefix(_queryTerm->isPrefix()), + _isRegex(_queryTerm->isRegex()) { if (isRegex()) { _regex = vespalib::Regex::from_pattern(_queryTerm->getTerm(), vespalib::Regex::Options::IgnoreCase); @@ -246,16 +253,17 @@ StringAttribute::StringSearchContext::~StringSearchContext() bool StringAttribute::StringSearchContext::valid() const { - return (_queryTerm.get() && (!_queryTerm->empty())); + return (_queryTerm && (!_queryTerm->empty())); } const QueryTermUCS4 * StringAttribute::StringSearchContext::queryTerm() const { - return static_cast<const QueryTermUCS4 *>(_queryTerm.get()); + return _queryTerm.get(); } -uint32_t StringAttribute::clearDoc(DocId doc) +uint32_t +StringAttribute::clearDoc(DocId doc) { uint32_t removed(0); if (hasMultiValue() && (doc < getNumDocs())) { @@ -303,19 +311,22 @@ StringAttribute::StringSearchContext::onFind(DocId docId, int32_t elemId) const return -1; } -bool StringAttribute::applyWeight(DocId doc, const FieldValue & fv, const ArithmeticValueUpdate & wAdjust) +bool +StringAttribute::applyWeight(DocId doc, const FieldValue & fv, const ArithmeticValueUpdate & wAdjust) { vespalib::string v = fv.getAsString(); return AttributeVector::adjustWeight(_changes, doc, StringChangeData(v), wAdjust); } -bool StringAttribute::applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) +bool +StringAttribute::applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) { vespalib::string v = fv.getAsString(); return AttributeVector::adjustWeight(_changes, doc, StringChangeData(v), wAdjust); } -bool StringAttribute::apply(DocId, const ArithmeticValueUpdate & ) +bool +StringAttribute::apply(DocId, const ArithmeticValueUpdate & ) { return false; } @@ -358,7 +369,8 @@ StringAttribute::onLoadEnumerated(ReaderBase &attrReader) return true; } -bool StringAttribute::onLoad() +bool +StringAttribute::onLoad() { ReaderBase attrReader(*this); bool ok(attrReader.getHasLoadData()); @@ -379,15 +391,18 @@ StringAttribute::onAddDoc(DocId ) return false; } -void StringAttribute::load_posting_lists(LoadedVector&) +void +StringAttribute::load_posting_lists(LoadedVector&) { } -void StringAttribute::load_enum_store(LoadedVector&) +void +StringAttribute::load_enum_store(LoadedVector&) { } -void StringAttribute::fillValues(LoadedVector & ) +void +StringAttribute::fillValues(LoadedVector & ) { } diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.h b/searchlib/src/vespa/searchlib/attribute/stringbase.h index e6238cc0f94..36c3d113b03 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.h +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.h @@ -60,7 +60,7 @@ public: protected: StringAttribute(const vespalib::string & name); StringAttribute(const vespalib::string & name, const Config & c); - ~StringAttribute(); + ~StringAttribute() override; static const char * defaultValue() { return ""; } using Change = ChangeTemplate<StringChangeData>; using ChangeVector = ChangeVectorT<Change>; @@ -71,9 +71,9 @@ protected: bool onLoadEnumerated(ReaderBase &attrReader); - virtual bool onAddDoc(DocId doc) override; + bool onAddDoc(DocId doc) override; - virtual vespalib::MemoryUsage getChangeVectorMemoryUsage() const override; + vespalib::MemoryUsage getChangeVectorMemoryUsage() const override; private: virtual void load_posting_lists(LoadedVector& loaded); virtual void load_enum_store(LoadedVector& loaded); @@ -83,7 +83,7 @@ private: virtual void load_enumerated_data(ReaderBase &attrReader, enumstore::EnumeratedLoader& loader); virtual void load_posting_lists_and_update_enum_store(enumstore::EnumeratedPostingsLoader& loader); - largeint_t getInt(DocId doc) const override { return strtoll(get(doc), NULL, 0); } + largeint_t getInt(DocId doc) const override { return strtoll(get(doc), nullptr, 0); } double getFloat(DocId doc) const override; const char * getString(DocId doc, char * v, size_t sz) const override { (void) v; (void) sz; return get(doc); } @@ -95,16 +95,13 @@ protected: public: StringSearchContext(QueryTermSimpleUP qTerm, const StringAttribute & toBeSearched); ~StringSearchContext() override; - private: - bool _isPrefix; - bool _isRegex; protected: bool valid() const override; const QueryTermUCS4 * queryTerm() const override; bool isMatch(const char *src) const { if (__builtin_expect(isRegex(), false)) { - return _regex ? _regex->partial_match(std::string_view(src)) : false; + return _regex.valid() ? _regex.partial_match(std::string_view(src)) : false; } vespalib::Utf8ReaderForZTS u8reader(src); uint32_t j = 0; @@ -161,9 +158,7 @@ protected: bool isPrefix() const { return _isPrefix; } bool isRegex() const { return _isRegex; } - QueryTermSimpleUP _queryTerm; - std::vector<ucs4_t> _termUCS4; - const std::optional<vespalib::Regex>& getRegex() const { return _regex; } + const vespalib::Regex & getRegex() const { return _regex; } private: WeightedConstChar * getBuffer() const { if (_buffer == nullptr) { @@ -171,11 +166,14 @@ protected: } return _buffer; } - unsigned _bufferLen; + std::unique_ptr<QueryTermUCS4> _queryTerm; + std::vector<ucs4_t> _termUCS4; mutable WeightedConstChar * _buffer; - std::optional<vespalib::Regex> _regex; + vespalib::Regex _regex; + unsigned _bufferLen; + bool _isPrefix; + bool _isRegex; }; -private: SearchContext::UP getSearch(QueryTermSimpleUP term, const attribute::SearchContextParams & params) const override; }; diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/ConfigServerLikeApplication.java b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/ConfigServerLikeApplication.java index b52b472d52d..91a439649a2 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/ConfigServerLikeApplication.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/ConfigServerLikeApplication.java @@ -21,7 +21,6 @@ public abstract class ConfigServerLikeApplication extends InfraApplication { return ClusterSpec.request(getClusterSpecType(), getClusterSpecId()) .vespaVersion(version) .stateful(true) - .exclusive(true) .build(); } diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/InfraApplication.java b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/InfraApplication.java index d113b34f3c6..da00ebc41e0 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/InfraApplication.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/InfraApplication.java @@ -75,7 +75,7 @@ public abstract class InfraApplication implements InfraApplicationApi { @Override public ClusterSpec getClusterSpecWithVersion(Version version) { - return ClusterSpec.request(clusterSpecType, clusterSpecId).vespaVersion(version).exclusive(true).build(); + return ClusterSpec.request(clusterSpecType, clusterSpecId).vespaVersion(version).build(); } public ClusterSpec.Type getClusterSpecType() { diff --git a/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricAggregator.java b/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricAggregator.java index 7d142a8e4d8..7168eb49676 100644 --- a/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricAggregator.java +++ b/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricAggregator.java @@ -10,7 +10,7 @@ import com.yahoo.metrics.ManagerConfig; /** * Worker thread to collect the data stored in worker threads and build * snapshots for external consumption. Using the correct executor gives the - * necessary guarantuees for this being invoked from only a single thread. + * necessary guarantees for this being invoked from only a single thread. * * @author Steinar Knutsen */ @@ -26,9 +26,8 @@ class MetricAggregator implements Runnable { MetricAggregator(ThreadLocalDirectory<Bucket, Sample> metricsCollection, AtomicReference<Bucket> currentSnapshot, ManagerConfig settings) { if (settings.reportPeriodSeconds() < 10) { - throw new IllegalArgumentException( - "Do not use this metrics implementation" - + " if report periods of less than 10 seconds is desired."); + throw new IllegalArgumentException("Do not use this metrics implementation" + + " if report periods of less than 10 seconds is desired."); } buffer = new Bucket[settings.reportPeriodSeconds()]; dimensions = new DimensionCache(settings.pointsToKeepPerMetric()); diff --git a/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricManager.java b/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricManager.java index a2a7c5b94ba..1956783b4c0 100644 --- a/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricManager.java +++ b/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricManager.java @@ -33,8 +33,8 @@ public class MetricManager extends AbstractComponent implements Provider<MetricR private MetricManager(ManagerConfig settings, Updater<Bucket, Sample> updater) { log.log(Level.CONFIG, "setting up simple metrics gathering." + - " reportPeriodSeconds=" + settings.reportPeriodSeconds() + - ", pointsToKeepPerMetric=" + settings.pointsToKeepPerMetric()); + " reportPeriodSeconds=" + settings.reportPeriodSeconds() + + ", pointsToKeepPerMetric=" + settings.pointsToKeepPerMetric()); metricsCollection = new ThreadLocalDirectory<>(updater); final AtomicReference<Bucket> currentSnapshot = new AtomicReference<>(null); executor = new ScheduledThreadPoolExecutor(1); diff --git a/storage/src/tests/bucketdb/bucketmanagertest.cpp b/storage/src/tests/bucketdb/bucketmanagertest.cpp index e14bb8e11d1..b83ccccbf48 100644 --- a/storage/src/tests/bucketdb/bucketmanagertest.cpp +++ b/storage/src/tests/bucketdb/bucketmanagertest.cpp @@ -556,7 +556,7 @@ class ConcurrentOperationFixture { public: explicit ConcurrentOperationFixture(BucketManagerTest& self) : _self(self), - _state("distributor:1 storage:1") + _state(std::make_shared<lib::ClusterState>("distributor:1 storage:1")) { _self.setupTestEnvironment(); _self._top->open(); @@ -566,9 +566,7 @@ public: // Need a cluster state to work with initially, so that processing // bucket requests can calculate a target distributor. - _self._node->setClusterState(_state); - _self._manager->onDown( - std::make_shared<api::SetSystemStateCommand>(_state)); + updater_internal_cluster_state_with_current(); } void setUp(const WithBuckets& buckets) { @@ -577,6 +575,16 @@ public: } } + void updater_internal_cluster_state_with_current() { + _self._node->setClusterState(*_state); + _self._manager->onDown(std::make_shared<api::SetSystemStateCommand>(*_state)); + } + + void update_cluster_state(const lib::ClusterState& state) { + _state = std::make_shared<lib::ClusterState>(state); + updater_internal_cluster_state_with_current(); + } + auto acquireBucketLock(const document::BucketId& bucket) { return _self._node->getStorageBucketDatabase().get(bucket, "foo"); } @@ -610,15 +618,15 @@ public: } auto createFullFetchCommand() const { - return std::make_shared<api::RequestBucketInfoCommand>(makeBucketSpace(), 0, _state); + return std::make_shared<api::RequestBucketInfoCommand>(makeBucketSpace(), 0, *_state); } auto createFullFetchCommandWithHash(vespalib::stringref hash) const { - return std::make_shared<api::RequestBucketInfoCommand>(makeBucketSpace(), 0, _state, hash); + return std::make_shared<api::RequestBucketInfoCommand>(makeBucketSpace(), 0, *_state, hash); } auto createFullFetchCommandWithHash(document::BucketSpace space, vespalib::stringref hash) const { - return std::make_shared<api::RequestBucketInfoCommand>(space, 0, _state, hash); + return std::make_shared<api::RequestBucketInfoCommand>(space, 0, *_state, hash); } auto acquireBucketLockAndSendInfoRequest(const document::BucketId& bucket) { @@ -726,7 +734,7 @@ group[2].nodes[2].index 5 private: BucketManagerTest& _self; - lib::ClusterState _state; + std::shared_ptr<lib::ClusterState> _state; }; TEST_F(BucketManagerTest, split_reply_ordered_after_bucket_reply) { @@ -1214,4 +1222,25 @@ TEST_F(BucketManagerTest, fall_back_to_legacy_global_distribution_hash_on_mismat EXPECT_EQ(api::ReturnCode::OK, reply.getResult().getResult()); // _not_ REJECTED } +// It's possible for the request processing thread and onSetSystemState (which use +// the same mutex) to race with the actual internal component cluster state switch-over. +// Ensure we detect and handle this by bouncing the request back to the distributor. +// It's for all intents and purposes guaranteed that the internal state has converged +// once the distributor has gotten around to retrying the operation. +TEST_F(BucketManagerTest, bounce_request_on_internal_cluster_state_version_mismatch) { + ConcurrentOperationFixture f(*this); + + // Make manager-internal and component-internal version state inconsistent + f.update_cluster_state(lib::ClusterState("version:2 distributor:1 storage:1")); + _manager->onDown(std::make_shared<api::SetSystemStateCommand>(lib::ClusterState("version:3 distributor:1 storage:1"))); + + // Info command is sent with state version 2, which mismatches that of internal state 3 + // even though it's the same as the component's current version. + _top->sendDown(f.createFullFetchCommand()); + + auto replies = f.awaitAndGetReplies(1); + auto& reply = dynamic_cast<api::RequestBucketInfoReply&>(*replies[0]); + EXPECT_EQ(api::ReturnCode::REJECTED, reply.getResult().getResult()); +} + } // storage diff --git a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp index 2d70ee8d3ba..4680414baa1 100644 --- a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp +++ b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp @@ -411,7 +411,7 @@ void BucketManager::startWorkerThread() bool BucketManager::onRequestBucketInfo( const std::shared_ptr<api::RequestBucketInfoCommand>& cmd) { - LOG(debug, "Got request bucket info command"); + LOG(debug, "Got request bucket info command %s", cmd->toString().c_str()); if (cmd->getBuckets().size() == 0 && cmd->hasSystemState()) { std::lock_guard<std::mutex> guard(_workerLock); @@ -542,9 +542,10 @@ BucketManager::processRequestBucketInfoCommands(document::BucketSpace bucketSpac const auto our_hash = distribution->getNodeGraph().getDistributionConfigHash(); - LOG(debug, "Processing %zu queued request bucket info commands. " + LOG(debug, "Processing %zu queued request bucket info commands for bucket space %s. " "Using cluster state '%s' and distribution hash '%s'", reqs.size(), + bucketSpace.toString().c_str(), clusterState->toString().c_str(), our_hash.c_str()); @@ -555,7 +556,15 @@ BucketManager::processRequestBucketInfoCommands(document::BucketSpace bucketSpac const auto their_hash = (*it)->getDistributionHash(); std::ostringstream error; - if ((*it)->getSystemState().getVersion() > _lastClusterStateSeen) { + if (clusterState->getVersion() != _lastClusterStateSeen) { + // Calling onSetSystemState() on _this_ component and actually switching over + // to another cluster state version does not happen atomically. Detect and + // gracefully deal with the case where we're not internally in sync. + error << "Inconsistent internal cluster state on node during transition; " + << "failing request from distributor " << (*it)->getDistributor() + << " so it can be retried. Node version is " << clusterState->getVersion() + << ", but last version seen by the bucket manager is " << _lastClusterStateSeen; + } else if ((*it)->getSystemState().getVersion() > _lastClusterStateSeen) { error << "Ignoring bucket info request for cluster state version " << (*it)->getSystemState().getVersion() << " as newest " << "version we know of is " << _lastClusterStateSeen; diff --git a/storage/src/vespa/storage/distributor/pendingclusterstate.cpp b/storage/src/vespa/storage/distributor/pendingclusterstate.cpp index c694fda2c1b..175c7d27033 100644 --- a/storage/src/vespa/storage/distributor/pendingclusterstate.cpp +++ b/storage/src/vespa/storage/distributor/pendingclusterstate.cpp @@ -219,7 +219,7 @@ PendingClusterState::requestNode(BucketSpaceAndNode bucketSpaceAndNode) "and distribution hash '%s'", bucketSpaceAndNode.bucketSpace.getId(), bucketSpaceAndNode.node, - getNewClusterStateBundleString().c_str(), + _newClusterStateBundle.getDerivedClusterState(bucketSpaceAndNode.bucketSpace)->toString().c_str(), distributionHash.c_str()); std::shared_ptr<api::RequestBucketInfoCommand> cmd( diff --git a/vespa-http-client/pom.xml b/vespa-http-client/pom.xml index 496b8043abb..19161e720ba 100644 --- a/vespa-http-client/pom.xml +++ b/vespa-http-client/pom.xml @@ -191,6 +191,28 @@ </execution> </executions> </plugin> + <plugin> + <groupId>org.codehaus.mojo</groupId> + <artifactId>build-helper-maven-plugin</artifactId> + <executions> + <execution> + <id>attach-artifacts</id> + <phase>package</phase> + <goals> + <goal>attach-artifact</goal> + </goals> + <configuration> + <artifacts> + <artifact> + <file>target/${project.artifactId}-jar-with-dependencies.jar</file> + <type>jar</type> + <classifier>jar-with-dependencies</classifier> + </artifact> + </artifacts> + </configuration> + </execution> + </executions> + </plugin> </plugins> </build> </project> diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java index e8336e54120..652fea9b332 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java @@ -43,7 +43,8 @@ public abstract class Maintainer implements Runnable { this.ignoreCollision = ignoreCollision; Objects.requireNonNull(startedAt); Objects.requireNonNull(clusterHostnames); - Duration initialDelay = staggeredDelay(interval, startedAt, HostName.getLocalhost(), clusterHostnames); + Duration initialDelay = staggeredDelay(interval, startedAt, HostName.getLocalhost(), clusterHostnames) + .plus(Duration.ofSeconds(30)); // Let the system stabilize before maintenance service = new ScheduledThreadPoolExecutor(1, r -> new Thread(r, name() + "-worker")); service.scheduleAtFixedRate(this, initialDelay.toMillis(), interval.toMillis(), TimeUnit.MILLISECONDS); jobControl.started(name(), this); diff --git a/vespalib/CMakeLists.txt b/vespalib/CMakeLists.txt index 2db3c89dfb5..c51e42176dc 100644 --- a/vespalib/CMakeLists.txt +++ b/vespalib/CMakeLists.txt @@ -130,6 +130,7 @@ vespa_define_module( src/tests/tutorial/simple src/tests/tutorial/threads src/tests/typify + src/tests/util/bfloat16 src/tests/util/generationhandler src/tests/util/generationhandler_stress src/tests/util/md5 diff --git a/vespalib/src/tests/regex/regex.cpp b/vespalib/src/tests/regex/regex.cpp index 7dc5a7f4aa9..0ca877ab006 100644 --- a/vespalib/src/tests/regex/regex.cpp +++ b/vespalib/src/tests/regex/regex.cpp @@ -139,9 +139,15 @@ TEST("can create case-insensitive regex matcher") { TEST("regex is case sensitive by default") { auto re = Regex::from_pattern("hello"); + ASSERT_TRUE(re.valid()); ASSERT_TRUE(re.parsed_ok()); EXPECT_FALSE(re.partial_match("HelLo world")); EXPECT_FALSE(re.full_match("HELLO")); } +TEST("Test that default constructed regex is invalid.") { + Regex dummy; + ASSERT_FALSE(dummy.valid()); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/vespalib/src/tests/util/bfloat16/CMakeLists.txt b/vespalib/src/tests/util/bfloat16/CMakeLists.txt new file mode 100644 index 00000000000..39a42e6f148 --- /dev/null +++ b/vespalib/src/tests/util/bfloat16/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(vespalib_bfloat16_test_app TEST + SOURCES + bfloat16_test.cpp + DEPENDS + vespalib + GTest::GTest +) +vespa_add_test(NAME vespalib_bfloat16_test_app COMMAND vespalib_bfloat16_test_app) diff --git a/vespalib/src/tests/util/bfloat16/bfloat16_test.cpp b/vespalib/src/tests/util/bfloat16/bfloat16_test.cpp new file mode 100644 index 00000000000..4e4129feb78 --- /dev/null +++ b/vespalib/src/tests/util/bfloat16/bfloat16_test.cpp @@ -0,0 +1,156 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/vespalib/util/bfloat16.h> +#include <vespa/vespalib/objects/nbostream.h> +#include <vespa/vespalib/gtest/gtest.h> +#include <stdio.h> +#include <cmath> +#include <cmath> +#include <vector> + +using namespace vespalib; + +using Limits = std::numeric_limits<BFloat16>; + +static std::vector<float> simple_values = { + 0.0, 1.0, -1.0, -0.0, 1.75, 0x1.02p20, -0x1.02p-20, 0x3.0p-100, 0x7.0p100 +}; + +TEST(BFloat16Test, normal_usage) { + EXPECT_EQ(sizeof(float), 4); + EXPECT_EQ(sizeof(BFloat16), 2); + BFloat16 answer = 42; + double fortytwo = answer; + EXPECT_EQ(fortytwo, 42); + std::vector<BFloat16> vec; + for (float value : simple_values) { + BFloat16 b = value; + float recover = b; + EXPECT_EQ(value, recover); + } + BFloat16 b1 = 0x101; + EXPECT_EQ(float(b1), 0x100); + BFloat16 b2 = 0x111; + EXPECT_EQ(float(b2), 0x110); +} + +TEST(BFloat16Test, with_nbostream) { + nbostream buf; + for (BFloat16 value : simple_values) { + buf << value; + } + for (float value : simple_values) { + BFloat16 stored; + buf >> stored; + EXPECT_EQ(float(stored), value); + } +} + +TEST(BFloat16Test, constants_check) { + EXPECT_EQ(0x1.0p-7, (1.0/128.0)); + + float n_min = Limits::min(); + float d_min = Limits::denorm_min(); + float eps = Limits::epsilon(); + float big = Limits::max(); + float low = Limits::lowest(); + + EXPECT_EQ(n_min, 0x1.0p-126); + EXPECT_EQ(d_min, 0x1.0p-133); + EXPECT_EQ(eps, 0x1.0p-7); + EXPECT_EQ(big, 0x1.FEp127); + EXPECT_EQ(low, -big); + + EXPECT_EQ(n_min, std::numeric_limits<float>::min()); + EXPECT_EQ(d_min, n_min / 128.0); + EXPECT_GT(eps, std::numeric_limits<float>::epsilon()); + + BFloat16 try_epsilon = 1.0f + eps; + EXPECT_GT(try_epsilon.to_float(), 1.0f); + BFloat16 try_half_epsilon = 1.0f + (0.5f * eps); + EXPECT_EQ(try_half_epsilon.to_float(), 1.0f); + + EXPECT_LT(big, std::numeric_limits<float>::max()); + EXPECT_GT(low, std::numeric_limits<float>::lowest()); + + printf("bfloat16 epsilon: %.10g (float has %.20g)\n", eps, std::numeric_limits<float>::epsilon()); + printf("bfloat16 norm_min: %.20g (float has %.20g)\n", n_min, std::numeric_limits<float>::min()); + printf("bfloat16 denorm_min: %.20g (float has %.20g)\n", d_min, std::numeric_limits<float>::denorm_min()); + printf("bfloat16 max: %.20g (float has %.20g)\n", big, std::numeric_limits<float>::max()); + printf("bfloat16 lowest: %.20g (float has %.20g)\n", low, std::numeric_limits<float>::lowest()); +} + +TEST(BFloat16Test, traits_check) { + EXPECT_TRUE(std::is_trivially_constructible<BFloat16>::value); + EXPECT_TRUE(std::is_trivially_move_constructible<BFloat16>::value); + EXPECT_TRUE(std::is_trivially_default_constructible<BFloat16>::value); + EXPECT_TRUE((std::is_trivially_assignable<BFloat16,BFloat16>::value)); + EXPECT_TRUE(std::is_trivially_move_assignable<BFloat16>::value); + EXPECT_TRUE(std::is_trivially_copy_assignable<BFloat16>::value); + EXPECT_TRUE(std::is_trivially_copyable<BFloat16>::value); + EXPECT_TRUE(std::is_trivially_destructible<BFloat16>::value); + EXPECT_TRUE(std::is_trivial<BFloat16>::value); + EXPECT_TRUE(std::is_swappable<BFloat16>::value); + EXPECT_TRUE(std::has_unique_object_representations<BFloat16>::value); +} + +static std::string hexdump(const void *p, size_t sz) { + char tmpbuf[10]; + if (sz == 2) { + uint16_t bits; + memcpy(&bits, p, sz); + snprintf(tmpbuf, 10, "%04x", bits); + } else if (sz == 4) { + uint32_t bits; + memcpy(&bits, p, sz); + snprintf(tmpbuf, 10, "%08x", bits); + } else { + abort(); + } + return tmpbuf; +} +#define HEX_DUMP(arg) hexdump(&arg, sizeof(arg)).c_str() + +TEST(BFloat16Test, check_special_values) { + // we should not need to support HW without normal float support: + EXPECT_TRUE(std::numeric_limits<float>::has_quiet_NaN); + EXPECT_TRUE(std::numeric_limits<float>::has_signaling_NaN); + EXPECT_TRUE(std::numeric_limits<BFloat16>::has_quiet_NaN); + EXPECT_TRUE(std::numeric_limits<BFloat16>::has_signaling_NaN); + float f_inf = std::numeric_limits<float>::infinity(); + float f_neg = -f_inf; + float f_qnan = std::numeric_limits<float>::quiet_NaN(); + float f_snan = std::numeric_limits<float>::signaling_NaN(); + BFloat16 b_inf = std::numeric_limits<BFloat16>::infinity(); + BFloat16 b_qnan = std::numeric_limits<BFloat16>::quiet_NaN(); + BFloat16 b_snan = std::numeric_limits<BFloat16>::signaling_NaN(); + BFloat16 b_from_f_inf = f_inf; + BFloat16 b_from_f_neg = f_neg; + BFloat16 b_from_f_qnan = f_qnan; + BFloat16 b_from_f_snan = f_snan; + EXPECT_EQ(memcmp(&b_inf, &b_from_f_inf, sizeof(BFloat16)), 0); + EXPECT_EQ(memcmp(&b_qnan, &b_from_f_qnan, sizeof(BFloat16)), 0); + EXPECT_EQ(memcmp(&b_snan, &b_from_f_snan, sizeof(BFloat16)), 0); + printf("+inf float is '%s' / bf16 is '%s'\n", HEX_DUMP(f_inf), HEX_DUMP(b_from_f_inf)); + printf("-inf float is '%s' / bf16 is '%s'\n", HEX_DUMP(f_neg), HEX_DUMP(b_from_f_neg)); + printf("qNaN float is '%s' / bf16 is '%s'\n", HEX_DUMP(f_qnan), HEX_DUMP(b_from_f_qnan)); + printf("sNan float is '%s' / bf16 is '%s'\n", HEX_DUMP(f_snan), HEX_DUMP(b_from_f_snan)); + double d_inf = b_inf; + double d_neg = b_from_f_neg; + double d_qnan = b_qnan; + double d_snan = b_snan; + EXPECT_EQ(d_inf, std::numeric_limits<double>::infinity()); + EXPECT_EQ(d_neg, -std::numeric_limits<double>::infinity()); + EXPECT_TRUE(std::isnan(d_qnan)); + EXPECT_TRUE(std::isnan(d_snan)); + float f_from_b_inf = b_inf; + float f_from_b_neg = b_from_f_neg; + float f_from_b_qnan = b_qnan; + float f_from_b_snan = b_snan; + EXPECT_EQ(memcmp(&f_inf, &f_from_b_inf, sizeof(float)), 0); + EXPECT_EQ(memcmp(&f_neg, &f_from_b_neg, sizeof(float)), 0); + EXPECT_EQ(memcmp(&f_qnan, &f_from_b_qnan, sizeof(float)), 0); + EXPECT_EQ(memcmp(&f_snan, &f_from_b_snan, sizeof(float)), 0); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/vespa/vespalib/cppunit/.gitignore b/vespalib/src/vespa/vespalib/cppunit/.gitignore deleted file mode 100644 index 583460ae288..00000000000 --- a/vespalib/src/vespa/vespalib/cppunit/.gitignore +++ /dev/null @@ -1,3 +0,0 @@ -*.So -.depend -Makefile diff --git a/vespalib/src/vespa/vespalib/objects/nbostream.h b/vespalib/src/vespa/vespalib/objects/nbostream.h index b0823f76d71..7119645622f 100644 --- a/vespalib/src/vespa/vespalib/objects/nbostream.h +++ b/vespalib/src/vespa/vespalib/objects/nbostream.h @@ -5,6 +5,7 @@ #include <vespa/vespalib/stllike/string.h> #include <vespa/vespalib/util/array.h> #include <vespa/vespalib/util/buffer.h> +#include <vespa/vespalib/util/bfloat16.h> #include "nbo.h" namespace vespalib { @@ -38,6 +39,8 @@ public: nbostream & operator >> (double & v) { double n; read8(&n); v = nbo::n2h(n); return *this; } nbostream & operator << (float v) { float n(nbo::n2h(v)); write4(&n); return *this; } nbostream & operator >> (float & v) { float n; read4(&n); v = nbo::n2h(n); return *this; } + nbostream & operator << (BFloat16 v) { uint16_t n(nbo::n2h(v.get_bits())); write2(&n); return *this; } + nbostream & operator >> (BFloat16 & v) { uint16_t n; read2(&n); v.assign_bits(nbo::n2h(n)); return *this; } nbostream & operator << (int64_t v) { int64_t n(nbo::n2h(v)); write8(&n); return *this; } nbostream & operator >> (int64_t & v) { int64_t n; read8(&n); v = nbo::n2h(n); return *this; } nbostream & operator << (uint64_t v) { uint64_t n(nbo::n2h(v)); write8(&n); return *this; } diff --git a/vespalib/src/vespa/vespalib/regex/regex.cpp b/vespalib/src/vespa/vespalib/regex/regex.cpp index ebdbb256d19..97e9512fbc6 100644 --- a/vespalib/src/vespa/vespalib/regex/regex.cpp +++ b/vespalib/src/vespa/vespalib/regex/regex.cpp @@ -12,12 +12,12 @@ using re2::StringPiece; // All RE2 instances use a Quiet option to prevent the library from // complaining to stderr if pattern compilation fails. -Regex::Regex(std::shared_ptr<const Impl> impl) +Regex::Regex(std::unique_ptr<const Impl> impl) : _impl(std::move(impl)) {} -Regex::Regex(const Regex&) = default; -Regex& Regex::operator=(const Regex&) = default; +Regex::Regex() : _impl() {} + Regex::Regex(Regex&&) noexcept = default; Regex& Regex::operator=(Regex&&) noexcept = default; @@ -61,7 +61,7 @@ Regex Regex::from_pattern(std::string_view pattern, uint32_t opt_mask) { if ((opt_mask & Options::DotMatchesNewline) != 0) { opts.set_dot_nl(true); } - return Regex(std::make_shared<const Impl>(pattern, opts)); + return Regex(std::make_unique<const Impl>(pattern, opts)); } bool Regex::parsed_ok() const noexcept { diff --git a/vespalib/src/vespa/vespalib/regex/regex.h b/vespalib/src/vespa/vespalib/regex/regex.h index 0c80f4e5d3a..0b096bef53e 100644 --- a/vespalib/src/vespa/vespalib/regex/regex.h +++ b/vespalib/src/vespa/vespalib/regex/regex.h @@ -37,9 +37,9 @@ namespace vespalib { */ class Regex { class Impl; - std::shared_ptr<const Impl> _impl; // shared_ptr to allow for cheap copying. + std::unique_ptr<const Impl> _impl; // shared_ptr to allow for cheap copying. - explicit Regex(std::shared_ptr<const Impl> impl); + explicit Regex(std::unique_ptr<const Impl> impl); public: // TODO consider using type-safe parameter instead. enum Options { @@ -48,12 +48,15 @@ public: DotMatchesNewline = 2 }; + //Default constructed object is invalid + Regex(); + ~Regex(); - Regex(const Regex&); - Regex& operator=(const Regex&); Regex(Regex&&) noexcept; Regex& operator=(Regex&&) noexcept; + bool valid() const { return bool(_impl); } + [[nodiscard]] bool parsed_ok() const noexcept; [[nodiscard]] bool partial_match(std::string_view input) const noexcept; diff --git a/vespalib/src/vespa/vespalib/util/CMakeLists.txt b/vespalib/src/vespa/vespalib/util/CMakeLists.txt index 64c27482e00..62d642b76b2 100644 --- a/vespalib/src/vespa/vespalib/util/CMakeLists.txt +++ b/vespalib/src/vespa/vespalib/util/CMakeLists.txt @@ -10,6 +10,7 @@ vespa_add_library(vespalib_vespalib_util OBJECT backtrace.cpp barrier.cpp benchmark_timer.cpp + bfloat16.cpp blockingthreadstackexecutor.cpp box.cpp child_process.cpp diff --git a/vespalib/src/vespa/vespalib/util/bfloat16.cpp b/vespalib/src/vespa/vespalib/util/bfloat16.cpp new file mode 100644 index 00000000000..5713ea66886 --- /dev/null +++ b/vespalib/src/vespa/vespalib/util/bfloat16.cpp @@ -0,0 +1,3 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "bfloat16.h" diff --git a/vespalib/src/vespa/vespalib/util/bfloat16.h b/vespalib/src/vespa/vespalib/util/bfloat16.h new file mode 100644 index 00000000000..573f94ec89f --- /dev/null +++ b/vespalib/src/vespa/vespalib/util/bfloat16.h @@ -0,0 +1,131 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <bit> +#include <cstdint> +#include <cstring> +#include <limits> + +namespace vespalib { + +/** + * Class holding 16-bit floating-point numbers. + * Truncated version of normal 32-bit float; the sign and + * exponent are kept as-is but the mantissa has only 8-bit + * precision. Well suited for ML / AI, halving memory + * requirements for large vectors and similar data. + * Direct HW support possible (AVX-512 BF16 extension etc.) + * See also: + * https://en.wikipedia.org/wiki/Bfloat16_floating-point_format + **/ +class BFloat16 { +private: + uint16_t _bits; + struct TwoU16 { + uint16_t u1; + uint16_t u2; + }; + + template<std::endian native_endian = std::endian::native> + static constexpr uint16_t float_to_bits(float value) noexcept { + TwoU16 both{0,0}; + static_assert(sizeof(TwoU16) == sizeof(float)); + memcpy(&both, &value, sizeof(float)); + if constexpr (native_endian == std::endian::big) { + return both.u1; + } else { + static_assert(native_endian == std::endian::little, + "Unknown endian, cannot handle"); + return both.u2; + } + } + + template<std::endian native_endian = std::endian::native> + static constexpr float bits_to_float(uint16_t bits) noexcept { + TwoU16 both{0,0}; + if constexpr (native_endian == std::endian::big) { + both.u1 = bits; + } else { + static_assert(native_endian == std::endian::little, + "Unknown endian, cannot handle"); + both.u2 = bits; + } + float result = 0.0; + static_assert(sizeof(TwoU16) == sizeof(float)); + memcpy(&result, &both, sizeof(float)); + return result; + } +public: + constexpr BFloat16(float value) noexcept : _bits(float_to_bits(value)) {} + BFloat16() noexcept = default; + ~BFloat16() noexcept = default; + constexpr BFloat16(const BFloat16 &other) noexcept = default; + constexpr BFloat16(BFloat16 &&other) noexcept = default; + constexpr BFloat16& operator=(const BFloat16 &other) noexcept = default; + constexpr BFloat16& operator=(BFloat16 &&other) noexcept = default; + constexpr BFloat16& operator=(float value) noexcept { + _bits = float_to_bits(value); + return *this; + } + + constexpr operator float () const noexcept { return bits_to_float(_bits); } + + constexpr float to_float() const noexcept { return bits_to_float(_bits); } + constexpr void assign(float value) noexcept { _bits = float_to_bits(value); } + + constexpr uint16_t get_bits() const { return _bits; } + constexpr void assign_bits(uint16_t value) noexcept { _bits = value; } +}; + +} + +namespace std { +template<> class numeric_limits<vespalib::BFloat16> { +public: + static constexpr bool is_specialized = true; + static constexpr bool is_signed = true; + static constexpr bool is_integer = false; + static constexpr bool is_exact = false; + static constexpr bool has_infinity = false; + static constexpr bool has_quiet_NaN = true; + static constexpr bool has_signaling_NaN = true; + static constexpr bool has_denorm = true; + static constexpr bool has_denorm_loss = false; + static constexpr bool is_iec559 = false; + static constexpr bool is_bounded = true; + static constexpr bool is_modulo = false; + static constexpr bool traps = false; + static constexpr bool tinyness_before = false; + + static constexpr std::float_round_style round_style = std::round_toward_zero; + static constexpr int radix = 2; + + static constexpr int digits = 8; + static constexpr int digits10 = 2; + static constexpr int max_digits10 = 4; + + static constexpr int min_exponent = -125; + static constexpr int min_exponent10 = -2; + + static constexpr int max_exponent = 128; + static constexpr int max_exponent10 = 38; + + static constexpr vespalib::BFloat16 denorm_min() noexcept { return 0x1.0p-133; } + static constexpr vespalib::BFloat16 epsilon() noexcept { return 0x1.0p-7; } + static constexpr vespalib::BFloat16 lowest() noexcept { return -0x1.FEp127; } + static constexpr vespalib::BFloat16 max() noexcept { return 0x1.FEp127; } + static constexpr vespalib::BFloat16 min() noexcept { return 0x1.0p-126; } + static constexpr vespalib::BFloat16 round_error() noexcept { return 1.0; } + static constexpr vespalib::BFloat16 infinity() noexcept { + return std::numeric_limits<float>::infinity(); + } + static constexpr vespalib::BFloat16 quiet_NaN() noexcept { + return std::numeric_limits<float>::quiet_NaN(); + } + static constexpr vespalib::BFloat16 signaling_NaN() noexcept { + return std::numeric_limits<float>::signaling_NaN(); + } +}; + +} diff --git a/zkfacade/pom.xml b/zkfacade/pom.xml index 70079e53c78..179856f053f 100644 --- a/zkfacade/pom.xml +++ b/zkfacade/pom.xml @@ -42,27 +42,10 @@ <scope>provided</scope> </dependency> <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>testutil</artifactId> - <version>${project.version}</version> - <scope>test</scope> - </dependency> - <dependency> <groupId>org.apache.curator</groupId> <artifactId>curator-recipes</artifactId> </dependency> <dependency> - <groupId>org.apache.curator</groupId> - <artifactId>curator-test</artifactId> - <scope>test</scope> - <exclusions> - <exclusion> - <groupId>com.google.guava</groupId> - <artifactId>guava</artifactId> - </exclusion> - </exclusions> - </dependency> - <dependency> <groupId>com.google.guava</groupId> <artifactId>guava</artifactId> <scope>provided</scope> diff --git a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java index 5341efaefe5..96619258ed3 100644 --- a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java +++ b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java @@ -1,55 +1,27 @@ -// 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.curator; import com.yahoo.cloud.config.CuratorConfig; import com.yahoo.net.HostName; -import org.apache.curator.test.TestingServer; -import org.junit.After; -import org.junit.Before; import org.junit.Test; -import java.io.IOException; import java.util.Optional; import static org.junit.Assert.assertEquals; /** - * Sets up actual ZooKeeper servers and verifies we can talk to them. + * Very simple testing of setting up curator * * @author Ulf Lilleengen */ public class CuratorTest { - private static String localhost = HostName.getLocalhost(); + private static final String localhost = HostName.getLocalhost(); - private String spec1; - private String spec2; - private TestingServer test1; - private TestingServer test2; - private int port1; - private int port2; - - @Before - public void setupServers() throws Exception { - port1 = allocatePort(); - port2 = allocatePort(); - test1 = new TestingServer(port1); - test2 = new TestingServer(port2); - spec1 = localhost + ":" + port1; - spec2 = localhost + ":" + port2; - } - - private int allocatePort() { - return PortAllocator.findAvailablePort(); - } - - @After - public void teardownServers() throws IOException { - test1.stop(); - test1.close(); - test2.close(); - test2.stop(); - } + private static final int port1 = 1; + private static final int port2 = 2; + private static final String spec1 = localhost + ":" + port1; + private static final String spec2 = localhost + ":" + port2; @Test public void require_curator_is_created_from_config() { @@ -90,28 +62,4 @@ public class CuratorTest { Optional.empty()); } - private static class PortAllocator { - - private static class PortRange { - private int first = 18621; - private int last = 18630; // see: factory/doc/port-ranges - private int value = first; - - synchronized int next() { - if (value > last) { - throw new RuntimeException("no port ports in range"); - } - return value++; - } - } - - private final static PortRange portRange = new PortRange(); - - // Get the next port from a pre-allocated range - static int findAvailablePort() { - return portRange.next(); - } - - } - } |