Merge pull request #18999 from vespa-engine/arnej/add-slobrok-feature-flag

Arnej/add slobrok feature flag

9 files changed, 47 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 1c5e0a31ad9..92a394d7036 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
@@ -97,6 +97,7 @@ public interface ModelContext {
         @ModelFeatureFlag(owners = {"hmusum"}) default double resourceLimitDisk() { return 0.8; }
         @ModelFeatureFlag(owners = {"hmusum"}) default double resourceLimitMemory() { return 0.8; }
         @ModelFeatureFlag(owners = {"geirst", "vekterli"}) default double minNodeRatioPerGroup() { return 0.0; }
+        @ModelFeatureFlag(owners = {"arnej"}) default boolean newLocationBrokerLogic() { 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/vespa/model/admin/Admin.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java
index e7d15ad9742..01243a75aee 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
@@ -147,13 +147,13 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
 
     public ClusterControllerContainerCluster getClusterControllers() { return clusterControllers; }
 
-    public void setClusterControllers(ClusterControllerContainerCluster clusterControllers, DeployLogger deployLogger) {
+    public void setClusterControllers(ClusterControllerContainerCluster clusterControllers, DeployState deployState) {
         this.clusterControllers = clusterControllers;
         if (isHostedVespa) {
             // Prefer to put Slobroks on the admin cluster running cluster controllers to avoid unnecessary
             // movement of the slobroks when there are changes to the content cluster nodes
             removeSlobroks();
-            addSlobroks(createSlobroksOn(clusterControllers, deployLogger));
+            addSlobroks(createSlobroksOn(clusterControllers, deployState));
         }
     }
 
@@ -162,13 +162,13 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
         slobroks.clear();
     }
 
-    private List<Slobrok> createSlobroksOn(ClusterControllerContainerCluster clusterControllers, DeployLogger deployLogger) {
+    private List<Slobrok> createSlobroksOn(ClusterControllerContainerCluster clusterControllers, DeployState deployState) {
         List<Slobrok> slobroks = new ArrayList<>();
         for (ClusterControllerContainer clusterController : clusterControllers.getContainers()) {
-            Slobrok slobrok = new Slobrok(this, clusterController.index());
+            Slobrok slobrok = new Slobrok(this, clusterController.index(), deployState.featureFlags());
             slobrok.setHostResource(clusterController.getHostResource());
             slobroks.add(slobrok);
-            slobrok.initService(deployLogger);
+            slobrok.initService(deployState.getDeployLogger());
         }
         return slobroks;
     }
@@ -218,7 +218,7 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
      */
     public void addPerHostServices(List<HostResource> hosts, DeployState deployState) {
         if (slobroks.isEmpty()) // TODO: Move to caller
-            slobroks.addAll(createDefaultSlobrokSetup(deployState.getDeployLogger()));
+            slobroks.addAll(createDefaultSlobrokSetup(deployState));
 
         if (! deployState.isHosted() || ! deployState.getProperties().applicationId().instance().isTester())
             addMetricsProxyCluster(hosts, deployState);
@@ -295,12 +295,12 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
     }
 
     // If not configured by user: Use default setup: max 3 slobroks, 1 on the default configserver host
-    private List<Slobrok> createDefaultSlobrokSetup(DeployLogger deployLogger) {
+    private List<Slobrok> createDefaultSlobrokSetup(DeployState deployState) {
         List<HostResource> hosts = hostSystem().getHosts();
         List<Slobrok> slobs = new ArrayList<>();
         if (logserver != null) {
-            Slobrok slobrok = new Slobrok(this, 0);
-            addAndInitializeService(deployLogger, logserver.getHostResource(), slobrok);
+            Slobrok slobrok = new Slobrok(this, 0, deployState.featureFlags());
+            addAndInitializeService(deployState.getDeployLogger(), logserver.getHostResource(), slobrok);
             slobs.add(slobrok);
         }
 
@@ -308,8 +308,8 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
         while ((n < hosts.size()) && (slobs.size() < 3)) {
             HostResource host = hosts.get(n);
             if ((logserver== null || host != logserver.getHostResource()) && ! host.getHost().runsConfigServer()) {
-                Slobrok newSlobrok = new Slobrok(this, slobs.size());
-                addAndInitializeService(deployLogger, host, newSlobrok);
+                Slobrok newSlobrok = new Slobrok(this, slobs.size(), deployState.featureFlags());
+                addAndInitializeService(deployState.getDeployLogger(), host, newSlobrok);
                 slobs.add(newSlobrok);
             }
             n++;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java
index 00b6d872ddd..07b1de3fdad 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.admin;
 
+import com.yahoo.config.model.api.ModelContext;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.vespa.config.core.StateserverConfig;
 import com.yahoo.vespa.model.AbstractService;
@@ -16,6 +17,7 @@ public class Slobrok extends AbstractService implements StateserverConfig.Produc
     private static final long serialVersionUID = 1L;
 
     public final static int BASEPORT = 19099;
+    public final boolean useNewLogic;
 
     @Override
     public void getConfig(StateserverConfig.Builder builder) {
@@ -26,8 +28,11 @@ public class Slobrok extends AbstractService implements StateserverConfig.Produc
      * @param parent the parent ConfigProducer.
      * @param index  unique index for all slobroks
      */
-    public Slobrok(AbstractConfigProducer<?> parent, int index) {
+    public Slobrok(AbstractConfigProducer<?> parent, int index,
+                   ModelContext.FeatureFlags featureFlags)
+    {
         super(parent, "slobrok." + index);
+        this.useNewLogic = featureFlags.newLocationBrokerLogic();
         portsMeta.on(0).tag("rpc").tag("admin").tag("status");
         portsMeta.on(1).tag("http").tag("state");
         setProp("index", index);
@@ -45,6 +50,9 @@ public class Slobrok extends AbstractService implements StateserverConfig.Produc
     }
 
     public String getStartupCommand() {
+        if (useNewLogic) {
+            return "exec $ROOT/sbin/vespa-slobrok -N -p " + getRpcPort() + " -c " + getConfigId();
+        }
         return "exec $ROOT/sbin/vespa-slobrok -p " + getRpcPort() + " -c " + getConfigId();
     }
 
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 04c7db8e701..8e3451540f9 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
@@ -45,7 +45,7 @@ public class DomAdminV2Builder extends DomAdminBuilderBase {
         admin.addSlobroks(getSlobroks(deployState, admin, XML.getChild(adminE, "slobroks")));
         if ( ! admin.multitenant())
             admin.setClusterControllers(addConfiguredClusterControllers(deployState, admin, adminE),
-                                        deployState.getDeployLogger());
+                                        deployState);
 
         ModelElement adminElement = new ModelElement(adminE);
         addLogForwarders(adminElement.child("logforwarding"), admin);
@@ -188,7 +188,7 @@ public class DomAdminV2Builder extends DomAdminBuilderBase {
 
         @Override
         protected Slobrok doBuild(DeployState deployState, AbstractConfigProducer parent, Element spec) {
-            return new Slobrok(parent, i);
+            return new Slobrok(parent, i, deployState.featureFlags());
         }
 
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
index c836c5557ea..5b5d1b401aa 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
@@ -53,20 +53,20 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
         Optional<NodesSpecification> requestedLogservers = 
                 NodesSpecification.optionalDedicatedFromParent(adminElement.child("logservers"), context);
 
-        assignSlobroks(deployState.getDeployLogger(), requestedSlobroks.orElse(NodesSpecification.nonDedicated(3, context)), admin);
+        assignSlobroks(deployState, requestedSlobroks.orElse(NodesSpecification.nonDedicated(3, context)), admin);
         assignLogserver(deployState, requestedLogservers.orElse(createNodesSpecificationForLogserver()), admin);
 
         addLogForwarders(adminElement.child("logforwarding"), admin);
     }
 
-    private void assignSlobroks(DeployLogger deployLogger, NodesSpecification nodesSpecification, Admin admin) {
+    private void assignSlobroks(DeployState deployState, NodesSpecification nodesSpecification, Admin admin) {
         if (nodesSpecification.isDedicated()) {
-            createSlobroks(deployLogger,
+            createSlobroks(deployState,
                            admin,
                            allocateHosts(admin.hostSystem(), "slobroks", nodesSpecification));
         }
         else { // These will be removed later, if an admin cluster (for cluster controllers) is assigned
-            createSlobroks(deployLogger,
+            createSlobroks(deployState,
                            admin,
                            pickContainerHostsForSlobrok(nodesSpecification.minResources().nodes(), 2));
         }
@@ -177,15 +177,15 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
         return logserver;
     }
 
-    private void createSlobroks(DeployLogger deployLogger, Admin admin, Collection<HostResource> hosts) {
+    private void createSlobroks(DeployState deployState, Admin admin, Collection<HostResource> hosts) {
         if (hosts.isEmpty()) return; // No slobroks can be created (and none are needed)
         List<Slobrok> slobroks = new ArrayList<>();
         int index = 0;
         for (HostResource host : hosts) {
-            Slobrok slobrok = new Slobrok(admin, index++);
+            Slobrok slobrok = new Slobrok(admin, index++, deployState.featureFlags());
             slobrok.setHostResource(host);
             slobroks.add(slobrok);
-            slobrok.initService(deployLogger);
+            slobrok.initService(deployState.getDeployLogger());
         }
         admin.addSlobroks(slobroks);
     }
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 ad8948153cb..1f443da51db 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
@@ -296,7 +296,7 @@ public class ContentCluster extends AbstractConfigProducer<AbstractConfigProduce
                                                                          "cluster-controllers",
                                                                          true,
                                                                          deployState),
-                                                deployState.getDeployLogger());
+                                                deployState);
                 }
                 clusterControllers = admin.getClusterControllers();
             }
@@ -313,7 +313,7 @@ public class ContentCluster extends AbstractConfigProducer<AbstractConfigProduce
                                                                          "cluster-controllers",
                                                                          false,
                                                                          deployState),
-                                                deployState.getDeployLogger());
+                                                deployState);
                 }
                 clusterControllers = admin.getClusterControllers();
             }
@@ -347,7 +347,7 @@ public class ContentCluster extends AbstractConfigProducer<AbstractConfigProduce
                                                                      "cluster-controllers",
                                                                      true,
                                                                      context.getDeployState()),
-                                            deployState.getDeployLogger());
+                                            deployState);
             }
             return admin.getClusterControllers();
         }
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 49b5609d3df..90d2ad27cf5 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
@@ -184,6 +184,7 @@ public class ModelContextImpl implements ModelContext {
         private final double resourceLimitMemory;
         private final double minNodeRatioPerGroup;
         private final int metricsproxyNumThreads;
+        private final boolean newLocationBrokerLogic;
 
         public FeatureFlags(FlagSource source, ApplicationId appId) {
             this.defaultTermwiseLimit = flagValue(source, appId, Flags.DEFAULT_TERM_WISE_LIMIT);
@@ -213,6 +214,7 @@ public class ModelContextImpl implements ModelContext {
             this.resourceLimitMemory = flagValue(source, appId, PermanentFlags.RESOURCE_LIMIT_MEMORY);
             this.minNodeRatioPerGroup = flagValue(source, appId, Flags.MIN_NODE_RATIO_PER_GROUP);
             this.metricsproxyNumThreads = flagValue(source, appId, Flags.METRICSPROXY_NUM_THREADS);
+            this.newLocationBrokerLogic = flagValue(source, appId, Flags.NEW_LOCATION_BROKER_LOGIC);
         }
 
         @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; }
@@ -244,6 +246,7 @@ public class ModelContextImpl implements ModelContext {
         @Override public double resourceLimitMemory() { return resourceLimitMemory; }
         @Override public double minNodeRatioPerGroup() { return minNodeRatioPerGroup; }
         @Override public int metricsproxyNumThreads() { return metricsproxyNumThreads; }
+        @Override public boolean newLocationBrokerLogic() { return newLocationBrokerLogic; }
 
         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 6c414468224..3eef6be7012 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -270,6 +270,13 @@ public class Flags {
             "Takes effect at redeployment",
             ZONE_ID, APPLICATION_ID);
 
+    public static final UnboundBooleanFlag NEW_LOCATION_BROKER_LOGIC = defineFeatureFlag(
+            "new-location-broker-logic", false,
+            List.of("arnej"), "2021-09-07", "2021-12-31",
+            "Use new implementation of internal logic in service location broker",
+            "Takes effect immediately",
+            ZONE_ID, 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,
diff --git a/slobrok/src/apps/slobrok/slobrok.cpp b/slobrok/src/apps/slobrok/slobrok.cpp
index 390b7356410..63212745644 100644
--- a/slobrok/src/apps/slobrok/slobrok.cpp
+++ b/slobrok/src/apps/slobrok/slobrok.cpp
@@ -54,7 +54,7 @@ App::Main()
     int argi = 1;
     const char* optArg;
     int c;
-    while ((c = GetOpt("c:s:p:", optArg, argi)) != -1) {
+    while ((c = GetOpt("c:s:p:N", optArg, argi)) != -1) {
         switch (c) {
         case 'c':
             cfgId = std::string(optArg);
@@ -62,6 +62,9 @@ App::Main()
         case 'p':
             portnum = atoi(optArg);
             break;
+        case 'N':
+            // ignore flag for now
+            break;
         default:
             LOG(error, "unknown option letter '%c'", c);
             return 1;