Trigger periodic redeployment for apps with readied reindexing

3 files changed, 41 insertions, 24 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java
index ba71bbf0d42..14693c75436 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java
@@ -13,9 +13,7 @@ import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 
 import java.time.Duration;
-import java.time.Instant;
 import java.util.Map;
-import java.util.function.Function;
 
 import static java.util.stream.Collectors.toMap;
 
@@ -41,34 +39,21 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer {
     @Override
     protected boolean canDeployNow(ApplicationId application) {
         return deployer().deployTime(application)
-                // Don't deploy if a regular deploy just happened
-                .map(lastDeployTime -> lastDeployTime.isBefore(nodeRepository().clock().instant().minus(minTimeBetweenRedeployments)))
-                // We only know last deploy time for applications that were deployed on this config server,
-                // the rest will be deployed on another config server
-                .orElse(false);
+                         .map(lastDeployTime ->    lastDeployTime.isBefore(nodeRepository().clock().instant().minus(minTimeBetweenRedeployments))
+                                                || deployer().readiedReindexingAfter(application, lastDeployTime))
+                         .orElse(false);
     }
 
     @Override
     protected Map<ApplicationId, String> applicationsNeedingMaintenance() {
         if (deployer().bootstrapping()) return Map.of();
 
-        // Collect all deployment times before sorting as deployments may happen while we build the set, breaking
-        // the comparable contract. Stale times are fine as the time is rechecked in ApplicationMaintainer#deployNow
-        Map<ApplicationId, Instant> deploymentTimes = nodesNeedingMaintenance().stream()
-                                                                               .map(node -> node.allocation().get().owner())
-                                                                               .distinct()
-                                                                               .filter(this::canDeployNow)
-                                                                               .collect(toMap(Function.identity(), this::deployTime));
-
-        return deploymentTimes.entrySet().stream()
-                              .sorted(Map.Entry.comparingByValue())
-                              .map(Map.Entry::getKey)
-                              .filter(this::shouldMaintain)
-                              .collect(toMap(applicationId -> applicationId, applicationId -> "current deployment being too old"));
-    }
-
-    private Instant deployTime(ApplicationId applicationId) {
-        return deployer().deployTime(applicationId).orElse(Instant.EPOCH);
+        return nodesNeedingMaintenance().stream()
+                                        .map(node -> node.allocation().get().owner())
+                                        .distinct()
+                                        .filter(this::shouldMaintain)
+                                        .filter(this::canDeployNow)
+                                        .collect(toMap(applicationId -> applicationId, applicationId -> "current deployment being too old"));
     }
 
     private boolean shouldMaintain(ApplicationId id) {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
index 4b4e9121be4..0573c6a877e 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
@@ -50,6 +50,7 @@ public class MockDeployer implements Deployer {
 
     private boolean failActivate = false;
     private boolean bootstrapping = true;
+    private Instant readiedReindexingAt = null;
 
     /** Create a mock deployer which returns empty on every deploy request. */
     @Inject
@@ -91,6 +92,8 @@ public class MockDeployer implements Deployer {
 
     public void setBootstrapping(boolean bootstrapping) { this.bootstrapping = bootstrapping; }
 
+    public void setReadiedReindexingAt(Instant readiedReindexingAt) { this.readiedReindexingAt = readiedReindexingAt; }
+
     @Override
     public Optional<Deployment> deployFromLocalActive(ApplicationId id, boolean bootstrap) {
         return deployFromLocalActive(id, Duration.ofSeconds(60));
@@ -131,6 +134,11 @@ public class MockDeployer implements Deployer {
     }
 
     @Override
+    public boolean readiedReindexingAfter(ApplicationId application, Instant instant) {
+        return readiedReindexingAt != null && readiedReindexingAt.isAfter(instant);
+    }
+
+    @Override
     public boolean bootstrapping() {
         return bootstrapping;
     }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java
index 9fea27a2cb0..f92526282d1 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java
@@ -182,6 +182,30 @@ public class PeriodicApplicationMaintainerTest {
     }
 
     @Test(timeout = 60_000)
+    public void application_deploy_triggered_by_reindexing_ready() {
+        fixture.activate();
+
+        assertEquals("No deployment expected", 2, fixture.deployer.activations);
+
+        // Holds off on deployments a while after starting
+        fixture.setBootstrapping(false);
+        fixture.runApplicationMaintainer();
+        assertEquals("No deployment expected", 2, fixture.deployer.activations);
+
+        Instant firstDeployTime = clock.instant();
+
+        // Reindexing readied before last deploy time triggers nothing.
+        fixture.deployer.setReadiedReindexingAt(firstDeployTime.minusSeconds(1));
+        fixture.runApplicationMaintainer();
+        assertEquals("No deployment expected", 2, fixture.deployer.activations);
+
+        // Reindexing readied after last deploy time triggers nothing.
+        fixture.deployer.setReadiedReindexingAt(firstDeployTime.plusSeconds(1));
+        fixture.runApplicationMaintainer();
+        assertEquals("No deployment expected", 4, fixture.deployer.activations);
+    }
+
+    @Test(timeout = 60_000)
     public void queues_all_eligible_applications_for_deployment() throws Exception {
         fixture.activate();