Deploy asynchronously to guarantee we deploy every 30 minutes

4 files changed, 48 insertions, 12 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
index b39c24422a0..9a126e2abb6 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
@@ -11,6 +11,8 @@ import com.yahoo.vespa.hosted.provision.NodeRepository;
 import java.time.Duration;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
 import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
@@ -29,8 +31,10 @@ public class ApplicationMaintainer extends Maintainer {
     private final Deployer deployer;
     private final Function<NodeRepository, Set<ApplicationId>> activeApplicationsGetter;
 
+    private final Executor deploymentExecutor = Executors.newCachedThreadPool();
+    
     public ApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval) {
-        this(deployer, nodeRepository, interval, ApplicationMaintainer::getActiveApplications);
+        this(deployer, nodeRepository, interval, ApplicationMaintainer::activeApplications);
     }
 
     ApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval,
@@ -51,31 +55,46 @@ public class ApplicationMaintainer extends Maintainer {
                 //
                 // Lock is acquired with a low timeout to reduce the chance of colliding with an external deployment.
                 try (Mutex lock = nodeRepository().lock(application, Duration.ofSeconds(1))) {
-                    if ( ! isActive(application)) continue;
+                    if ( ! isActive(application)) continue; // became inactive since we started the loop
                     Optional<Deployment> deployment = deployer.deployFromLocalActive(application, Duration.ofMinutes(30));
                     if ( ! deployment.isPresent()) continue; // this will be done at another config server
 
-                    deployment.get().prepare();
-                    deployment.get().activate();
+                    // deploy asynchronously to make sure we do all applications even when deployments are slow
+                    deployAsynchronously(deployment.get()); 
                 }
+                
+                // Throttle deployments somewhat. 
+                // With a maintenance interval of 30 mins, 1 second is good until 1800 applications
+                try { Thread.sleep(1000); } catch (InterruptedException e) { return; }
             }
             catch (RuntimeException e) {
                 log.log(Level.WARNING, "Exception on maintenance redeploy of " + application, e);
             }
         }
     }
-
-    @Override
-    public String toString() { return "Periodic application redeployer"; }
+    
+    protected void deployAsynchronously(Deployment deployment) {
+        deploymentExecutor.execute(() -> {
+            try {
+                deployment.activate();
+            }
+            catch (RuntimeException e) {
+                log.log(Level.WARNING, "Exception on maintenance redeploy", e);
+            }
+        });
+    }
 
     private boolean isActive(ApplicationId application) {
         return ! nodeRepository().getNodes(application, Node.State.active).isEmpty();
     }
 
-    static Set<ApplicationId> getActiveApplications(NodeRepository nodeRepository) {
+    static Set<ApplicationId> activeApplications(NodeRepository nodeRepository) {
         return nodeRepository.getNodes(Node.State.active).stream()
                 .map(node -> node.allocation().get().owner())
                 .collect(Collectors.toSet());
     }
 
+    @Override
+    public String toString() { return "Periodic application redeployer"; }
+
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
index 11e19a8912e..0a49dd9f36d 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
@@ -235,7 +235,6 @@ public class NodeFailer extends Maintainer {
             if (! allTenantNodesFailedOutSuccessfully) return false;
             node = nodeRepository().fail(node.hostname(), reason);
             try {
-                deployment.get().prepare();
                 deployment.get().activate();
                 return true;
             }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainerTest.java
index c976c7f1ee2..d6388d359f4 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainerTest.java
@@ -5,6 +5,8 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.Capacity;
 import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.Deployer;
+import com.yahoo.config.provision.Deployment;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostSpec;
 import com.yahoo.config.provision.InstanceName;
@@ -185,7 +187,7 @@ public class ApplicationMaintainerTest {
         }
 
         void runApplicationMaintainer() {
-            runApplicationMaintainer(ApplicationMaintainer::getActiveApplications);
+            runApplicationMaintainer(ApplicationMaintainer::activeApplications);
         }
 
         void runApplicationMaintainer(Function<NodeRepository, Set<ApplicationId>> activeApplicationsGetter) {
@@ -195,7 +197,7 @@ public class ApplicationMaintainerTest {
             apps.put(app2, new MockDeployer.ApplicationContext(app2, clusterApp2, 
                                                                Capacity.fromNodeCount(wantedNodesApp2, Optional.of("default")), 1));
             MockDeployer deployer = new MockDeployer(provisioner, apps);
-            new ApplicationMaintainer(deployer, nodeRepository, Duration.ofMinutes(30), activeApplicationsGetter).run();
+            new SynchronousApplicationMaintainer(deployer, nodeRepository, Duration.ofMinutes(30), activeApplicationsGetter).run();
         }
 
         NodeList getNodes(Node.State ... states) {
@@ -203,5 +205,19 @@ public class ApplicationMaintainerTest {
         }
 
     }
+    
+    private static class SynchronousApplicationMaintainer extends ApplicationMaintainer {
+
+        SynchronousApplicationMaintainer(Deployer deployer, NodeRepository nodeRepository, Duration interval,
+                              Function<NodeRepository, Set<ApplicationId>> activeApplicationsGetter) {
+            super(deployer, nodeRepository, interval, activeApplicationsGetter);
+        }
+
+        @Override
+        protected void deployAsynchronously(Deployment deployment) {
+            deployment.activate();
+        }
+
+    }
 
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MockDeployer.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MockDeployer.java
index ea7ed099b4e..377cb2e4443 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MockDeployer.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MockDeployer.java
@@ -47,7 +47,7 @@ public class MockDeployer implements Deployer {
         private final ApplicationContext application;
 
         /** The list of hosts prepared in this. Only set after prepare is called (and a provisioner is assigned) */
-        private List<HostSpec> preparedHosts;
+        private List<HostSpec> preparedHosts = null;
 
         private MockDeployment(NodeRepositoryProvisioner provisioner, ApplicationContext application) {
             this.provisioner = provisioner;
@@ -61,6 +61,8 @@ public class MockDeployer implements Deployer {
 
         @Override
         public void activate() {
+            if (preparedHosts == null)
+                prepare();
             redeployments++;
             try (NestedTransaction t = new NestedTransaction()) {
                 provisioner.activate(t, application.id(), preparedHosts);