Update reindexing status clients in controller, and trigger only for indexed

7 files changed, 18 insertions, 47 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ApplicationReindexing.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ApplicationReindexing.java
index e143cdd6d9e..f94a91dc0c6 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ApplicationReindexing.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ApplicationReindexing.java
@@ -6,8 +6,6 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 
-import static java.util.Objects.requireNonNull;
-
 /**
  * Reindexing status for a single Vespa application.
  *
@@ -16,12 +14,10 @@ import static java.util.Objects.requireNonNull;
 public class ApplicationReindexing {
 
     private final boolean enabled;
-    private final Status common;
     private final Map<String, Cluster> clusters;
 
-    public ApplicationReindexing(boolean enabled, Status common, Map<String, Cluster> clusters) {
+    public ApplicationReindexing(boolean enabled, Map<String, Cluster> clusters) {
         this.enabled = enabled;
-        this.common = requireNonNull(common);
         this.clusters = Map.copyOf(clusters);
     }
 
@@ -29,10 +25,6 @@ public class ApplicationReindexing {
         return enabled;
     }
 
-    public Status common() {
-        return common;
-    }
-
     public Map<String, Cluster> clusters() {
         return clusters;
     }
@@ -43,20 +35,18 @@ public class ApplicationReindexing {
         if (o == null || getClass() != o.getClass()) return false;
         ApplicationReindexing that = (ApplicationReindexing) o;
         return enabled == that.enabled &&
-               common.equals(that.common) &&
                clusters.equals(that.clusters);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(enabled, common, clusters);
+        return Objects.hash(enabled, clusters);
     }
 
     @Override
     public String toString() {
         return "ApplicationReindexing{" +
                "enabled=" + enabled +
-               ", common=" + common +
                ", clusters=" + clusters +
                '}';
     }
@@ -64,20 +54,14 @@ public class ApplicationReindexing {
 
     public static class Cluster {
 
-        private final Optional<Status> common;
         private final Map<String, Long> pending;
         private final Map<String, Status> ready;
 
-        public Cluster(Status common, Map<String, Long> pending, Map<String, Status> ready) {
-            this.common = Optional.ofNullable(common);
+        public Cluster(Map<String, Long> pending, Map<String, Status> ready) {
             this.pending = Map.copyOf(pending);
             this.ready = Map.copyOf(ready);
         }
 
-        public Optional<Status> common() {
-            return common;
-        }
-
         public Map<String, Long> pending() {
             return pending;
         }
@@ -91,20 +75,18 @@ public class ApplicationReindexing {
             if (this == o) return true;
             if (o == null || getClass() != o.getClass()) return false;
             Cluster cluster = (Cluster) o;
-            return common.equals(cluster.common) &&
-                   pending.equals(cluster.pending) &&
+            return pending.equals(cluster.pending) &&
                    ready.equals(cluster.ready);
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(common, pending, ready);
+            return Objects.hash(pending, ready);
         }
 
         @Override
         public String toString() {
             return "Cluster{" +
-                   "common=" + common +
                    ", pending=" + pending +
                    ", ready=" + ready +
                    '}';
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
index 11940b30ac1..a90155d4e3e 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
@@ -35,7 +35,7 @@ public interface ConfigServer {
 
     PreparedApplication deploy(DeploymentData deployment);
 
-    void reindex(DeploymentId deployment, List<String> clusterNames, List<String> documentTypes);
+    void reindex(DeploymentId deployment, List<String> clusterNames, List<String> documentTypes, boolean indexedOnly);
 
     Optional<ApplicationReindexing> getReindexing(DeploymentId deployment);
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index e071221dd05..8447353a869 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -189,8 +189,8 @@ public class ApplicationController {
      * if no documents types are given, reindexing is triggered for all given clusters; otherwise
      * reindexing is triggered for the cartesian product of the given clusters and document types.
      */
-    public void reindex(ApplicationId id, ZoneId zoneId, List<String> clusterNames, List<String> documentTypes) {
-        configServer.reindex(new DeploymentId(id, zoneId), clusterNames, documentTypes);
+    public void reindex(ApplicationId id, ZoneId zoneId, List<String> clusterNames, List<String> documentTypes, boolean indexedOnly) {
+        configServer.reindex(new DeploymentId(id, zoneId), clusterNames, documentTypes, indexedOnly);
     }
 
     /** Returns the reindexing status for the given application in the given zone. */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggerer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggerer.java
index f787c5d62e7..263a33cf266 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggerer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggerer.java
@@ -49,7 +49,7 @@ public class ReindexingTriggerer extends ControllerMaintainer {
                     for (Deployment deployment : deployments)
                         if (   inWindowOfOpportunity(now, id, deployment.zone())
                             && reindexingIsReady(controller().applications().applicationReindexing(id, deployment.zone()), now))
-                            controller().applications().reindex(id, deployment.zone(), List.of(), List.of());
+                            controller().applications().reindex(id, deployment.zone(), List.of(), List.of(), true);
                 });
             return true;
         }
@@ -74,11 +74,9 @@ public class ReindexingTriggerer extends ControllerMaintainer {
     }
 
     static boolean reindexingIsReady(ApplicationReindexing reindexing, Instant now) {
-        if (reindexing.clusters().values().stream().flatMap(cluster -> cluster.ready().values().stream())
-                      .anyMatch(status -> status.startedAt().isPresent() && status.endedAt().isEmpty()))
-            return false;
-
-        return reindexing.common().readyAt().orElse(Instant.EPOCH).isBefore(now.minus(reindexingPeriod.dividedBy(2)));
+        return reindexing.clusters().values().stream().flatMap(cluster -> cluster.ready().values().stream())
+                         .allMatch(status ->    status.readyAt().map(now.minus(reindexingPeriod.dividedBy(2))::isAfter).orElse(true)
+                                             && (status.startedAt().isEmpty() || status.endedAt().isPresent()));
     }
 
 }
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 be4372af526..8378b914fe4 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
@@ -1561,7 +1561,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
                                              .filter(type -> ! type.isBlank())
                                              .collect(toUnmodifiableList());
 
-        controller.applications().reindex(id, zone, clusterNames, documentTypes);
+        controller.applications().reindex(id, zone, clusterNames, documentTypes, request.getBooleanProperty("indexedOnly"));
         return new MessageResponse("Requested reindexing of " + id + " in " + zone +
                                    (clusterNames.isEmpty() ? "" : ", on clusters " + String.join(", ", clusterNames) +
                                                                   (documentTypes.isEmpty() ? "" : ", for types " + String.join(", ", documentTypes))));
@@ -1577,14 +1577,12 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         Cursor root = slime.setObject();
 
         root.setBool("enabled", reindexing.enabled());
-        setStatus(root.setObject("status"), reindexing.common());
 
         Cursor clustersArray = root.setArray("clusters");
         reindexing.clusters().entrySet().stream().sorted(comparingByKey())
                   .forEach(cluster -> {
                       Cursor clusterObject = clustersArray.addObject();
                       clusterObject.setString("name", cluster.getKey());
-                      cluster.getValue().common().ifPresent(common -> setStatus(clusterObject.setObject("status"), common));
 
                       Cursor pendingArray = clusterObject.setArray("pending");
                       cluster.getValue().pending().entrySet().stream().sorted(comparingByKey())
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 7753570b72d..ee3c523a497 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
@@ -422,15 +422,13 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     }
 
     @Override
-    public void reindex(DeploymentId deployment, List<String> clusterNames, List<String> documentTypes) { }
+    public void reindex(DeploymentId deployment, List<String> clusterNames, List<String> documentTypes, boolean indexedOnly) { }
 
     @Override
     public Optional<ApplicationReindexing> getReindexing(DeploymentId deployment) {
         return Optional.of(new ApplicationReindexing(true,
-                                                     new Status(Instant.ofEpochMilli(123)),
                                                      Map.of("cluster",
-                                                            new ApplicationReindexing.Cluster(new Status(Instant.ofEpochMilli(234)),
-                                                                                              Map.of("type", 100L),
+                                                            new ApplicationReindexing.Cluster(Map.of("type", 100L),
                                                                                               Map.of("type", new Status(Instant.ofEpochMilli(345),
                                                                                                                         Instant.ofEpochMilli(456),
                                                                                                                         Instant.ofEpochMilli(567),
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggererTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggererTest.java
index 848426b6581..3c22ee3f4c3 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggererTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ReindexingTriggererTest.java
@@ -50,8 +50,7 @@ public class ReindexingTriggererTest {
     public void testReindexingIsReady() {
         Instant then = Instant.now();
         ApplicationReindexing reindexing = new ApplicationReindexing(true,
-                                                                     new Status(then),
-                                                                     Map.of());
+                                                                     Map.of("c", new Cluster(Map.of(), Map.of("d", new Status(then)))));
 
         Instant now = then;
         assertFalse("Should not be ready less than one half-period after last triggering",
@@ -66,20 +65,16 @@ public class ReindexingTriggererTest {
                    reindexingIsReady(reindexing, now));
 
         reindexing = new ApplicationReindexing(true,
-                                               new Status(then),
                                                Map.of("cluster",
-                                                      new Cluster(new Status(then),
-                                                                  Map.of(),
+                                                      new Cluster(Map.of(),
                                                                   Map.of("type",
                                                                          new Status(then, then, null, null, null, null)))));
         assertFalse("Should not be ready when reindexing is already running",
                     reindexingIsReady(reindexing, now));
 
         reindexing = new ApplicationReindexing(true,
-                                               new Status(then),
                                                Map.of("cluster",
-                                                      new Cluster(new Status(then),
-                                                                  Map.of("type", 123L),
+                                                      new Cluster(Map.of("type", 123L),
                                                                   Map.of("type",
                                                                          new Status(then, then, now, null, null, null)))));
         assertTrue("Should be ready when reindexing is no longer running",