From 2d06a59a788b1f910e492a0aec8725a83d1f1411 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Wed, 16 Dec 2020 13:11:55 +0100 Subject: Use feature flag to decide how to run ZooKeeper server Add flag to choose between reconfigurable ZooKeeper server or not for the ZooKeeper server used by cluster controller --- .../com/yahoo/config/model/api/ModelContext.java | 3 ++- .../yahoo/config/model/deploy/TestProperties.java | 8 +++++++ .../ClusterControllerContainer.java | 25 +++++++++++++++------- .../model/builder/xml/dom/DomAdminV2Builder.java | 4 ++-- .../model/content/cluster/ContentCluster.java | 8 ++----- .../model/container/ContainerClusterTest.java | 15 +++++++------ .../vespa/model/content/ContentClusterTest.java | 20 ++++++++++++++++- .../config/server/deploy/ModelContextImpl.java | 3 +++ .../src/main/java/com/yahoo/vespa/flags/Flags.java | 9 +++++++- 9 files changed, 70 insertions(+), 25 deletions(-) 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 0de802e19d4..1cf698af9cb 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 @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.api; import com.yahoo.component.Version; @@ -81,6 +81,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"baldersheim"}) default int contentNodeBucketDBStripeBits() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default int mergeChunkSize() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default double feedConcurrency() { throw new UnsupportedOperationException("TODO specify default value"); } + @ModelFeatureFlag(owners = {"musum", "mpolden"}, comment = "Revisit in February 2021") default boolean reconfigurableZookeeperServer() { return false; } } /** Warning: As elsewhere in this package, do not make backwards incompatible changes that will break old config models! */ 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 d4802fd8f75..3e7017b78e1 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 @@ -54,6 +54,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private int mergeChunkSize = 0x400000 - 0x1000; // 4M -4k private double feedConcurrency = 0.5; private boolean enableAutomaticReindexing = false; + private boolean reconfigurableZookeeperServer = false; @Override public ModelContext.FeatureFlags featureFlags() { return this; } @Override public boolean multitenant() { return multitenant; } @@ -89,6 +90,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public int mergeChunkSize() { return mergeChunkSize; } @Override public double feedConcurrency() { return feedConcurrency; } @Override public boolean enableAutomaticReindexing() { return enableAutomaticReindexing; } + @Override public boolean reconfigurableZookeeperServer() { return reconfigurableZookeeperServer; } public TestProperties setFeedConcurrency(double feedConcurrency) { this.feedConcurrency = feedConcurrency; @@ -197,6 +199,12 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea public TestProperties enableAutomaticReindexing(boolean enabled) { this.enableAutomaticReindexing = enabled; return this; } + public TestProperties reconfigurableZookeeperServer(boolean enabled) { + this.reconfigurableZookeeperServer = enabled; + return this; + } + + public static class Spec implements ConfigServerSpec { private final String hostName; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java index b2aa1e6b704..10f9565e981 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java @@ -1,9 +1,10 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.admin.clustercontroller; import com.yahoo.cloud.config.ZookeeperServerConfig; import com.yahoo.component.ComponentSpecification; import com.yahoo.config.model.api.container.ContainerServiceType; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.container.core.documentapi.DocumentAccessProvider; @@ -43,9 +44,8 @@ public class ClusterControllerContainer extends Container implements AbstractConfigProducer parent, int index, boolean runStandaloneZooKeeper, - boolean isHosted) { - super(parent, "" + index, index, isHosted); - + DeployState deployState) { + super(parent, "" + index, index, deployState.isHosted()); addHandler("clustercontroller-status", "com.yahoo.vespa.clustercontroller.apps.clustercontroller.StatusHandler", "/clustercontroller-status/*", @@ -55,11 +55,11 @@ public class ClusterControllerContainer extends Container implements "/cluster/v2/*", CLUSTERCONTROLLER_BUNDLE); addComponent("clustercontroller-zookeeper-server", - runStandaloneZooKeeper - ? "com.yahoo.vespa.zookeeper.VespaZooKeeperServerImpl" - : "com.yahoo.vespa.zookeeper.DummyVespaZooKeeperServer", + zooKeeperServerImplementation(runStandaloneZooKeeper, deployState.featureFlags().reconfigurableZookeeperServer()), ZOOKEEPER_SERVER_BUNDLE); - addComponent(new AccessLogComponent(AccessLogComponent.AccessLogType.jsonAccessLog, "controller", isHosted)); + addComponent(new AccessLogComponent(AccessLogComponent.AccessLogType.jsonAccessLog, + "controller", + deployState.isHosted())); // TODO: Why are bundles added here instead of in the cluster? addFileBundle("clustercontroller-apps"); @@ -84,6 +84,15 @@ public class ClusterControllerContainer extends Container implements return ContainerServiceType.CLUSTERCONTROLLER_CONTAINER; } + public String zooKeeperServerImplementation(boolean runStandaloneZooKeeper, boolean reconfigurable) { + if (reconfigurable) + return "com.yahoo.vespa.zookeeper.ReconfigurableVespaZooKeeperServer"; + else + return runStandaloneZooKeeper + ? "com.yahoo.vespa.zookeeper.VespaZooKeeperServerImpl" + : "com.yahoo.vespa.zookeeper.DummyVespaZooKeeperServer"; + } + private void addHandler(Handler h, String path) { h.addServerBindings(SystemBindingPattern.fromHttpPath(path)); super.addHandler(h); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java index 3c72361d814..3879df52390 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.builder.xml.dom; import com.yahoo.config.application.api.FileRegistry; @@ -207,7 +207,7 @@ public class DomAdminV2Builder extends DomAdminBuilderBase { @Override protected ClusterControllerContainer doBuild(DeployState deployState, AbstractConfigProducer parent, Element spec) { - return new ClusterControllerContainer(parent, i, runStandaloneZooKeeper, deployState.isHosted()); + return new ClusterControllerContainer(parent, i, runStandaloneZooKeeper, deployState); } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java index cf9a567e409..7ec2dc67cf2 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.content.cluster; import com.google.common.base.Preconditions; @@ -465,11 +465,7 @@ public class ContentCluster extends AbstractConfigProducer implements int index = 0; for (HostResource host : hosts) { var clusterControllerContainer = - new ClusterControllerContainer( - clusterControllers, - index, - multitenant, - deployState.isHosted()); + new ClusterControllerContainer(clusterControllers, index, multitenant, deployState); clusterControllerContainer.setHostResource(host); clusterControllerContainer.initService(deployState.getDeployLogger()); clusterControllerContainer.setProp("clustertype", "admin") diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java index 4cee4517982..21db5bd2d4a 100755 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.container; import com.yahoo.cloud.config.ClusterInfoConfig; @@ -72,7 +72,7 @@ public class ContainerClusterTest { } @Test - public void requreThatWeCanGetTheZoneConfig() { + public void requireThatWeCanGetTheZoneConfig() { DeployState state = new DeployState.Builder().properties(new TestProperties().setHostedVespa(true)) .zone(new Zone(SystemName.cd, Environment.test, RegionName.from("some-region"))) .build(); @@ -166,7 +166,7 @@ public class ContainerClusterTest { public void testClusterControllerResourceUsage() { MockRoot root = createRoot(false); ClusterControllerContainerCluster cluster = createClusterControllerCluster(root); - addClusterController(root.deployLogger(), cluster, "host-c1"); + addClusterController(root.deployLogger(), cluster, "host-c1", root.getDeployState()); assertEquals(1, cluster.getContainers().size()); QrStartConfig.Builder qrBuilder = new QrStartConfig.Builder(); cluster.getConfig(qrBuilder); @@ -186,7 +186,7 @@ public class ContainerClusterTest { public void testThatLinguisticsIsExcludedForClusterControllerCluster() { MockRoot root = createRoot(false); ClusterControllerContainerCluster cluster = createClusterControllerCluster(root); - addClusterController(root.deployLogger(), cluster, "host-c1"); + addClusterController(root.deployLogger(), cluster, "host-c1", root.getDeployState()); assertFalse(contains("com.yahoo.language.provider.DefaultLinguisticsProvider", cluster.getAllComponents())); } @@ -391,8 +391,11 @@ public class ContainerClusterTest { cluster.addContainer(container); } - private static void addClusterController(DeployLogger deployLogger, ClusterControllerContainerCluster cluster, String hostName) { - ClusterControllerContainer container = new ClusterControllerContainer(cluster, 1, false, cluster.isHostedVespa()); + private static void addClusterController(DeployLogger deployLogger, + ClusterControllerContainerCluster cluster, + String hostName, + DeployState deployState) { + ClusterControllerContainer container = new ClusterControllerContainer(cluster, 1, false, deployState); container.setHostResource(new HostResource(new Host(null, hostName))); container.initService(deployLogger); cluster.addContainer(container); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java index fe0b9841d1c..d0231ee4278 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java @@ -22,6 +22,7 @@ import com.yahoo.vespa.config.content.core.StorServerConfig; import com.yahoo.vespa.config.search.DispatchConfig; import com.yahoo.vespa.config.search.core.ProtonConfig; import com.yahoo.vespa.model.VespaModel; +import com.yahoo.vespa.model.admin.clustercontroller.ClusterControllerContainer; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.content.cluster.ContentCluster; import com.yahoo.vespa.model.content.engines.ProtonEngine; @@ -30,7 +31,6 @@ import com.yahoo.vespa.model.content.utils.ContentClusterUtils; import com.yahoo.vespa.model.content.utils.SchemaBuilder; import com.yahoo.vespa.model.routing.DocumentProtocol; import com.yahoo.vespa.model.routing.Routing; -import com.yahoo.vespa.model.search.SearchNode; import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; import org.junit.Rule; @@ -993,4 +993,22 @@ public class ContentClusterTest extends ContentBaseTest { assertDirectStorageApiRpcFlagIsPropagatedToConfig(true); } + void assertZookeeperServerImplementation(boolean reconfigurable, String expectedClassName) { + VespaModel model = createEnd2EndOneNode( + new TestProperties() + .reconfigurableZookeeperServer(reconfigurable) + .setMultitenant(true)); + + ContentCluster cc = model.getContentClusters().get("storage"); + for (ClusterControllerContainer c : cc.getClusterControllers().getContainers()) { + assertEquals(expectedClassName, c.zooKeeperServerImplementation(true, reconfigurable)); + } + } + + @Test + public void reconfigurableZookeeperServerForClusterController() { + assertZookeeperServerImplementation(false, "com.yahoo.vespa.zookeeper.VespaZooKeeperServerImpl"); + assertZookeeperServerImplementation(true, "com.yahoo.vespa.zookeeper.ReconfigurableVespaZooKeeperServer"); + } + } 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 2d96c92bd88..14d156d39fb 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 @@ -165,6 +165,7 @@ public class ModelContextImpl implements ModelContext { private final int contentNodeBucketDBStripeBits; private final int mergeChunkSize; private final double feedConcurrency; + private final boolean reconfigurableZookeeperServer; public FeatureFlags(FlagSource source, ApplicationId appId) { this.enableAutomaticReindexing = flagValue(source, appId, Flags.ENABLE_AUTOMATIC_REINDEXING); @@ -184,6 +185,7 @@ public class ModelContextImpl implements ModelContext { this.contentNodeBucketDBStripeBits = flagValue(source, appId, Flags.CONTENT_NODE_BUCKET_DB_STRIPE_BITS); this.mergeChunkSize = flagValue(source, appId, Flags.MERGE_CHUNK_SIZE); this.feedConcurrency = flagValue(source, appId, Flags.FEED_CONCURRENCY); + this.reconfigurableZookeeperServer = flagValue(source, appId, Flags.RECONFIGURABLE_ZOOKEEPER_SERVER_FOR_CLUSTER_CONTROLLER); } @Override public boolean enableAutomaticReindexing() { return enableAutomaticReindexing; } @@ -203,6 +205,7 @@ public class ModelContextImpl implements ModelContext { @Override public int contentNodeBucketDBStripeBits() { return contentNodeBucketDBStripeBits; } @Override public int mergeChunkSize() { return mergeChunkSize; } @Override public double feedConcurrency() { return feedConcurrency; } + @Override public boolean reconfigurableZookeeperServer() { return reconfigurableZookeeperServer; } private static V flagValue(FlagSource source, ApplicationId appId, UnboundFlag flag) { return flag.bindTo(source) 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 a53c0bd6a78..8662dbc2bfe 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -1,4 +1,4 @@ -// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.flags; import com.yahoo.component.Vtag; @@ -286,6 +286,13 @@ public class Flags { "Takes effect on next internal redeployment", APPLICATION_ID); + public static final UnboundBooleanFlag RECONFIGURABLE_ZOOKEEPER_SERVER_FOR_CLUSTER_CONTROLLER = defineFeatureFlag( + "reconfigurable-zookeeper-server-for-cluster-controller", false, + List.of("musum", "mpolden"), "2020-12-16", "2021-02-16", + "Whether to use reconfigurable zookeeper server for cluster controller", + "Takes effect on (re)redeployment", + APPLICATION_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List owners, String createdAt, String expiresAt, String description, -- cgit v1.2.3 From 276121efb65afb8d5c6989929f197b136cd85761 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Wed, 16 Dec 2020 13:32:13 +0100 Subject: Verify config instead of depending on method in ClusterControllerContainer --- .../model/admin/clustercontroller/ClusterControllerContainer.java | 2 +- .../java/com/yahoo/vespa/model/content/ContentClusterTest.java | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java index 10f9565e981..d7223018b73 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java @@ -84,7 +84,7 @@ public class ClusterControllerContainer extends Container implements return ContainerServiceType.CLUSTERCONTROLLER_CONTAINER; } - public String zooKeeperServerImplementation(boolean runStandaloneZooKeeper, boolean reconfigurable) { + private String zooKeeperServerImplementation(boolean runStandaloneZooKeeper, boolean reconfigurable) { if (reconfigurable) return "com.yahoo.vespa.zookeeper.ReconfigurableVespaZooKeeperServer"; else diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java index d0231ee4278..a55d221f8c4 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java @@ -10,6 +10,7 @@ import com.yahoo.config.model.test.TestRoot; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; +import com.yahoo.container.ComponentsConfig; import com.yahoo.messagebus.routing.RoutingTableSpec; import com.yahoo.metrics.MetricsmanagerConfig; import com.yahoo.vespa.config.content.AllClustersBucketSpacesConfig; @@ -1001,7 +1002,12 @@ public class ContentClusterTest extends ContentBaseTest { ContentCluster cc = model.getContentClusters().get("storage"); for (ClusterControllerContainer c : cc.getClusterControllers().getContainers()) { - assertEquals(expectedClassName, c.zooKeeperServerImplementation(true, reconfigurable)); + var builder = new ComponentsConfig.Builder(); + c.getConfig(builder); + assertEquals(1, new ComponentsConfig(builder).components().stream() + .filter(component -> component.id().equals("clustercontroller-zookeeper-server")) + .map(component -> component.classId().equals(expectedClassName)) + .count()); } } -- cgit v1.2.3