diff options
53 files changed, 499 insertions, 409 deletions
diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index aebc0e93c9c..aa6b16f8791 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -30,7 +30,7 @@ <javax.inject.version>1</javax.inject.version> <javax.servlet-api.version>3.1.0</javax.servlet-api.version> <jaxb.version>2.3.0</jaxb.version> - <jetty.version>9.4.36.v20210114</jetty.version> + <jetty.version>9.4.38.v20210224</jetty.version> <junit5.version>5.7.0</junit5.version> <junit5.platform.version>1.7.0</junit5.platform.version> <org.lz4.version>1.7.1</org.lz4.version> 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 367977e117f..53ecc5892b6 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 @@ -7,6 +7,7 @@ import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.application.api.FileRegistry; 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.HostName; import com.yahoo.config.provision.NodeResources; @@ -82,11 +83,10 @@ public interface ModelContext { @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 = {"bjorncs", "tokle"}, removeAfter = "7.357") default boolean enableJdiscConnectionLog() { return true; } - @ModelFeatureFlag(owners = {"bjorncs", "tokle", "baldersheim"}, removeAfter = "7.357") default boolean enableZstdCompressionAccessLog() { 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 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; } @ModelFeatureFlag(owners = {"vekterli"}) default int maxActivationInhibitedOutOfSyncGroups() { return 0; } 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 3d06fdf6758..be15abc5ac8 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 @@ -10,6 +10,7 @@ import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.api.Quota; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.AthenzDomain; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.Zone; @@ -56,6 +57,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private boolean enableFeedBlockInDistributor = false; private double maxDeadBytesRatio = 0.2; private int clusterControllerMaxHeapSizeInMb = 512; + private int metricsProxyMaxHeapSizeInMb = 512; private int maxActivationInhibitedOutOfSyncGroups = 0; @Override public ModelContext.FeatureFlags featureFlags() { return this; } @@ -95,6 +97,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public boolean enableFeedBlockInDistributor() { return enableFeedBlockInDistributor; } @Override public double maxDeadBytesRatio() { return maxDeadBytesRatio; } @Override public int clusterControllerMaxHeapSizeInMb() { return clusterControllerMaxHeapSizeInMb; } + @Override public int metricsProxyMaxHeapSizeInMb(ClusterSpec.Type type) { return metricsProxyMaxHeapSizeInMb; } @Override public int maxActivationInhibitedOutOfSyncGroups() { return maxActivationInhibitedOutOfSyncGroups; } public TestProperties setFeedConcurrency(double feedConcurrency) { @@ -227,6 +230,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea return this; } + public TestProperties metricsProxyMaxHeapSizeInMb(int heapSize) { + metricsProxyMaxHeapSizeInMb = heapSize; + return this; + } + public TestProperties maxActivationInhibitedOutOfSyncGroups(int nGroups) { maxActivationInhibitedOutOfSyncGroups = nGroups; return this; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java index 08f7ff82f4e..0f006c31959 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java @@ -75,7 +75,7 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable private Optional<LogserverContainerCluster> logServerContainerCluster = Optional.empty(); private ZooKeepersConfigProvider zooKeepersConfigProvider; - private FileDistributionConfigProducer fileDistribution; + private final FileDistributionConfigProducer fileDistribution; private final boolean multitenant; public Admin(AbstractConfigProducer parent, @@ -220,7 +220,7 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable for (var host : hosts) { // Send hostname to be used in configId (instead of index), as the sorting of hosts seems to be unstable // between config changes, even when the set of hosts is unchanged. - var container = new MetricsProxyContainer(metricsProxyCluster, host.getHostname(), index, deployState.isHosted()); + var container = new MetricsProxyContainer(metricsProxyCluster, host, index, deployState); addAndInitializeService(deployState.getDeployLogger(), host, container); metricsProxyCluster.addContainer(container); } 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 aef7c846a19..690900865ad 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 @@ -11,14 +11,19 @@ import ai.vespa.metricsproxy.rpc.RpcConnector; import ai.vespa.metricsproxy.rpc.RpcConnectorConfig; import ai.vespa.metricsproxy.service.VespaServices; import ai.vespa.metricsproxy.service.VespaServicesConfig; +import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.api.container.ContainerServiceType; -import com.yahoo.config.model.producer.AbstractConfigProducer; +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; import com.yahoo.vespa.model.container.Container; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Optional; import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_PROXY_CONTAINER; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.METRICS_PROXY_BUNDLE_NAME; @@ -33,15 +38,23 @@ public class MetricsProxyContainer extends Container implements NodeDimensionsConfig.Producer, NodeInfoConfig.Producer, RpcConnectorConfig.Producer, - VespaServicesConfig.Producer + VespaServicesConfig.Producer, + QrStartConfig.Producer { public static final int BASEPORT = 19092; final boolean isHostedVespa; + private final Optional<ClusterMembership> clusterMembership; + private final ModelContext.FeatureFlags featureFlags; + private final MetricsProxyContainerCluster cluster; - public MetricsProxyContainer(AbstractConfigProducer<?> parent, String hostname, int index, boolean isHostedVespa) { - super(parent, hostname, index, isHostedVespa); - this.isHostedVespa = isHostedVespa; + + public MetricsProxyContainer(MetricsProxyContainerCluster cluster, HostResource host, int index, DeployState deployState) { + super(cluster, host.getHostname(), index, deployState.isHosted()); + this.isHostedVespa = deployState.isHosted(); + this.clusterMembership = host.spec().membership(); + this.featureFlags = deployState.featureFlags(); + this.cluster = cluster; setProp("clustertype", "admin"); setProp("index", String.valueOf(index)); addNodeSpecificComponents(); @@ -131,6 +144,19 @@ public class MetricsProxyContainer extends Container implements .hostname(getHostName()); } + @Override + public void getConfig(QrStartConfig.Builder builder) { + cluster.getConfig(builder); + + if (clusterMembership.isPresent()) { + int maxHeapSize = featureFlags.metricsProxyMaxHeapSizeInMb(clusterMembership.get().cluster().type()); + boolean verboseGc = (maxHeapSize < 512); + builder.jvm + .verbosegc(verboseGc) + .heapsize(maxHeapSize); + } + } + private String getNodeRole() { String hostConfigId = getHost().getHost().getConfigId(); if (! isHostedVespa) return hostConfigId; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java index a0b99516ce3..194967f9868 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java @@ -86,7 +86,6 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC private final AbstractConfigProducer<?> parent; private final ApplicationId applicationId; - public MetricsProxyContainerCluster(AbstractConfigProducer<?> parent, String name, DeployState deployState) { super(parent, name, name, deployState, true); this.parent = parent; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java index b28a2edff0e..674d2ce3325 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java @@ -78,7 +78,7 @@ public class JettyHttpServer extends SimpleComponent implements ServerConfig.Pro if (cluster instanceof ApplicationContainerCluster) { configureApplicationClusterJettyThreadPool(builder); } else { - builder.minWorkerThreads(2).maxWorkerThreads(4); + builder.minWorkerThreads(4).maxWorkerThreads(4); } } private void configureApplicationClusterJettyThreadPool(ServerConfig.Builder builder) { diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java index eeded9e6c2f..f1c81e29923 100644 --- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java @@ -1795,7 +1795,7 @@ public class ModelProvisioningTest { assertTrue("Initial servers are not joining", config.build().server().stream().noneMatch(ZookeeperServerConfig.Server::joining)); } { - VespaModel nextModel = tester.createModel(Zone.defaultZone(), servicesXml.apply(5), true, false, 0, Optional.of(model)); + VespaModel nextModel = tester.createModel(Zone.defaultZone(), servicesXml.apply(5), true, false, 0, Optional.of(model), new DeployState.Builder()); ApplicationContainerCluster cluster = nextModel.getContainerClusters().get("zk"); ZookeeperServerConfig.Builder config = new ZookeeperServerConfig.Builder(); cluster.getContainers().forEach(c -> c.getConfig(config)); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java index 7ed546c445d..1a7258db7e2 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java @@ -13,6 +13,9 @@ import ai.vespa.metricsproxy.http.yamas.YamasHandler; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensionsConfig; import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions; import com.yahoo.component.ComponentSpecification; +import com.yahoo.config.model.api.HostInfo; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.config.model.deploy.TestProperties; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.provision.Zone; import com.yahoo.container.core.ApplicationMetadataConfig; @@ -23,9 +26,9 @@ import com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.App import com.yahoo.vespa.model.container.component.Component; import com.yahoo.vespa.model.container.component.Handler; import org.junit.Test; + import java.util.Collection; -import static com.yahoo.vespa.model.container.ContainerCluster.G1GC; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.METRICS_PROXY_BUNDLE_FILE; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.zoneString; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.CLUSTER_CONFIG_ID; @@ -39,6 +42,7 @@ import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.g import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getModel; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getQrStartConfig; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.servicesWithAdminOnly; +import static com.yahoo.vespa.model.container.ContainerCluster.G1GC; import static java.util.stream.Collectors.toList; import static org.hamcrest.CoreMatchers.endsWith; import static org.hamcrest.CoreMatchers.hasItem; @@ -74,24 +78,39 @@ public class MetricsProxyContainerClusterTest { } private void metrics_proxy_has_expected_qr_start_options(MetricsProxyModelTester.TestMode mode) { - VespaModel model = getModel(servicesWithAdminOnly(), mode); - QrStartConfig qrStartConfig = getQrStartConfig(model); - assertEquals(32, qrStartConfig.jvm().minHeapsize()); - assertEquals(512, qrStartConfig.jvm().heapsize()); - assertEquals(0, qrStartConfig.jvm().heapSizeAsPercentageOfPhysicalMemory()); - assertEquals(2, qrStartConfig.jvm().availableProcessors()); - assertFalse(qrStartConfig.jvm().verbosegc()); - assertEquals(G1GC, qrStartConfig.jvm().gcopts()); - assertEquals(512, qrStartConfig.jvm().stacksize()); - assertEquals(0, qrStartConfig.jvm().directMemorySizeCache()); - assertEquals(32, qrStartConfig.jvm().compressedClassSpaceSize()); - assertEquals(75, qrStartConfig.jvm().baseMaxDirectMemorySize()); + metrics_proxy_has_expected_qr_start_options(mode, 0); + } + + private void metrics_proxy_has_expected_qr_start_options(MetricsProxyModelTester.TestMode mode, int maxHeapForAdminClusterNodes) { + DeployState.Builder builder = new DeployState.Builder(); + if (maxHeapForAdminClusterNodes > 0) { + builder.properties(new TestProperties().metricsProxyMaxHeapSizeInMb(maxHeapForAdminClusterNodes)); + } + + VespaModel model = getModel(servicesWithAdminOnly(), mode, builder); + for (HostInfo host : model.getHosts()) { + QrStartConfig qrStartConfig = getQrStartConfig(model, host.getHostname()); + assertEquals(32, qrStartConfig.jvm().minHeapsize()); + assertEquals(maxHeapForAdminClusterNodes > 0 ? maxHeapForAdminClusterNodes : 512, qrStartConfig.jvm().heapsize()); + assertEquals(0, qrStartConfig.jvm().heapSizeAsPercentageOfPhysicalMemory()); + assertEquals(2, qrStartConfig.jvm().availableProcessors()); + assertFalse(qrStartConfig.jvm().verbosegc()); + assertEquals(G1GC, qrStartConfig.jvm().gcopts()); + assertEquals(512, qrStartConfig.jvm().stacksize()); + assertEquals(0, qrStartConfig.jvm().directMemorySizeCache()); + assertEquals(32, qrStartConfig.jvm().compressedClassSpaceSize()); + assertEquals(75, qrStartConfig.jvm().baseMaxDirectMemorySize()); + } } @Test public void metrics_proxy_has_expected_qr_start_options() { metrics_proxy_has_expected_qr_start_options(self_hosted); metrics_proxy_has_expected_qr_start_options(hosted); + + // With max heap from feature flag + metrics_proxy_has_expected_qr_start_options(self_hosted, 123); + metrics_proxy_has_expected_qr_start_options(hosted, 123); } @Test diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java index b6037d2614e..7fcd5c14a0f 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java @@ -1,7 +1,4 @@ -// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -/* - * 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.metricsproxy; @@ -11,12 +8,15 @@ import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensionsConfig; import ai.vespa.metricsproxy.metric.dimensions.NodeDimensionsConfig; import ai.vespa.metricsproxy.rpc.RpcConnectorConfig; import ai.vespa.metricsproxy.service.VespaServicesConfig; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.admin.monitoring.Metric; import com.yahoo.vespa.model.admin.monitoring.MetricsConsumer; import com.yahoo.vespa.model.test.VespaModelTester; +import java.util.Optional; + import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.TestMode.hosted; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.TestMode.self_hosted; @@ -40,12 +40,16 @@ class MetricsProxyModelTester { } static VespaModel getModel(String servicesXml, TestMode testMode) { + return getModel(servicesXml, testMode, new DeployState.Builder()); + } + + static VespaModel getModel(String servicesXml, TestMode testMode, DeployState.Builder builder) { var numberOfHosts = testMode == hosted ? 2 : 1; var tester = new VespaModelTester(); tester.addHosts(numberOfHosts); tester.setHosted(testMode == hosted); if (testMode == hosted) tester.setApplicationId(MY_TENANT, MY_APPLICATION, MY_INSTANCE); - return tester.createModel(servicesXml, true); + return tester.createModel(servicesXml, true, builder); } static String containerConfigId(VespaModel model, MetricsProxyModelTester.TestMode mode) { @@ -98,8 +102,8 @@ class MetricsProxyModelTester { return model.getConfig(ApplicationDimensionsConfig.class, CLUSTER_CONFIG_ID); } - static QrStartConfig getQrStartConfig(VespaModel model) { - return model.getConfig(QrStartConfig.class, CLUSTER_CONFIG_ID); + static QrStartConfig getQrStartConfig(VespaModel model, String hostname) { + return model.getConfig(QrStartConfig.class, CLUSTER_CONFIG_ID + "/" + hostname); } static NodeDimensionsConfig getNodeDimensionsConfig(VespaModel model, String configId) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java index e779b5e43cb..220f87001aa 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java @@ -6,14 +6,12 @@ import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.ConfigModelRegistry; import com.yahoo.config.model.NullConfigModelRegistry; import com.yahoo.config.model.api.HostProvisioner; -import com.yahoo.config.model.api.Provisioned; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; import com.yahoo.config.model.provision.Host; import com.yahoo.config.model.provision.Hosts; import com.yahoo.config.model.provision.InMemoryProvisioner; import com.yahoo.config.model.provision.SingleNodeProvisioner; -import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterSpec; @@ -21,7 +19,6 @@ import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.ProvisionLogger; -import com.yahoo.config.provision.Provisioner; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils; @@ -116,22 +113,38 @@ public class VespaModelTester { /** Creates a model which uses 0 as start index */ public VespaModel createModel(String services, boolean failOnOutOfCapacity, String ... retiredHostNames) { - return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, 0, Optional.empty(), retiredHostNames); + return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, 0, + Optional.empty(), new DeployState.Builder(), retiredHostNames); + } + + /** Creates a model which uses 0 as start index */ + public VespaModel createModel(String services, boolean failOnOutOfCapacity, DeployState.Builder builder) { + return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, 0, Optional.empty(), builder); } /** Creates a model which uses 0 as start index */ public VespaModel createModel(String services, boolean failOnOutOfCapacity, boolean useMaxResources, String ... retiredHostNames) { - return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, useMaxResources, 0, Optional.empty(), retiredHostNames); + return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, useMaxResources, 0, + Optional.empty(), new DeployState.Builder(), retiredHostNames); } /** Creates a model which uses 0 as start index */ public VespaModel createModel(String services, boolean failOnOutOfCapacity, int startIndexForClusters, String ... retiredHostNames) { - return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, startIndexForClusters, Optional.empty(), retiredHostNames); + return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, startIndexForClusters, + Optional.empty(), new DeployState.Builder(), retiredHostNames); } /** Creates a model which uses 0 as start index */ public VespaModel createModel(Zone zone, String services, boolean failOnOutOfCapacity, String ... retiredHostNames) { - return createModel(zone, services, failOnOutOfCapacity, false, 0, Optional.empty(), retiredHostNames); + return createModel(zone, services, failOnOutOfCapacity, false, 0, + Optional.empty(), new DeployState.Builder(), retiredHostNames); + } + + /** Creates a model which uses 0 as start index */ + public VespaModel createModel(Zone zone, String services, boolean failOnOutOfCapacity, + DeployState.Builder deployStateBuilder, String ... retiredHostNames) { + return createModel(zone, services, failOnOutOfCapacity, false, 0, + Optional.empty(), deployStateBuilder, retiredHostNames); } /** @@ -144,7 +157,8 @@ public class VespaModelTester { * @return the resulting model */ public VespaModel createModel(Zone zone, String services, boolean failOnOutOfCapacity, boolean useMaxResources, - int startIndexForClusters, Optional<VespaModel> previousModel, String ... retiredHostNames) { + int startIndexForClusters, Optional<VespaModel> previousModel, + DeployState.Builder deployStatebuilder, String ... retiredHostNames) { VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(null, services, ApplicationPackageUtils.generateSearchDefinition("type1")); ApplicationPackage appPkg = modelCreatorWithMockPkg.appPkg; @@ -164,7 +178,7 @@ public class VespaModelTester { .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver) .setDedicatedClusterControllerCluster(dedicatedClusterControllerCluster); - DeployState.Builder deployState = new DeployState.Builder() + DeployState.Builder deployState = deployStatebuilder .applicationPackage(appPkg) .modelHostProvisioner(provisioner) .properties(properties) 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 a5b8c57ccaa..2568ff23f9e 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 @@ -19,6 +19,7 @@ import com.yahoo.config.model.api.Quota; import com.yahoo.config.model.api.Reindexing; 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; @@ -28,6 +29,7 @@ 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; @@ -36,8 +38,10 @@ import java.net.URI; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.ToIntFunction; import static com.yahoo.vespa.config.server.ConfigServerSpec.fromConfig; +import static com.yahoo.vespa.flags.FetchVector.Dimension.CLUSTER_TYPE; /** * Implementation of {@link ModelContext} for configserver. @@ -169,6 +173,7 @@ public class ModelContextImpl implements ModelContext { private final boolean enableFeedBlockInDistributor; private final double maxDeadBytesRatio; private final int clusterControllerMaxHeapSizeInMb; + private final ToIntFunction<ClusterSpec.Type> metricsProxyMaxHeapSizeInMb; private final List<String> allowedAthenzProxyIdentities; private final boolean tenantIamRole; private final int maxActivationInhibitedOutOfSyncGroups; @@ -193,6 +198,7 @@ public class ModelContextImpl implements ModelContext { this.enableFeedBlockInDistributor = flagValue(source, appId, Flags.ENABLE_FEED_BLOCK_IN_DISTRIBUTOR); this.maxDeadBytesRatio = flagValue(source, appId, Flags.MAX_DEAD_BYTES_RATIO); this.clusterControllerMaxHeapSizeInMb = flagValue(source, appId, Flags.CLUSTER_CONTROLLER_MAX_HEAP_SIZE_IN_MB); + this.metricsProxyMaxHeapSizeInMb = type -> Flags.METRICS_PROXY_MAX_HEAP_SIZE_IN_MB.bindTo(source).with(CLUSTER_TYPE, type.name()).value(); this.allowedAthenzProxyIdentities = flagValue(source, appId, Flags.ALLOWED_ATHENZ_PROXY_IDENTITIES); this.tenantIamRole = flagValue(source, appId.tenant(), Flags.TENANT_IAM_ROLE); this.maxActivationInhibitedOutOfSyncGroups = flagValue(source, appId, Flags.MAX_ACTIVATION_INHIBITED_OUT_OF_SYNC_GROUPS); @@ -217,6 +223,7 @@ public class ModelContextImpl implements ModelContext { @Override public boolean enableFeedBlockInDistributor() { return enableFeedBlockInDistributor; } @Override public double maxDeadBytesRatio() { return maxDeadBytesRatio; } @Override public int clusterControllerMaxHeapSizeInMb() { return clusterControllerMaxHeapSizeInMb; } + @Override public int metricsProxyMaxHeapSizeInMb(ClusterSpec.Type type) { return metricsProxyMaxHeapSizeInMb.applyAsInt(type); } @Override public List<String> allowedAthenzProxyIdentities() { return allowedAthenzProxyIdentities; } @Override public boolean tenantIamRole() { return tenantIamRole; } @Override public int maxActivationInhibitedOutOfSyncGroups() { return maxActivationInhibitedOutOfSyncGroups; } diff --git a/container-dependency-versions/pom.xml b/container-dependency-versions/pom.xml index 2bedd898fd9..a278421f603 100644 --- a/container-dependency-versions/pom.xml +++ b/container-dependency-versions/pom.xml @@ -458,7 +458,7 @@ <javax.inject.version>1</javax.inject.version> <javax.servlet-api.version>3.1.0</javax.servlet-api.version> <jaxb.version>2.3.0</jaxb.version> - <jetty.version>9.4.36.v20210114</jetty.version> + <jetty.version>9.4.38.v20210224</jetty.version> <org.lz4.version>1.7.1</org.lz4.version> <org.json.version>20090211</org.json.version> <slf4j.version>1.7.5</slf4j.version> diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java index 2a8dc34ea72..f5753e4eb5c 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java @@ -58,7 +58,6 @@ enum PathGroup { tenantInfo(Matcher.tenant, PathPrefix.api, "/application/v4/tenant/{tenant}/application/", - "/application/v4/tenant/{tenant}/secret-store/", "/application/v4/tenant/{tenant}/info/", "/routing/v1/status/tenant/{tenant}/{*}"), @@ -244,7 +243,10 @@ enum PathGroup { "/billing/v1/billing"), /** Path used for listing endpoint certificate request info */ - endpointCertificateRequestInfo(PathPrefix.none, "/certificateRequests/"); + endpointCertificateRequestInfo(PathPrefix.none, "/certificateRequests/"), + + /** Path used for secret store management */ + secretStore(Matcher.tenant, PathPrefix.api, "/application/v4/tenant/{tenant}/secret-store/{*}"); final List<String> pathSpecs; final PathPrefix prefix; 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 12bae955c20..ecf3d29bc1a 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 @@ -186,7 +186,12 @@ enum Policy { /** Listing endpoint certificate request info */ endpointCertificateRequestInfo(Privilege.grant(Action.read) .on(PathGroup.endpointCertificateRequestInfo) - .in(SystemName.all())); + .in(SystemName.all())), + + /** Secret store operations */ + secretStoreOperations(Privilege.grant(Action.all()) + .on(PathGroup.secretStore) + .in(SystemName.PublicCd, SystemName.Public)); private final Set<Privilege> privileges; 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 85db447dfbd..3b861c607b1 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 @@ -68,7 +68,8 @@ public enum RoleDefinition { Policy.paymentInstrumentDelete, Policy.paymentInstrumentCreate, Policy.planUpdate, - Policy.billingInformationRead), + Policy.billingInformationRead, + Policy.secretStoreOperations), /** Headless — the application specific role identified by deployment keys for production */ headless(Policy.submission), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index a53d5bddbfc..3649bc2ac80 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -223,6 +223,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant")) return tenants(request); if (path.matches("/application/v4/tenant/{tenant}")) return tenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/info")) return tenantInfo(path.get("tenant"), request); + if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}/validate")) return validateSecretStore(path.get("tenant"), path.get("name")); if (path.matches("/application/v4/tenant/{tenant}/application")) return applications(path.get("tenant"), Optional.empty(), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return application(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/compile-version")) return compileVersion(path.get("tenant"), path.get("application")); @@ -268,7 +269,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private HttpResponse handlePUT(Path path, HttpRequest request) { if (path.matches("/application/v4/tenant/{tenant}")) return updateTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/info")) return updateTenantInfo(path.get("tenant"), request); - if (path.matches("/application/v4/tenant/{tenant}/secret-store")) return addSecretStore(path.get("tenant"), request); + if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}")) return addSecretStore(path.get("tenant"), path.get("name"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/global-rotation/override")) return setGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), false, request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation/override")) return setGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), false, request); return ErrorResponse.notFoundError("Nothing at " + path); @@ -297,7 +298,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/reindexing")) return enableReindexing(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/restart")) return restart(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/suspend")) return suspend(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), true); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/validate-parameter-store")) return validateParameterStore(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deploy(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/deploy")) return deploy(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); // legacy synonym of the above if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/restart")) return restart(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); @@ -583,25 +583,44 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } - private HttpResponse validateParameterStore(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) { + private HttpResponse validateSecretStore(String tenantName, String name) { var tenant = TenantName.from(tenantName); if (controller.tenants().require(tenant).type() != Tenant.Type.cloud) - throw new IllegalArgumentException("Tenant '" + tenant + "' is not a cloud tenant"); + return ErrorResponse.badRequest("Tenant '" + tenant + "' is not a cloud tenant"); - var application = ApplicationId.from(tenantName, applicationName, instanceName); - var zone = requireZone(environment, region); - var deployment = new DeploymentId(application, zone); + var cloudTenant = (CloudTenant)controller.tenants().require(tenant); + var tenantSecretStore = cloudTenant.tenantSecretStores() + .stream() + .filter(secretStore -> secretStore.getName().equals(name)) + .findFirst(); + var deployment = getActiveDeployment(tenant); - var data = toSlime(request.getData()).get(); - var awsId = mandatory("awsId", data).asString(); - var name = mandatory("name", data).asString(); - var role = mandatory("role", data).asString(); - var tenantSecretStore = new TenantSecretStore(name, awsId, role); + if (deployment.isEmpty()) + return ErrorResponse.badRequest("Tenant '" + tenantName + "' has no active deployments"); + if (tenantSecretStore.isEmpty()) + return ErrorResponse.notFoundError("No secret store '" + name + "' configured for tenant '" + tenantName + "'"); - var response = controller.serviceRegistry().configServer().validateSecretStore(deployment, tenantSecretStore); + var response = controller.serviceRegistry().configServer().validateSecretStore(deployment.get(), tenantSecretStore.get()); return new MessageResponse(response); } + private Optional<DeploymentId> getActiveDeployment(TenantName tenant) { + for (var application : controller.applications().asList(tenant)) { + var optionalInstance = application.instances().values() + .stream() + .filter(instance -> instance.deployments().keySet().size() > 0) + .findFirst(); + + if (optionalInstance.isPresent()) { + var instance = optionalInstance.get(); + var applicationId = instance.id(); + var zoneId = instance.deployments().keySet().stream().findFirst().orElseThrow(); + return Optional.of(new DeploymentId(applicationId, zoneId)); + } + } + return Optional.empty(); + } + private HttpResponse removeDeveloperKey(String tenantName, HttpRequest request) { if (controller.tenants().require(TenantName.from(tenantName)).type() != Tenant.Type.cloud) throw new IllegalArgumentException("Tenant '" + tenantName + "' is not a cloud tenant"); @@ -654,14 +673,13 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return new SlimeJsonResponse(root); } - private HttpResponse addSecretStore(String tenantName, HttpRequest request) { + private HttpResponse addSecretStore(String tenantName, String name, HttpRequest request) { if (controller.tenants().require(TenantName.from(tenantName)).type() != Tenant.Type.cloud) throw new IllegalArgumentException("Tenant '" + tenantName + "' is not a cloud tenant"); var data = toSlime(request.getData()).get(); var awsId = mandatory("awsId", data).asString(); var externalId = mandatory("externalId", data).asString(); - var name = mandatory("name", data).asString(); var role = mandatory("role", data).asString(); var tenant = (CloudTenant) controller.tenants().require(TenantName.from(tenantName)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index 35b0a7ba5b3..b669e942494 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -577,7 +577,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer @Override public String validateSecretStore(DeploymentId deployment, TenantSecretStore tenantSecretStore) { - return ""; + return deployment.toString() + " - " + tenantSecretStore.toString(); } public static class Application { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java index 8d6b4f1f629..6d8a1b6c025 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java @@ -1,12 +1,17 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.restapi.application; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.flags.PermanentFlags; +import com.yahoo.vespa.hosted.controller.LockedTenant; +import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.integration.billing.PlanId; +import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore; import com.yahoo.vespa.hosted.controller.api.role.Role; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; @@ -20,6 +25,7 @@ import org.junit.Test; import javax.ws.rs.ForbiddenException; import java.util.Collections; +import java.util.Optional; import java.util.Set; import static com.yahoo.application.container.handler.Request.Method.GET; @@ -117,10 +123,9 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { @Test public void test_secret_store_configuration() { var secretStoreRequest = - request("/application/v4/tenant/scoober/secret-store", PUT) + request("/application/v4/tenant/scoober/secret-store/some-name", PUT) .data("{" + "\"awsId\": \"123\"," + - "\"name\": \"some-name\"," + "\"role\": \"role-id\"," + "\"externalId\": \"321\"" + "}") @@ -132,20 +137,52 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { "}", 400); secretStoreRequest = - request("/application/v4/tenant/scoober/secret-store", PUT) + request("/application/v4/tenant/scoober/secret-store/should-fail", PUT) .data("{" + - "\"awsId\": \"123\"," + - "\"name\": \" \"," + + "\"awsId\": \" \"," + "\"role\": \"role-id\"," + "\"externalId\": \"321\"" + "}") .roles(Set.of(Role.administrator(tenantName))); tester.assertResponse(secretStoreRequest, "{" + "\"error-code\":\"BAD_REQUEST\"," + - "\"message\":\"Secret store TenantSecretStore{name=' ', awsId='123', role='role-id'} is invalid\"" + + "\"message\":\"Secret store TenantSecretStore{name='should-fail', awsId=' ', role='role-id'} is invalid\"" + "}", 400); } + @Test + public void validate_secret_store() { + var secretStoreRequest = + request("/application/v4/tenant/scoober/secret-store/secret-foo/validate", GET) + .roles(Set.of(Role.administrator(tenantName))); + tester.assertResponse(secretStoreRequest, "{" + + "\"error-code\":\"BAD_REQUEST\"," + + "\"message\":\"Tenant 'scoober' has no active deployments\"" + + "}", 400); + + deployApplication(); + secretStoreRequest = + request("/application/v4/tenant/scoober/secret-store/secret-foo/validate", GET) + .roles(Set.of(Role.administrator(tenantName))); + tester.assertResponse(secretStoreRequest, "{" + + "\"error-code\":\"NOT_FOUND\"," + + "\"message\":\"No secret store 'secret-foo' configured for tenant 'scoober'\"" + + "}", 404); + + tester.controller().tenants().lockOrThrow(tenantName, LockedTenant.Cloud.class, lockedTenant -> { + lockedTenant = lockedTenant.withSecretStore(new TenantSecretStore("secret-foo", "123", "some-role")); + tester.controller().tenants().store(lockedTenant); + }); + + // ConfigServerMock returns message on format deployment.toString() + " - " + tenantSecretStore.toString() + secretStoreRequest = + request("/application/v4/tenant/scoober/secret-store/secret-foo/validate", GET) + .roles(Set.of(Role.administrator(tenantName))); + tester.assertResponse(secretStoreRequest, "{" + + "\"message\":\"scoober.albums in prod.us-central-1 - TenantSecretStore{name='secret-foo', awsId='123', role='some-role'}\"" + + "}", 200); + } + private ApplicationPackageBuilder prodBuilder() { return new ApplicationPackageBuilder() .instances("default") @@ -167,4 +204,17 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { private static Credentials credentials(String name) { return new Auth0Credentials(() -> name, Collections.emptySet()); } + + private void deployApplication() { + var applicationPackage = new ApplicationPackageBuilder() + .instances("default") + .globalServiceId("foo") + .region("us-central-1") + .build(); + + tester.controller().applications().deploy(ApplicationId.from("scoober", "albums", "default"), + ZoneId.from("prod", "us-central-1"), + Optional.of(applicationPackage), + new DeployOptions(true, Optional.empty(), false, false)); + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java index b765b2c3afb..9e0d645583a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java @@ -174,10 +174,10 @@ public class UserApiTest extends ControllerContainerCloudTest { new File("second-developer-key.json")); // PUT in a new secret store for the tenant - tester.assertResponse(request("/application/v4/tenant/my-tenant/secret-store/", PUT) + tester.assertResponse(request("/application/v4/tenant/my-tenant/secret-store/secret-foo", PUT) .principal("admin@tenant") .roles(Set.of(Role.administrator(id.tenant()))) - .data("{\"name\":\"secret-foo\",\"awsId\":\"123\",\"role\":\"secret-role\",\"externalId\":\"abc\"}"), + .data("{\"awsId\":\"123\",\"role\":\"secret-role\",\"externalId\":\"abc\"}"), "{\"message\":\"Configured secret store: TenantSecretStore{name='secret-foo', awsId='123', role='secret-role'}\"}", 200); diff --git a/eval/CMakeLists.txt b/eval/CMakeLists.txt index 66270bb323b..05ac4cb78ab 100644 --- a/eval/CMakeLists.txt +++ b/eval/CMakeLists.txt @@ -50,6 +50,7 @@ vespa_define_module( src/tests/instruction/dense_tensor_peek_function src/tests/instruction/dense_xw_product_function src/tests/instruction/fast_rename_optimizer + src/tests/instruction/generic_cell_cast src/tests/instruction/generic_concat src/tests/instruction/generic_create src/tests/instruction/generic_join diff --git a/eval/src/tests/instruction/generic_cell_cast/CMakeLists.txt b/eval/src/tests/instruction/generic_cell_cast/CMakeLists.txt new file mode 100644 index 00000000000..3c7e9c5b83f --- /dev/null +++ b/eval/src/tests/instruction/generic_cell_cast/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(eval_generic_cell_cast_test_app TEST + SOURCES + generic_cell_cast_test.cpp + DEPENDS + vespaeval + GTest::GTest +) +vespa_add_test(NAME eval_generic_cell_cast_test_app COMMAND eval_generic_cell_cast_test_app) 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 new file mode 100644 index 00000000000..6a0b63bd562 --- /dev/null +++ b/eval/src/tests/instruction/generic_cell_cast/generic_cell_cast_test.cpp @@ -0,0 +1,68 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/eval/eval/simple_value.h> +#include <vespa/eval/eval/fast_value.h> +#include <vespa/eval/eval/value_codec.h> +#include <vespa/eval/instruction/generic_cell_cast.h> +#include <vespa/eval/eval/interpreted_function.h> +#include <vespa/eval/eval/test/reference_operations.h> +#include <vespa/eval/eval/test/gen_spec.h> +#include <vespa/vespalib/util/stringfmt.h> +#include <vespa/vespalib/gtest/gtest.h> + +using namespace vespalib; +using namespace vespalib::eval; +using namespace vespalib::eval::instruction; +using namespace vespalib::eval::test; + +using vespalib::make_string_short::fmt; + +GenSpec::seq_t N_16ths = [] (size_t i) noexcept { return (i + 1.0) / 16.0; }; + +GenSpec G() { return GenSpec().seq(N_16ths); } + +const std::vector<GenSpec> layouts = { + G(), + G().idx("x", 3), + G().idx("x", 3).idx("y", 5), + G().idx("x", 3).idx("y", 5).idx("z", 7), + G().map("x", {"a","b","c"}), + G().map("x", {"a","b","c"}).map("y", {"foo","bar"}), + G().map("x", {"a","b","c"}).map("y", {"foo","bar"}).map("z", {"i","j","k","l"}), + G().idx("x", 3).map("y", {"foo", "bar"}).idx("z", 7), + G().map("x", {"a","b","c"}).idx("y", 5).map("z", {"i","j","k","l"}) +}; + +TensorSpec perform_generic_cell_cast(const TensorSpec &a, CellType to, const ValueBuilderFactory &factory) +{ + Stash stash; + auto lhs = value_from_spec(a, factory); + auto my_op = GenericCellCast::make_instruction(lhs->type(), to, stash); + InterpretedFunction::EvalSingle single(factory, my_op); + return spec_from_value(single.eval(std::vector<Value::CREF>({*lhs}))); +} + +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 }) { + 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); + EXPECT_EQ(actual, expect); + } + } + } +} + +TEST(GenericCellCastTest, generic_cell_cast_works_for_simple_values) { + test_generic_cell_cast_with(SimpleValueBuilderFactory::get()); +} + +TEST(GenericCellCastTest, generic_cell_cast_works_for_fast_values) { + test_generic_cell_cast_with(FastValueBuilderFactory::get()); +} + +GTEST_MAIN_RUN_ALL_TESTS() 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 dedb22f2763..9c17dce972f 100644 --- a/eval/src/tests/instruction/generic_rename/generic_rename_test.cpp +++ b/eval/src/tests/instruction/generic_rename/generic_rename_test.cpp @@ -53,8 +53,8 @@ TEST(GenericRenameTest, dense_rename_plan_can_be_created_and_executed) { std::vector<vespalib::string> to({"f", "a", "b"}); ValueType renamed = lhs.rename(from, to); auto plan = DenseRenamePlan(lhs, renamed, from, to); - std::vector<size_t> expect_loop = {15,2,7}; - std::vector<size_t> expect_stride = {7,105,1}; + SmallVector<size_t> expect_loop = {15,2,7}; + SmallVector<size_t> expect_stride = {7,105,1}; EXPECT_EQ(plan.subspace_size, 210); EXPECT_EQ(plan.loop_cnt, expect_loop); EXPECT_EQ(plan.stride, expect_stride); @@ -84,7 +84,7 @@ TEST(GenericRenameTest, sparse_rename_plan_can_be_created) { ValueType renamed = lhs.rename(from, to); auto plan = SparseRenamePlan(lhs, renamed, from, to); EXPECT_EQ(plan.mapped_dims, 4); - std::vector<size_t> expect = {2,0,1,3}; + SmallVector<size_t> expect = {2,0,1,3}; EXPECT_EQ(plan.output_dimensions, expect); } diff --git a/eval/src/vespa/eval/eval/tensor_function.cpp b/eval/src/vespa/eval/eval/tensor_function.cpp index 4ea7348f61e..19dbe812d3e 100644 --- a/eval/src/vespa/eval/eval/tensor_function.cpp +++ b/eval/src/vespa/eval/eval/tensor_function.cpp @@ -5,6 +5,7 @@ #include "operation.h" #include "visit_stuff.h" #include "string_stuff.h" +#include <vespa/eval/instruction/generic_cell_cast.h> #include <vespa/eval/instruction/generic_concat.h> #include <vespa/eval/instruction/generic_create.h> #include <vespa/eval/instruction/generic_join.h> @@ -311,6 +312,14 @@ Lambda::visit_self(vespalib::ObjectVisitor &visitor) const //----------------------------------------------------------------------------- +InterpretedFunction::Instruction +CellCast::compile_self(const ValueBuilderFactory &, Stash &stash) const +{ + return instruction::GenericCellCast::make_instruction(child().result_type(), result_type().cell_type(), stash); +} + +//----------------------------------------------------------------------------- + void Peek::push_children(std::vector<Child::CREF> &children) const { diff --git a/eval/src/vespa/eval/eval/tensor_function.h b/eval/src/vespa/eval/eval/tensor_function.h index ed1106cccc1..55532bc4bf7 100644 --- a/eval/src/vespa/eval/eval/tensor_function.h +++ b/eval/src/vespa/eval/eval/tensor_function.h @@ -355,6 +355,20 @@ public: //----------------------------------------------------------------------------- +class CellCast : public Op1 +{ +private: + using Super = Op1; +public: + CellCast(const TensorFunction &child_in, CellType to_cell_type) + : Super(ValueType::cell_cast(child_in.result_type(), to_cell_type), child_in) + {} + bool result_is_mutable() const override { return true; } + InterpretedFunction::Instruction compile_self(const ValueBuilderFactory &factory, Stash &stash) const override; +}; + +//----------------------------------------------------------------------------- + class Peek : public Node { using Super = Node; diff --git a/eval/src/vespa/eval/eval/test/reference_operations.cpp b/eval/src/vespa/eval/eval/test/reference_operations.cpp index 5edd1a4f9a0..8709150bdc4 100644 --- a/eval/src/vespa/eval/eval/test/reference_operations.cpp +++ b/eval/src/vespa/eval/eval/test/reference_operations.cpp @@ -70,8 +70,28 @@ vespalib::string rename_dimension(const vespalib::string &name, const std::vecto return name; } +struct CopyCellsWithCast { + template<typename CT> + static void invoke(const TensorSpec &input, TensorSpec &output) { + for (const auto & [ addr, value ]: input.cells()) { + CT tmp = (CT) value; + output.add(addr, tmp); + } + } +}; + } // namespace <unnamed> +TensorSpec ReferenceOperations::cell_cast(const TensorSpec &a, CellType to) { + ValueType a_type = ValueType::from_spec(a.type()); + ValueType res_type = ValueType::cell_cast(a_type, to); + TensorSpec result(res_type.to_spec()); + if (res_type.is_error()) { + return result; + } + typify_invoke<1,TypifyCellType,CopyCellsWithCast>(to, a, result); + return result; +} TensorSpec ReferenceOperations::concat(const TensorSpec &a, const TensorSpec &b, const std::string &concat_dim) { ValueType a_type = ValueType::from_spec(a.type()); diff --git a/eval/src/vespa/eval/eval/test/reference_operations.h b/eval/src/vespa/eval/eval/test/reference_operations.h index 588215853f9..02651e3971c 100644 --- a/eval/src/vespa/eval/eval/test/reference_operations.h +++ b/eval/src/vespa/eval/eval/test/reference_operations.h @@ -28,6 +28,7 @@ struct ReferenceOperations { // start at 1. using PeekSpec = tensor_function::Peek::Spec; + static TensorSpec cell_cast(const TensorSpec &a, CellType to); static TensorSpec concat(const TensorSpec &a, const TensorSpec &b, const std::string &concat_dim); static TensorSpec create(const vespalib::string &type, const CreateSpec &spec, const std::vector<TensorSpec> &children); static TensorSpec join(const TensorSpec &a, const TensorSpec &b, join_fun_t function); diff --git a/eval/src/vespa/eval/eval/value_type.cpp b/eval/src/vespa/eval/eval/value_type.cpp index a5960a8de4b..57b101f6eeb 100644 --- a/eval/src/vespa/eval/eval/value_type.cpp +++ b/eval/src/vespa/eval/eval/value_type.cpp @@ -388,6 +388,11 @@ ValueType::either(const ValueType &one, const ValueType &other) { return one; } +ValueType +ValueType::cell_cast(const ValueType &from, CellType to_cell_type) { + return make_type(to_cell_type, from.dimensions()); +} + std::ostream & operator<<(std::ostream &os, const ValueType &type) { return os << type.to_spec(); diff --git a/eval/src/vespa/eval/eval/value_type.h b/eval/src/vespa/eval/eval/value_type.h index 6d9316e76ed..4609f6a0b38 100644 --- a/eval/src/vespa/eval/eval/value_type.h +++ b/eval/src/vespa/eval/eval/value_type.h @@ -105,6 +105,8 @@ public: static CellType unify_cell_types(const ValueType &a, const ValueType &b); static ValueType concat(const ValueType &lhs, const ValueType &rhs, const vespalib::string &dimension); static ValueType either(const ValueType &one, const ValueType &other); + static ValueType cell_cast(const ValueType &from, CellType to_cell_type); + }; std::ostream &operator<<(std::ostream &os, const ValueType &type); diff --git a/eval/src/vespa/eval/instruction/CMakeLists.txt b/eval/src/vespa/eval/instruction/CMakeLists.txt index 97838f2adb9..7a299738e78 100644 --- a/eval/src/vespa/eval/instruction/CMakeLists.txt +++ b/eval/src/vespa/eval/instruction/CMakeLists.txt @@ -15,6 +15,7 @@ vespa_add_library(eval_instruction OBJECT dense_tensor_peek_function.cpp dense_xw_product_function.cpp fast_rename_optimizer.cpp + generic_cell_cast.cpp generic_concat.cpp generic_create.cpp generic_join.cpp diff --git a/eval/src/vespa/eval/instruction/generic_cell_cast.cpp b/eval/src/vespa/eval/instruction/generic_cell_cast.cpp new file mode 100644 index 00000000000..58221b9c62f --- /dev/null +++ b/eval/src/vespa/eval/instruction/generic_cell_cast.cpp @@ -0,0 +1,56 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "generic_cell_cast.h" +#include <vespa/eval/eval/value.h> +#include <vespa/eval/eval/wrap_param.h> +#include <vespa/vespalib/util/stash.h> +#include <vespa/vespalib/util/typify.h> +#include <cassert> + +using namespace vespalib::eval::tensor_function; + +namespace vespalib::eval::instruction { + +using State = InterpretedFunction::State; +using Instruction = InterpretedFunction::Instruction; + +namespace { + +template <typename ICT, typename OCT> +void my_generic_cell_cast_op(State &state, uint64_t param_in) { + const auto &res_type = unwrap_param<ValueType>(param_in); + const Value &a = state.peek(0); + auto input_cells = a.cells().typify<ICT>(); + auto output_cells = state.stash.create_uninitialized_array<OCT>(input_cells.size()); + auto pos = output_cells.begin(); + for (ICT value : input_cells) { + *pos++ = (OCT) value; + } + assert(pos == output_cells.end()); + Value &result_ref = state.stash.create<ValueView>(res_type, a.index(), TypedCells(output_cells)); + state.pop_push(result_ref); +} + +struct SelectGenericCellCastOp { + template <typename ICT, typename OCT> + static auto invoke() { + return my_generic_cell_cast_op<ICT, OCT>; + } +}; + +} // namespace <unnamed> + +InterpretedFunction::Instruction +GenericCellCast::make_instruction(const ValueType &input_type, + CellType to_cell_type, + Stash &stash) +{ + CellType from = input_type.cell_type(); + auto result_type = ValueType::cell_cast(input_type, to_cell_type); + auto ¶m = stash.create<ValueType>(result_type); + CellType to = result_type.cell_type(); + auto op = typify_invoke<2,TypifyCellType,SelectGenericCellCastOp>(from, to); + return Instruction(op, wrap_param<ValueType>(param)); +} + +} // namespace diff --git a/eval/src/vespa/eval/instruction/generic_cell_cast.h b/eval/src/vespa/eval/instruction/generic_cell_cast.h new file mode 100644 index 00000000000..cb911304a76 --- /dev/null +++ b/eval/src/vespa/eval/instruction/generic_cell_cast.h @@ -0,0 +1,18 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/eval/eval/value_type.h> +#include <vespa/eval/eval/interpreted_function.h> + +namespace vespalib::eval { struct ValueBuilderFactory; } + +namespace vespalib::eval::instruction { + +struct GenericCellCast { + static InterpretedFunction::Instruction + make_instruction(const ValueType &input_type, CellType to_cell_type, + Stash &stash); +}; + +} // namespace diff --git a/eval/src/vespa/eval/instruction/generic_rename.cpp b/eval/src/vespa/eval/instruction/generic_rename.cpp index 4fe347375c4..8363d4db573 100644 --- a/eval/src/vespa/eval/instruction/generic_rename.cpp +++ b/eval/src/vespa/eval/instruction/generic_rename.cpp @@ -69,10 +69,10 @@ generic_rename(const Value &a, const ValueType &res_type, const ValueBuilderFactory &factory) { auto cells = a.cells().typify<CT>(); - std::vector<string_id> output_address(sparse_plan.mapped_dims); - std::vector<string_id*> input_address; + SmallVector<string_id> output_address(sparse_plan.mapped_dims); + SmallVector<string_id*> input_address; for (size_t maps_to : sparse_plan.output_dimensions) { - input_address.push_back(&output_address[maps_to]); + input_address.emplace_back(&output_address[maps_to]); } auto builder = factory.create_transient_value_builder<CT>(res_type, sparse_plan.mapped_dims, @@ -152,7 +152,7 @@ SparseRenamePlan::SparseRenamePlan(const ValueType &input_type, if (index != output_dimensions.size()) { can_forward_index = false; } - output_dimensions.push_back(index); + output_dimensions.emplace_back(index); } assert(output_dimensions.size() == mapped_dims); } @@ -172,8 +172,8 @@ DenseRenamePlan::DenseRenamePlan(const ValueType &lhs_type, const auto out_dims = output_type.nontrivial_indexed_dimensions(); size_t num_dense_dims = lhs_dims.size(); assert(num_dense_dims == out_dims.size()); - std::vector<size_t> lhs_loopcnt(num_dense_dims); - std::vector<size_t> lhs_stride(num_dense_dims, 1); + SmallVector<size_t> lhs_loopcnt(num_dense_dims); + SmallVector<size_t> lhs_stride(num_dense_dims, 1); size_t lhs_size = 1; for (size_t i = num_dense_dims; i-- > 0; ) { lhs_stride[i] = lhs_size; @@ -191,8 +191,8 @@ DenseRenamePlan::DenseRenamePlan(const ValueType &lhs_type, loop_cnt.back() *= lhs_loopcnt[index]; stride.back() = lhs_stride[index]; } else { - loop_cnt.push_back(lhs_loopcnt[index]); - stride.push_back(lhs_stride[index]); + loop_cnt.emplace_back(lhs_loopcnt[index]); + stride.emplace_back(lhs_stride[index]); } prev_index = index; } diff --git a/eval/src/vespa/eval/instruction/generic_rename.h b/eval/src/vespa/eval/instruction/generic_rename.h index 7834c967488..4d8ca54a248 100644 --- a/eval/src/vespa/eval/instruction/generic_rename.h +++ b/eval/src/vespa/eval/instruction/generic_rename.h @@ -6,6 +6,7 @@ #include <vespa/eval/eval/value_type.h> #include <vespa/eval/eval/interpreted_function.h> #include <vespa/vespalib/stllike/string.h> +#include <vespa/vespalib/util/small_vector.h> #include <vector> namespace vespalib::eval { struct ValueBuilderFactory; } @@ -13,8 +14,8 @@ namespace vespalib::eval { struct ValueBuilderFactory; } namespace vespalib::eval::instruction { struct DenseRenamePlan { - std::vector<size_t> loop_cnt; - std::vector<size_t> stride; + SmallVector<size_t> loop_cnt; + SmallVector<size_t> stride; const size_t subspace_size; DenseRenamePlan(const ValueType &lhs_type, const ValueType &output_type, @@ -28,7 +29,7 @@ struct DenseRenamePlan { struct SparseRenamePlan { size_t mapped_dims; - std::vector<size_t> output_dimensions; + SmallVector<size_t> output_dimensions; bool can_forward_index; SparseRenamePlan(const ValueType &input_type, const ValueType &output_type, 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 62e5c34d5ee..e53195e3a71 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -239,6 +239,12 @@ public class Flags { "JVM max heap size for cluster controller in Mb", "Takes effect when restarting cluster controller"); + public static final UnboundIntFlag METRICS_PROXY_MAX_HEAP_SIZE_IN_MB = defineIntFlag( + "metrics-proxy-max-heap-size-in-mb", 512, + List.of("hmusum"), "2021-03-01", "2021-05-01", + "JVM max heap size for metrics proxy in Mb", + "Takes effect when restarting metrics proxy"); + public static final UnboundBooleanFlag DEDICATED_CLUSTER_CONTROLLER_CLUSTER = defineFeatureFlag( "dedicated-cluster-controller-cluster", false, List.of("jonmv"), "2021-02-15", "2021-04-15", @@ -273,12 +279,6 @@ 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"), "2021-02-26", "2021-05-01", - "Enable dynamic provisioning of config servers", - "Takes effect immediately"); - /** 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/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index 622299a27d3..c00525a3ddc 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -188,9 +188,9 @@ public class HttpServerTest { binder -> binder.bind(RequestLog.class).toInstance(requestLogMock)); driver.client().get("/status.html") .expectStatusCode(is(REQUEST_URI_TOO_LONG)); - assertThat(driver.close(), is(true)); RequestLogEntry entry = requestLogMock.poll(Duration.ofSeconds(30)); assertEquals(414, entry.statusCode().getAsInt()); + assertThat(driver.close(), is(true)); } @Test diff --git a/node-admin/pom.xml b/node-admin/pom.xml index ad41ac63915..52873501744 100644 --- a/node-admin/pom.xml +++ b/node-admin/pom.xml @@ -149,12 +149,6 @@ <version>${project.version}</version> <scope>test</scope> </dependency> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>zookeeper-server-common</artifactId> - <version>${project.version}</version> - <scope>test</scope> - </dependency> </dependencies> <build> <plugins> diff --git a/node-repository/pom.xml b/node-repository/pom.xml index 8e2a7ca0627..fb46735ec73 100644 --- a/node-repository/pom.xml +++ b/node-repository/pom.xml @@ -83,12 +83,6 @@ <version>${project.version}</version> <scope>provided</scope> </dependency> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>zookeeper-server-common</artifactId> - <version>${project.version}</version> - <scope>provided</scope> - </dependency> <!-- compile --> <dependency> diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ConfigServerReconfigurer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ConfigServerReconfigurer.java deleted file mode 100644 index 97113d663d4..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ConfigServerReconfigurer.java +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.maintenance; - -import com.yahoo.config.provision.NodeType; -import com.yahoo.jdisc.Metric; -import com.yahoo.vespa.flags.BooleanFlag; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.NodeList; -import com.yahoo.vespa.hosted.provision.NodeRepository; -import com.yahoo.vespa.zookeeper.Reconfigurer; -import com.yahoo.vespa.zookeeper.ZooKeeperServer; - -import java.time.Duration; -import java.util.List; -import java.util.stream.Collectors; - -/** - * Reconfigure members of the config server ZooKeeper cluster, according to the config servers currently active in the - * node repository. - * - * @author mpolden - */ -public class ConfigServerReconfigurer extends NodeRepositoryMaintainer { - - /** Minimum number of config servers required before attempting reconfiguration */ - private static final int MIN_ACTIVE_NODES = 3; - - private final Reconfigurer reconfigurer; - private final BooleanFlag featureFlag; - - public ConfigServerReconfigurer(NodeRepository nodeRepository, Duration interval, Metric metric, Reconfigurer reconfigurer) { - super(nodeRepository, interval, metric); - this.reconfigurer = reconfigurer; - this.featureFlag = Flags.DYNAMIC_CONFIG_SERVER_PROVISIONING.bindTo(nodeRepository.flagSource()); - } - - @Override - protected boolean maintain() { - if (!nodeRepository().zone().getCloud().dynamicProvisioning()) return true; - if (!featureFlag.value()) return true; - - NodeList configNodes = nodeRepository().nodes().list(Node.State.active) - .nodeType(NodeType.config); - if (configNodes.size() < MIN_ACTIVE_NODES) return true; - List<ZooKeeperServer> servers = configNodes.stream() - .map(node -> new ZooKeeperServer(node.allocation().get().membership().index(), node.hostname())) - .collect(Collectors.toList()); - reconfigurer.reconfigure(servers); - return true; - } - -} 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 01bbaffa1ed..8929d7f9939 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 @@ -17,7 +17,6 @@ import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; import com.yahoo.vespa.hosted.provision.provisioning.ProvisionServiceProvider; import com.yahoo.vespa.orchestrator.Orchestrator; import com.yahoo.vespa.service.monitor.ServiceMonitor; -import com.yahoo.vespa.zookeeper.Reconfigurer; import java.time.Duration; import java.util.List; @@ -39,8 +38,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { HostLivenessTracker hostLivenessTracker, ServiceMonitor serviceMonitor, Zone zone, Orchestrator orchestrator, Metric metric, ProvisionServiceProvider provisionServiceProvider, FlagSource flagSource, - MetricsFetcher metricsFetcher, MetricsDb metricsDb, - Reconfigurer reconfigurer) { + MetricsFetcher metricsFetcher, MetricsDb metricsDb) { DefaultTimes defaults = new DefaultTimes(zone, deployer); PeriodicApplicationMaintainer periodicApplicationMaintainer = new PeriodicApplicationMaintainer(deployer, metric, nodeRepository, defaults.redeployMaintainerInterval, @@ -67,7 +65,6 @@ 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)); - maintainers.add(new ConfigServerReconfigurer(nodeRepository, defaults.configServerReconfigurerInterval, metric, reconfigurer)); 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)); @@ -120,7 +117,6 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final Duration autoscalingInterval; private final Duration scalingSuggestionsInterval; private final Duration switchRebalancerInterval; - private final Duration configServerReconfigurerInterval; private final Duration dedicatedClusterControllerMigratorInterval; private final NodeFailer.ThrottlePolicy throttlePolicy; @@ -148,7 +144,6 @@ public class NodeRepositoryMaintenance extends AbstractComponent { scalingSuggestionsInterval = Duration.ofMinutes(31); spareCapacityMaintenanceInterval = Duration.ofMinutes(30); switchRebalancerInterval = Duration.ofHours(1); - configServerReconfigurerInterval = Duration.ofSeconds(90); throttlePolicy = NodeFailer.ThrottlePolicy.hosted; retiredExpiry = Duration.ofDays(4); // give up migrating data after 4 days dedicatedClusterControllerMigratorInterval = zone.environment() == Environment.staging || zone.system().isCd() ? Duration.ofMinutes(3) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/ContainerConfig.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/ContainerConfig.java index e1850b03944..5e40c0bd9ff 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/ContainerConfig.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/ContainerConfig.java @@ -29,7 +29,6 @@ public class ContainerConfig { " <component id='com.yahoo.vespa.hosted.provision.testutils.MockMetricsFetcher'/>\n" + " <component id='com.yahoo.vespa.hosted.provision.testutils.MockNodeRepository'/>\n" + " <component id='com.yahoo.vespa.hosted.provision.testutils.MockProvisionServiceProvider'/>\n" + - " <component id='com.yahoo.vespa.hosted.provision.testutils.MockReconfigurer'/>\n" + " <component id='com.yahoo.vespa.hosted.provision.maintenance.NodeRepositoryMaintenance'/>\n" + " <component id='com.yahoo.vespa.flags.InMemoryFlagSource'/>\n" + " <component id='com.yahoo.config.provision.Zone'/>\n" + diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockReconfigurer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockReconfigurer.java deleted file mode 100644 index 0b18c144621..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockReconfigurer.java +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.testutils; - -import com.yahoo.vespa.zookeeper.ReconfigException; -import com.yahoo.vespa.zookeeper.Reconfigurer; -import com.yahoo.vespa.zookeeper.Sleeper; -import com.yahoo.vespa.zookeeper.VespaZooKeeperAdmin; -import com.yahoo.vespa.zookeeper.ZooKeeperServer; - -import java.time.Duration; -import java.util.Comparator; -import java.util.List; -import java.util.stream.Collectors; - -/** - * @author mpolden - */ -public class MockReconfigurer extends Reconfigurer { - - private List<ZooKeeperServer> servers = List.of(); - private int reconfigurations = 0; - - public MockReconfigurer() { - super(new MockVespaZooKeperAdmin(), new Sleeper() { - @Override - public void sleep(Duration duration) { - // Ignored - } - }); - } - - @Override - public void reconfigure(List<ZooKeeperServer> wantedServers) { - servers = wantedServers.stream() - .sorted(Comparator.comparing(ZooKeeperServer::id)) - .collect(Collectors.toUnmodifiableList()); - reconfigurations++; - } - - public List<ZooKeeperServer> servers() { - return servers; - } - - public int reconfigurations() { - return reconfigurations; - } - - private static class MockVespaZooKeperAdmin implements VespaZooKeeperAdmin { - - @Override - public void reconfigure(String connectionSpec, String joiningServers, String leavingServers) throws ReconfigException { - } - - } - -} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ConfigServerReconfigurerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ConfigServerReconfigurerTest.java deleted file mode 100644 index 1b7d90e426b..00000000000 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ConfigServerReconfigurerTest.java +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.maintenance; - -import com.yahoo.component.Version; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.Cloud; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.NodeResources; -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.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.NodeList; -import com.yahoo.vespa.hosted.provision.node.Agent; -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 com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; -import com.yahoo.vespa.hosted.provision.testutils.MockReconfigurer; -import com.yahoo.vespa.zookeeper.ZooKeeperServer; -import org.junit.Test; - -import java.time.Duration; -import java.util.List; -import java.util.Set; - -import static org.junit.Assert.assertEquals; - -/** - * @author mpolden - */ -public class ConfigServerReconfigurerTest { - - @Test - public void maintain() { - InMemoryFlagSource flagSource = new InMemoryFlagSource(); - flagSource.withBooleanFlag(Flags.DYNAMIC_CONFIG_SERVER_PROVISIONING.id(), true); - ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Cloud.builder().dynamicProvisioning(true).build(), - SystemName.defaultSystem(), - Environment.defaultEnvironment(), - RegionName.defaultName())) - .hostProvisioner(new MockHostProvisioner()) - .flagSource(flagSource) - .build(); - MockReconfigurer reconfigurer = new MockReconfigurer(); - ConfigServerReconfigurer maintainer = new ConfigServerReconfigurer(tester.nodeRepository(), Duration.ofDays(1), - new TestMetric(), reconfigurer); - - // Initially there are not enough config servers to trigger reconfiguration - tester.makeConfigServers(2, 1, "default"); - maintainer.maintain(); - assertEquals("No change: Too few servers", List.of(), reconfigurer.servers()); - - // Another is added, triggering reconfiguration - NodeList configServer = tester.makeConfigServers(1, 3, "default"); - maintainer.maintain(); - List<ZooKeeperServer> configuredServers = List.of(new ZooKeeperServer(0, "cfg1"), - new ZooKeeperServer(1, "cfg2"), - new ZooKeeperServer(2, "cfg3")); - assertEquals("Reconfigured", configuredServers, reconfigurer.servers()); - assertEquals(1, reconfigurer.reconfigurations()); - - // A config server is deallocated, no longer enough active nodes to reconfigure - tester.nodeRepository().nodes().deallocate(configServer.first().get(), Agent.system, this.getClass().getSimpleName()); - maintainer.maintain(); - assertEquals("No change: Too few active servers", configuredServers, reconfigurer.servers()); - assertEquals(1, reconfigurer.reconfigurations()); - } - - private static class MockHostProvisioner implements HostProvisioner { - - @Override - public List<ProvisionedHost> provisionHosts(List<Integer> provisionIndexes, NodeResources resources, ApplicationId applicationId, Version osVersion, HostSharing sharing) { - return List.of(); - } - - @Override - public List<Node> provision(Node host, Set<Node> children) throws FatalProvisioningException { - return List.of(); - } - - @Override - public void deprovision(Node host) { - } - - } - -} 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 6ebdfe984a0..23f504a9c0f 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 @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.provision.provisioning; import com.yahoo.component.Version; -import com.yahoo.component.Vtag; import com.yahoo.config.provision.ActivationContext; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; @@ -442,19 +441,11 @@ public class ProvisioningTester { return nodes; } - public NodeList makeConfigServers(int n, int startIndex, String flavor) { - return makeConfigServers(n, startIndex, flavor, Vtag.currentVersion); - } - - public NodeList makeConfigServers(int n, String flavor, Version version) { - return makeConfigServers(n, 1, flavor, version); - } - - public NodeList makeConfigServers(int n, int startIndex, String flavor, Version version) { + public NodeList makeConfigServers(int n, String flavor, Version configServersVersion) { List<Node> nodes = new ArrayList<>(n); MockNameResolver nameResolver = (MockNameResolver)nodeRepository().nameResolver(); - for (int i = startIndex; i < startIndex + n; i++) { + for (int i = 1; i <= n; i++) { String hostname = "cfg" + i; String ipv4 = "127.0.1." + i; @@ -470,7 +461,7 @@ public class ProvisioningTester { ConfigServerApplication application = new ConfigServerApplication(); List<HostSpec> hosts = prepare(application.getApplicationId(), - application.getClusterSpecWithVersion(version), + application.getClusterSpecWithVersion(configServersVersion), application.getCapacity()); activate(application.getApplicationId(), new HashSet<>(hosts)); return nodeRepository.nodes().list(Node.State.active).owner(application.getApplicationId()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json index d788b321af9..bd4029ec0c0 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json @@ -4,9 +4,6 @@ "name": "AutoscalingMaintainer" }, { - "name": "ConfigServerReconfigurer" - }, - { "name": "DedicatedClusterControllerClusterMigrator" }, { diff --git a/vespalib/src/tests/small_vector/small_vector_test.cpp b/vespalib/src/tests/small_vector/small_vector_test.cpp index 808571acef6..6d8238fefb4 100644 --- a/vespalib/src/tests/small_vector/small_vector_test.cpp +++ b/vespalib/src/tests/small_vector/small_vector_test.cpp @@ -243,4 +243,24 @@ TEST(SmallVectorTest, equal_operator) { SmallVector<EqOnly>({EqOnly{1},EqOnly{5},EqOnly{3}})); } +// to check "back() const" +template<size_t N> +int last_value_of(const SmallVector<int,N> &v) { + return v.back(); +} + +TEST(SmallVectorTest, check_back_method) { + SmallVector<int> vec; + for (int i = 0; i < 1000; ++i) { + vec.emplace_back(17); + EXPECT_EQ(vec.back(), 17); + EXPECT_EQ(last_value_of(vec), 17); + vec.back() = 42; + EXPECT_EQ(vec[i], 42); + vec.back() = i; + EXPECT_EQ(last_value_of(vec), i); + } + EXPECT_EQ(&vec.back(), vec.end() - 1); +} + GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/vespa/vespalib/util/arrayref.h b/vespalib/src/vespa/vespalib/util/arrayref.h index 03634a7a094..2433f9251cf 100644 --- a/vespalib/src/vespa/vespalib/util/arrayref.h +++ b/vespalib/src/vespa/vespalib/util/arrayref.h @@ -2,6 +2,7 @@ #pragma once #include "array.h" +#include "small_vector.h" #include <vector> namespace vespalib { @@ -17,6 +18,8 @@ public: ArrayRef(T * v, size_t sz) noexcept : _v(v), _sz(sz) { } template<typename A=std::allocator<T>> ArrayRef(std::vector<T, A> & v) noexcept : _v(&v[0]), _sz(v.size()) { } + template<size_t N> + ArrayRef(SmallVector<T, N> &v) noexcept : _v(&v[0]), _sz(v.size()) { } ArrayRef(Array<T> &v) noexcept : _v(&v[0]), _sz(v.size()) { } T & operator [] (size_t i) { return _v[i]; } const T & operator [] (size_t i) const { return _v[i]; } @@ -35,6 +38,8 @@ public: ConstArrayRef(const T *v, size_t sz) noexcept : _v(v), _sz(sz) { } template<typename A=std::allocator<T>> ConstArrayRef(const std::vector<T, A> & v) noexcept : _v(&v[0]), _sz(v.size()) { } + template<size_t N> + ConstArrayRef(const SmallVector<T, N> &v) noexcept : _v(&v[0]), _sz(v.size()) { } ConstArrayRef(const ArrayRef<T> & v) noexcept : _v(&v[0]), _sz(v.size()) { } ConstArrayRef(const Array<T> &v) noexcept : _v(&v[0]), _sz(v.size()) { } ConstArrayRef() noexcept : _v(nullptr), _sz(0) {} diff --git a/vespalib/src/vespa/vespalib/util/small_vector.h b/vespalib/src/vespa/vespalib/util/small_vector.h index 21a21fc2cbb..9cbc10951cf 100644 --- a/vespalib/src/vespa/vespalib/util/small_vector.h +++ b/vespalib/src/vespa/vespalib/util/small_vector.h @@ -186,6 +186,8 @@ public: const T *end() const { return (_data + _size); } T &operator[](size_t idx) { return _data[idx]; } const T &operator[](size_t idx) const { return _data[idx]; } + T &back() { return _data[_size - 1]; } + const T &back() const { return _data[_size - 1]; } void clear() { small_vector::destroy_objects(_data, _size); _size = 0; diff --git a/zookeeper-server/zookeeper-server-3.6.2/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java b/zookeeper-server/zookeeper-server-3.6.2/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java index 57ae62c0ebc..8ae30942d55 100644 --- a/zookeeper-server/zookeeper-server-3.6.2/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java +++ b/zookeeper-server/zookeeper-server-3.6.2/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java @@ -19,10 +19,9 @@ public class VespaZooKeeperAdminImpl implements VespaZooKeeperAdmin { @Override public void reconfigure(String connectionSpec, String joiningServers, String leavingServers) throws ReconfigException { + ZooKeeperAdmin zooKeeperAdmin = null; try { - ZooKeeperAdmin zooKeeperAdmin = new ZooKeeperAdmin(connectionSpec, - (int) sessionTimeout().toMillis(), - (event) -> log.log(Level.INFO, event.toString())); + zooKeeperAdmin = createAdmin(connectionSpec); long fromConfig = -1; // Using string parameters because the List variant of reconfigure fails to join empty lists (observed on 3.5.6, fixed in 3.7.0) byte[] appliedConfig = zooKeeperAdmin.reconfigure(joiningServers, leavingServers, null, fromConfig, null); @@ -34,9 +33,21 @@ public class VespaZooKeeperAdminImpl implements VespaZooKeeperAdmin { throw new RuntimeException(e); } catch (IOException | InterruptedException e) { throw new RuntimeException(e); + } finally { + if (zooKeeperAdmin != null) { + try { + zooKeeperAdmin.close(); + } catch (InterruptedException e) { + } + } } } + private ZooKeeperAdmin createAdmin(String connectionSpec) throws IOException { + return new ZooKeeperAdmin(connectionSpec, (int) sessionTimeout().toMillis(), + (event) -> log.log(Level.INFO, event.toString())); + } + private static boolean retryOn(KeeperException e) { return e instanceof KeeperException.ReconfigInProgress || e instanceof KeeperException.ConnectionLossException || diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java index 1d47d890421..9c2a543d2ef 100644 --- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java +++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java @@ -20,7 +20,7 @@ import java.util.logging.Logger; import java.util.stream.Collectors; /** - * Starts or reconfigures a ZooKeeper server as necessary. This is created as a component + * Starts zookeeper server and supports reconfiguring zookeeper cluster. Created as a component * without any config injected, to make sure that it is not recreated when config changes. * * @author hmusum @@ -45,13 +45,12 @@ public class Reconfigurer extends AbstractComponent { this(vespaZooKeeperAdmin, new Sleeper()); } - public Reconfigurer(VespaZooKeeperAdmin vespaZooKeeperAdmin, Sleeper sleeper) { + Reconfigurer(VespaZooKeeperAdmin vespaZooKeeperAdmin, Sleeper sleeper) { this.vespaZooKeeperAdmin = Objects.requireNonNull(vespaZooKeeperAdmin); this.sleeper = Objects.requireNonNull(sleeper); log.log(Level.FINE, "Created ZooKeeperReconfigurer"); } - /** Start a ZooKeeper server or reconfigure a currently running cluster */ void startOrReconfigure(ZookeeperServerConfig newConfig, VespaZooKeeperServer server, Supplier<QuorumPeer> quorumPeerGetter, Consumer<QuorumPeer> quorumPeerSetter) { if (zooKeeperRunner == null) { @@ -59,22 +58,10 @@ public class Reconfigurer extends AbstractComponent { zooKeeperRunner = startServer(newConfig, server); } quorumPeerSetter.accept(peer); - reconfigure(newConfig); - } - /** Reconfigure a running ZooKeeper cluster with given servers. This is a no-op if servers are unchanged */ - public void reconfigure(List<ZooKeeperServer> wantedServers) { - ZookeeperServerConfig.Builder b = new ZookeeperServerConfig.Builder(); - b.myid(-1) // Required by ZookeeperServerConfig, but not used for reconfiguration - .dynamicReconfiguration(true); - for (var server : wantedServers) { - ZookeeperServerConfig.Server.Builder serverBuilder = new ZookeeperServerConfig.Server.Builder(); - serverBuilder.id(server.id()) - .hostname(server.hostname()); - b.server(serverBuilder); + if (shouldReconfigure(newConfig)) { + reconfigure(newConfig); } - ZookeeperServerConfig newConfig = b.build(); - reconfigure(newConfig); } ZookeeperServerConfig activeConfig() { @@ -90,7 +77,6 @@ public class Reconfigurer extends AbstractComponent { private boolean shouldReconfigure(ZookeeperServerConfig newConfig) { if (!newConfig.dynamicReconfiguration()) return false; if (activeConfig == null) return false; - if (newConfig.server().isEmpty()) return false; return !newConfig.equals(activeConfig()); } @@ -100,9 +86,7 @@ public class Reconfigurer extends AbstractComponent { return runner; } - /** Reconfigure ZooKeeper cluster with given config, if necessary */ private void reconfigure(ZookeeperServerConfig newConfig) { - if (!shouldReconfigure(newConfig)) return; Instant reconfigTriggered = Instant.now(); // No point in trying to reconfigure if there is only one server in the new ensemble, // the others will be shutdown or are about to be shutdown diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java deleted file mode 100644 index ef75d0b5da7..00000000000 --- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.zookeeper; - -import java.util.Objects; - -/** - * A ZooKeeper server and its ID. - * - * @author mpolden - */ -public class ZooKeeperServer { - - private final int id; - private final String hostname; - - public ZooKeeperServer(int id, String hostname) { - if (id < 0 || id > 255) throw new IllegalArgumentException("server id must be between 0 and 255"); - this.id = id; - this.hostname = Objects.requireNonNull(hostname); - } - - public int id() { - return id; - } - - public String hostname() { - return hostname; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ZooKeeperServer that = (ZooKeeperServer) o; - return id == that.id && hostname.equals(that.hostname); - } - - @Override - public int hashCode() { - return Objects.hash(id, hostname); - } - - @Override - public String toString() { - return "server " + id + "=" + hostname; - } -} diff --git a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java index 97102ed7f58..94ccbc26f03 100644 --- a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java +++ b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java @@ -14,7 +14,6 @@ import java.io.IOException; import java.nio.file.Path; import java.time.Duration; import java.util.Arrays; -import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -66,11 +65,6 @@ public class ReconfigurerTest { assertEquals(1, reconfigurer.reconfigurations()); assertSame(nextConfig, reconfigurer.activeConfig()); - // No reconfiguration happens with empty config - reconfigurer.startOrReconfigure(createConfig(0, true)); - assertEquals(1, reconfigurer.reconfigurations()); - assertSame(nextConfig, reconfigurer.activeConfig()); - // Cluster shrinks nextConfig = createConfig(3, true); reconfigurer.startOrReconfigure(nextConfig); @@ -87,14 +81,6 @@ public class ReconfigurerTest { assertEquals("1=node2:2182:2183;2181,2=node3:2182:2183;2181", reconfigurer.joiningServers()); assertEquals("1,2", reconfigurer.leavingServers()); assertSame(nextConfig, reconfigurer.activeConfig()); - - // Reconfigure without using config - reconfigurer.reconfigure(List.of(new ZooKeeperServer(0, "node0"), - new ZooKeeperServer(1, "node2"), - new ZooKeeperServer(3, "node4"))); - assertEquals(4, reconfigurer.reconfigurations()); - assertEquals("3=node4:2182:2183;2181", reconfigurer.joiningServers()); - assertEquals("2", reconfigurer.leavingServers()); } @Test |