Correctly configure reindexing for shared cluster controller cluster

Moved ReindexingContext to ClusterControllerContainerCluster.
Add document types to reindexing context during construction of ContentCluster instance.
Add disabled implementation of Reindexing to its Java interface.

6 files changed, 69 insertions, 53 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 24d4f088f8e..32cdb03aeab 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
@@ -38,16 +38,13 @@ public class ClusterControllerContainer extends Container implements
     private static final ComponentSpecification REINDEXING_CONTROLLER_BUNDLE = new ComponentSpecification("clustercontroller-reindexer");
 
     private final Set<String> bundles = new TreeSet<>();
-    private final ReindexingContext reindexingContext;
 
     public ClusterControllerContainer(
             AbstractConfigProducer<?> parent,
             int index,
             boolean runStandaloneZooKeeper,
-            boolean isHosted,
-            ReindexingContext reindexingContext) {
+            boolean isHosted) {
         super(parent, "" + index, index, isHosted);
-        this.reindexingContext = reindexingContext;
 
         addHandler("clustercontroller-status",
                    "com.yahoo.vespa.clustercontroller.apps.clustercontroller.StatusHandler",
@@ -122,8 +119,12 @@ public class ClusterControllerContainer extends Container implements
         addHandler(new Handler(createComponentModel(id, className, bundle)), path);
     }
 
+    private ReindexingContext reindexingContext() {
+        return ((ClusterControllerContainerCluster) parent).reindexingContext();
+    }
+
     private void configureReindexing() {
-        if (reindexingContext != null) {
+        if (reindexingContext().reindexing().enabled()) {
             addFileBundle(REINDEXING_CONTROLLER_BUNDLE.getName());
             addComponent(new SimpleComponent(DocumentAccessProvider.class.getName()));
             addComponent("reindexing-maintainer",
@@ -149,17 +150,21 @@ public class ClusterControllerContainer extends Container implements
 
     @Override
     public void getConfig(ReindexingConfig.Builder builder) {
-        if (reindexingContext == null)
-            return;
-
-        builder.clusterName(reindexingContext.contentClusterName());
-        builder.enabled(reindexingContext.reindexing().enabled());
-        for (NewDocumentType type : reindexingContext.documentTypes()) {
-            String typeName = type.getFullName().getName();
-            reindexingContext.reindexing().status(reindexingContext.contentClusterName(), typeName)
-                             .ifPresent(status -> builder.status(typeName,
-                                                                 new ReindexingConfig.Status.Builder()
-                                                                         .readyAtMillis(status.ready().toEpochMilli())));
+        ReindexingContext ctx = reindexingContext();
+        if (ctx.reindexing().enabled()) return;
+
+        builder.enabled(ctx.reindexing().enabled());
+        for (String clusterId : ctx.clusterIds()) {
+            ReindexingConfig.Clusters.Builder clusterBuilder = new ReindexingConfig.Clusters.Builder();
+            for (NewDocumentType type : ctx.documentTypesForCluster(clusterId)) {
+                String typeName = type.getFullName().getName();
+                ctx.reindexing().status(clusterId, typeName).ifPresent(
+                        status -> clusterBuilder.documentTypes(
+                                typeName,
+                                new ReindexingConfig.Clusters.DocumentTypes.Builder()
+                                        .readyAtMillis(status.ready().toEpochMilli())));
+            }
+            builder.clusters(clusterId, clusterBuilder);
         }
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
index ed4c50999fe..c202d29cc5d 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
@@ -1,6 +1,7 @@
 // Copyright 2019 Oath Inc. 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.config.model.api.Reindexing;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.vespa.model.container.ContainerCluster;
@@ -12,9 +13,14 @@ import com.yahoo.vespa.model.container.ContainerCluster;
  */
 public class ClusterControllerContainerCluster extends ContainerCluster<ClusterControllerContainer>
 {
-    public ClusterControllerContainerCluster(AbstractConfigProducer<?> parent, String subId, String name, DeployState deployState) {
+
+    private final ReindexingContext reindexingContext;
+
+    public ClusterControllerContainerCluster(
+            AbstractConfigProducer<?> parent, String subId, String name, DeployState deployState) {
         super(parent, subId, name, deployState, false);
         addDefaultHandlersWithVip();
+        this.reindexingContext = new ReindexingContext(deployState.reindexing().orElse(Reindexing.DISABLED_INSTANCE));
     }
 
     @Override
@@ -22,4 +28,6 @@ public class ClusterControllerContainerCluster extends ContainerCluster<ClusterC
 
     @Override protected boolean messageBusEnabled() { return false; }
 
+    public ReindexingContext reindexingContext() { return reindexingContext; }
+
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ReindexingContext.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ReindexingContext.java
index fdd12088d04..712498f78cb 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ReindexingContext.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ReindexingContext.java
@@ -5,7 +5,11 @@ import com.yahoo.config.model.api.Reindexing;
 import com.yahoo.documentmodel.NewDocumentType;
 
 import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 
 /**
  * Context required to configure automatic reindexing for a given cluster controller cluster (for a given content cluster).
@@ -14,20 +18,32 @@ import java.util.Objects;
  */
 public class ReindexingContext {
 
+    private final Object monitor = new Object();
+    private final Map<String, Set<NewDocumentType>> documentTypesPerCluster = new HashMap<>();
     private final Reindexing reindexing;
-    private final String contentClusterName;
-    private final Collection<NewDocumentType> documentTypes;
 
-    public ReindexingContext(
-            Reindexing reindexing,
-            String contentClusterName,
-            Collection<NewDocumentType> documentTypes) {
+    public ReindexingContext(Reindexing reindexing) {
         this.reindexing = Objects.requireNonNull(reindexing);
-        this.contentClusterName = Objects.requireNonNull(contentClusterName);
-        this.documentTypes = Objects.requireNonNull(documentTypes);
+    }
+
+    public void addDocumentType(String clusterId, NewDocumentType type) {
+        synchronized (monitor) {
+            documentTypesPerCluster.computeIfAbsent(clusterId, ignored -> new HashSet<>())
+                    .add(type);
+        }
+    }
+
+    public Collection<String> clusterIds() {
+        synchronized (monitor) {
+            return new HashSet<>(documentTypesPerCluster.keySet());
+        }
+    }
+
+    public Collection<NewDocumentType> documentTypesForCluster(String clusterId) {
+        synchronized (monitor) {
+            return new HashSet<>(documentTypesPerCluster.getOrDefault(clusterId, Set.of()));
+        }
     }
 
     public Reindexing reindexing() { return reindexing; }
-    public String contentClusterName() { return contentClusterName; }
-    public Collection<NewDocumentType> documentTypes() { return documentTypes; }
 }
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 3719c8a43a2..3c72361d814 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
@@ -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(), /*reindexingContext*/null);
+            return new ClusterControllerContainer(parent, i, runStandaloneZooKeeper, deployState.isHosted());
         }
     }
 }
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 93367f1a286..cf9a567e409 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
@@ -5,7 +5,6 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.Sets;
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.model.ConfigModelContext;
-import com.yahoo.config.model.api.Reindexing;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.config.model.producer.AbstractConfigProducerRoot;
@@ -165,7 +164,7 @@ public class ContentCluster extends AbstractConfigProducer implements
 
             if (context.getParentProducer().getRoot() == null) return c;
 
-            addClusterControllers(containers, context, c.rootGroup, contentElement, c.clusterId, c, documentDefinitions);
+            addClusterControllers(containers, context, c.rootGroup, contentElement, c.clusterId, c);
             return c;
         }
 
@@ -278,10 +277,9 @@ public class ContentCluster extends AbstractConfigProducer implements
             }
         }
 
-        private void addClusterControllers(Collection<ContainerModel> containers, ConfigModelContext context, 
-                                           StorageGroup rootGroup, ModelElement contentElement, 
-                                           String contentClusterName, ContentCluster contentCluster,
-                                           Map<String, NewDocumentType> documentDefinitions) {
+        private void addClusterControllers(Collection<ContainerModel> containers, ConfigModelContext context,
+                                           StorageGroup rootGroup, ModelElement contentElement,
+                                           String contentClusterName, ContentCluster contentCluster) {
             if (admin == null) return; // only in tests
             if (contentCluster.getPersistence() == null) return;
 
@@ -303,13 +301,11 @@ public class ContentCluster extends AbstractConfigProducer implements
                 Collection<HostResource> hosts = nodesSpecification.isDedicated() ?
                                                  getControllerHosts(nodesSpecification, admin, clusterName, context) :
                                                  drawControllerHosts(nodesSpecification.minResources().nodes(), rootGroup, containers);
-                ReindexingContext reindexingContext = createReindexingContent(context, contentClusterName, documentDefinitions);
                 clusterControllers = createClusterControllers(new ClusterControllerCluster(contentCluster, "standalone"),
                                                               hosts,
                                                               clusterName,
                                                               true,
-                                                              context.getDeployState(),
-                                                              reindexingContext);
+                                                              context.getDeployState());
                 contentCluster.clusterControllers = clusterControllers;
             }
             else {
@@ -320,22 +316,15 @@ public class ContentCluster extends AbstractConfigProducer implements
                         context.getDeployState().getDeployLogger().log(Level.INFO,
                                                                        "When having content cluster(s) and more than 1 config server it is recommended to configure cluster controllers explicitly.");
                     }
-                    ReindexingContext reindexingContext = createReindexingContent(context, contentClusterName, documentDefinitions);
-                    clusterControllers = createClusterControllers(admin, hosts, "cluster-controllers", false, context.getDeployState(), reindexingContext);
+                    clusterControllers = createClusterControllers(admin, hosts, "cluster-controllers", false, context.getDeployState());
                     admin.setClusterControllers(clusterControllers);
                 }
             }
 
             addClusterControllerComponentsForThisCluster(clusterControllers, contentCluster);
-        }
-
-        private static ReindexingContext createReindexingContent(
-                ConfigModelContext ctx, String contentClusterName, Map<String, NewDocumentType> documentDefinitions) {
-            class DisabledReindexing implements Reindexing {}
-            Reindexing reindexing = ctx.properties().featureFlags().enableAutomaticReindexing()
-                    ? ctx.getDeployState().reindexing().orElse(new DisabledReindexing())
-                    : new DisabledReindexing();
-            return new ReindexingContext(reindexing, contentClusterName, documentDefinitions.values());
+            ReindexingContext reindexingContext = clusterControllers.reindexingContext();
+            contentCluster.documentDefinitions.values()
+                    .forEach(type -> reindexingContext.addDocumentType(contentCluster.clusterId, type));
         }
 
         /** Returns any other content cluster which shares nodes with this, or null if none are built */
@@ -468,8 +457,7 @@ public class ContentCluster extends AbstractConfigProducer implements
                                                                            Collection<HostResource> hosts,
                                                                            String name,
                                                                            boolean multitenant,
-                                                                           DeployState deployState,
-                                                                           ReindexingContext reindexingContext) {
+                                                                           DeployState deployState) {
             var clusterControllers = new ClusterControllerContainerCluster(parent, name, name, deployState);
             List<ClusterControllerContainer> containers = new ArrayList<>();
             // Add a cluster controller on each config server (there is always at least one).
@@ -481,8 +469,7 @@ public class ContentCluster extends AbstractConfigProducer implements
                                     clusterControllers,
                                     index,
                                     multitenant,
-                                    deployState.isHosted(),
-                                    reindexingContext);
+                                    deployState.isHosted());
                     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 d151d34b6a7..4cee4517982 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
@@ -392,7 +392,7 @@ public class ContainerClusterTest {
     }
 
     private static void addClusterController(DeployLogger deployLogger, ClusterControllerContainerCluster cluster, String hostName) {
-        ClusterControllerContainer container = new ClusterControllerContainer(cluster, 1, false, cluster.isHostedVespa(), /*reindexingContext*/null);
+        ClusterControllerContainer container = new ClusterControllerContainer(cluster, 1, false, cluster.isHostedVespa());
         container.setHostResource(new HostResource(new Host(null, hostName)));
         container.initService(deployLogger);
         cluster.addContainer(container);