Merge pull request #15837 from vespa-engine/hmusum/use-flag-to-choose-vespa-zookeeper-server-implementation

Use feature flag to decide how to run ZooKeeper server

9 files changed, 76 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..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
@@ -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;
     }
 
+    private 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..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;
@@ -22,6 +23,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 +32,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 +994,27 @@ 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()) {
+            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());
+        }
+    }
+
+    @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> V flagValue(FlagSource source, ApplicationId appId, UnboundFlag<? extends V, ?, ?> 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 98cccbc265e..f1eaf522308 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<String> owners,
                                                        String createdAt, String expiresAt, String description,