Merge pull request #2637 from yahoo/hmusum/redeploy-at-bootstrap-in-parallel

Redeploy application in parallel when bootstrapping

7 files changed, 99 insertions, 73 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index 3e132137468..229f24152c3 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -10,6 +10,7 @@ import com.yahoo.config.application.api.ApplicationMetaData;
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.Deployer;
 import com.yahoo.config.provision.HostFilter;
 import com.yahoo.config.provision.Provisioner;
 import com.yahoo.config.provision.TenantName;
@@ -43,8 +44,12 @@ import java.io.File;
 import java.net.URI;
 import java.time.Clock;
 import java.time.Duration;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.logging.Logger;
 
 /**
@@ -66,6 +71,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     private final HttpProxy httpProxy;
     private final Clock clock;
     private final DeployLogger logger = new SilentDeployLogger();
+    private final ConfigserverConfig configserverConfig;
     private final Environment environment;
 
     @Inject
@@ -83,6 +89,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         this.convergeChecker = applicationConvergenceChecker;
         this.httpProxy = httpProxy;
         this.clock = Clock.systemUTC();
+        this.configserverConfig = configserverConfig;
         this.environment = Environment.from(configserverConfig.environment());
     }
 
@@ -327,6 +334,24 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return session.getSessionId();
     }
 
+    void redeployAllApplications(Deployer deployer) {
+        ExecutorService deploymentExecutor = Executors.newFixedThreadPool(configserverConfig.numParallelTenantLoaders());
+        tenants.getAllTenants().forEach(tenant -> listApplicationIds(tenant)
+                .forEach(applicationId -> redeployApplication(applicationId, deployer, deploymentExecutor)));
+    }
+
+    private void redeployApplication(ApplicationId applicationId, Deployer deployer, ExecutorService deploymentExecutor) {
+        log.log(LogLevel.DEBUG, () -> "Redeploying " + applicationId);
+        deployer.deployFromLocalActive(applicationId, Duration.ofMinutes(30))
+                .ifPresent(deployment -> deploymentExecutor.execute(() -> {
+                    try {
+                        deployment.activate();
+                    } catch (RuntimeException e) {
+                        log.log(LogLevel.ERROR, "Redeploying " + applicationId + " failed", e);
+                    }
+                }));
+    }
+
     public ApplicationFile getApplicationFileFromSession(TenantName tenantName, long sessionId, String path, LocalSession.Mode mode) {
         Tenant tenant = tenants.getTenant(tenantName);
         return getLocalSession(tenant, sessionId).getApplicationFile(Path.fromString(path), mode);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
index 1af101c074e..e0def2b0e4b 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
@@ -6,7 +6,6 @@ import com.yahoo.component.AbstractComponent;
 import com.yahoo.config.provision.Deployer;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.config.server.rpc.RpcServer;
-import com.yahoo.vespa.config.server.tenant.Tenants;
 import com.yahoo.vespa.config.server.version.VersionState;
 
 /**
@@ -19,7 +18,7 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
 
     private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(ConfigServerBootstrap.class.getName());
 
-    private final Tenants tenants;
+    private final ApplicationRepository applicationRepository;
     private final RpcServer server;
     private final Thread serverThread;
     private final Deployer deployer;
@@ -29,8 +28,9 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
     // added to the rpc server before it starts answering rpc requests.
     @SuppressWarnings("UnusedParameters")
     @Inject
-    public ConfigServerBootstrap(Tenants tenants, RpcServer server, Deployer deployer, VersionState versionState) {
-        this.tenants = tenants;
+    public ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
+                                 Deployer deployer, VersionState versionState) {
+        this.applicationRepository = applicationRepository;
         this.server = server;
         this.deployer = deployer;
         this.versionState = versionState;
@@ -52,8 +52,9 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
     @Override
     public void run() {
         if (versionState.isUpgraded()) {
-            log.log(LogLevel.INFO, "Configserver upgraded from " + versionState.storedVersion() + " to " + versionState.currentVersion() + ". Redeploying all applications");
-            tenants.redeployApplications(deployer);
+            log.log(LogLevel.INFO, "Configserver upgraded from " + versionState.storedVersion() + " to "
+                    + versionState.currentVersion() + ". Redeploying all applications");
+            applicationRepository.redeployAllApplications(deployer);
             log.log(LogLevel.INFO, "All applications redeployed");
         }
         versionState.saveNewVersion();
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ListTenantsHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ListTenantsHandler.java
index 84df1834712..28591f8f198 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ListTenantsHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ListTenantsHandler.java
@@ -27,7 +27,7 @@ public class ListTenantsHandler extends HttpHandler {
 
     @Override
     protected HttpResponse handleGET(HttpRequest request) {
-        return new ListTenantsResponse(tenants.getAllTenants());
+        return new ListTenantsResponse(tenants.getAllTenantNames());
     }
 
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java
index e03de29a9e4..b430b78038b 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java
@@ -163,21 +163,5 @@ public class Tenant implements TenantHandlerProvider {
         curator.delete(path);
     }
 
-    public void redeployApplications(Deployer deployer) {
-        // TODO: Configurable timeout
-        applicationRepo.listApplications().forEach(applicationId -> redeployApplication(applicationId, deployer));
-    }
-
-    private void redeployApplication(ApplicationId applicationId, Deployer deployer) {
-        try {
-            log.log(LogLevel.DEBUG, "Redeploying " + applicationId);
-            deployer.deployFromLocalActive(applicationId, Duration.ofMinutes(30))
-                    .ifPresent(deployment -> {
-                        deployment.prepare();
-                        deployment.activate();
-                    });
-        } catch (Exception e) {
-            log.log(LogLevel.ERROR, "Redeploying " + applicationId + " failed", e);
-        }
-    }
+
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java
index e0f470001fa..cc21616050a 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java
@@ -5,7 +5,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.inject.Inject;
 import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.Deployer;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.log.LogLevel;
 import com.yahoo.path.Path;
@@ -87,7 +86,7 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen
         // Note: unit tests may want to use the constructor below to avoid setting watch by calling readTenants().
         this.globalComponentRegistry = globalComponentRegistry;
         this.curator = globalComponentRegistry.getCurator();
-        metricUpdater = metrics.getOrCreateMetricUpdater(Collections.<String, String>emptyMap());
+        metricUpdater = metrics.getOrCreateMetricUpdater(Collections.emptyMap());
         this.tenantListeners.add(globalComponentRegistry.getTenantListener());
         curator.framework().getConnectionStateListenable().addListener(this);
 
@@ -95,19 +94,13 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen
         createSystemTenants();
         curator.create(vespaPath);
 
-        this.directoryCache = globalComponentRegistry.getCurator().createDirectoryCache(tenantsPath.getAbsolute(), false, false, pathChildrenExecutor);
+        this.directoryCache = curator.createDirectoryCache(tenantsPath.getAbsolute(), false, false, pathChildrenExecutor);
         directoryCache.start();
         directoryCache.addListener(this);
         tenantsChanged(readTenants());
         notifyTenantsLoaded();
     }
 
-    private void notifyTenantsLoaded() {
-        for (TenantListener tenantListener : tenantListeners) {
-            tenantListener.onTenantsLoaded();
-        }
-    }
-
     /**
      * New instance containing the given tenants. This will not watch in ZooKeeper. 
      * @param globalComponentRegistry a {@link com.yahoo.vespa.config.server.GlobalComponentRegistry} instance
@@ -124,6 +117,12 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen
         this.tenants.putAll(addTenants(tenants));
     }
 
+    private void notifyTenantsLoaded() {
+        for (TenantListener tenantListener : tenantListeners) {
+            tenantListener.onTenantsLoaded();
+        }
+    }
+
     // Pre-condition: tenants path needs to exist in zk
     private LinkedHashMap<TenantName, Tenant> addTenants(Collection<Tenant> newTenants) {
         LinkedHashMap<TenantName, Tenant> sessionTenants = new LinkedHashMap<>();
@@ -326,18 +325,6 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen
         pathChildrenExecutor.shutdown();
     }
 
-    // TODO: Deploy applications in parallel (with throttling)
-    public void redeployApplications(Deployer deployer) {
-        Set<Tenant> allTenants = ImmutableSet.copyOf(tenants.values());
-        int totalNumberOfApplications = allTenants.stream()
-                .mapToInt(tenant -> tenant.getApplicationRepo().listApplications().size()).sum();
-        int applicationsRedeployed = 0;
-        for (Tenant tenant : allTenants) {
-            tenant.redeployApplications(deployer);
-            applicationsRedeployed += redeployProgress(tenant, applicationsRedeployed, totalNumberOfApplications);
-        }
-    }
-
     public boolean checkThatTenantExists(TenantName tenant) {
         return tenants.containsKey(tenant);
     }
@@ -346,16 +333,12 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen
         return tenants.get(tenantName);
     }
 
-    public Set<TenantName> getAllTenants() {
+    public Set<TenantName> getAllTenantNames() {
         return ImmutableSet.copyOf(tenants.keySet());
     }
 
-    private static int redeployProgress(Tenant tenant, int applicationsRedeployed, int totalNumberOfApplications) {
-        int size = tenant.getApplicationRepo().listApplications().size();
-        if (size > 0) {
-            log.log(LogLevel.INFO, String.format("Redeployed %s of %s applications", applicationsRedeployed + size, totalNumberOfApplications));
-        }
-        return size;
+    public Collection<Tenant> getAllTenants() {
+        return ImmutableSet.copyOf(tenants.values());
     }
 
     /**
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
index 5fbe4fba64d..2443748a99c 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
@@ -2,14 +2,27 @@
 package com.yahoo.vespa.config.server;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ApplicationName;
+import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.io.IOUtils;
+import com.yahoo.vespa.config.server.application.ApplicationConvergenceChecker;
+import com.yahoo.vespa.config.server.application.HttpProxy;
+import com.yahoo.vespa.config.server.application.LogServerLogGrabber;
+import com.yahoo.vespa.config.server.deploy.MockDeployer;
 import com.yahoo.vespa.config.server.host.HostRegistries;
+import com.yahoo.vespa.config.server.http.SimpleHttpFetcher;
+import com.yahoo.vespa.config.server.http.v2.SessionActiveHandlerTest;
 import com.yahoo.vespa.config.server.monitoring.Metrics;
+import com.yahoo.vespa.config.server.provision.HostProvisionerProvider;
 import com.yahoo.vespa.config.server.rpc.UncompressedConfigResponseFactory;
+import com.yahoo.vespa.config.server.tenant.Tenant;
 import com.yahoo.vespa.config.server.tenant.TenantRequestHandler;
 import com.yahoo.vespa.config.server.tenant.TestWithTenant;
 import com.yahoo.vespa.config.server.version.VersionState;
+import org.hamcrest.core.Is;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -29,9 +42,28 @@ import static org.junit.Assert.assertTrue;
  * @since 5.1
  */
 public class ConfigServerBootstrapTest extends TestWithTenant {
+    private final TenantName tenant1 = TenantName.from("tenant1");
+    private final TenantName tenant2 = TenantName.from("tenant2");
+
+    private ApplicationRepository applicationRepository;
+
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
+    @Before
+    public void setup() {
+        tenants.writeTenantPath(tenant1);
+        tenants.writeTenantPath(tenant2);
+
+        applicationRepository = new ApplicationRepository(tenants,
+                                                          HostProvisionerProvider.withProvisioner(new SessionActiveHandlerTest.MockProvisioner()),
+                                                          curator,
+                                                          new LogServerLogGrabber(),
+                                                          new ApplicationConvergenceChecker(),
+                                                          new HttpProxy(new SimpleHttpFetcher()),
+                                                          new ConfigserverConfig(new ConfigserverConfig.Builder()));
+    }
+
     @Test
     public void testConfigServerBootstrap() throws Exception {
         File versionFile = temporaryFolder.newFile();
@@ -43,7 +75,7 @@ public class ConfigServerBootstrapTest extends TestWithTenant {
         assertFalse(myServer.stopped);
         VersionState versionState = new VersionState(versionFile);
         assertTrue(versionState.isUpgraded());
-        ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tenants, rpc, (application, timeout) -> Optional.empty(), versionState);
+        ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(applicationRepository, rpc, (application, timeout) -> Optional.empty(), versionState);
         waitUntilStarted(rpc, 60000);
         assertFalse(versionState.isUpgraded());
         assertThat(versionState.currentVersion(), is(versionState.storedVersion()));
@@ -55,6 +87,16 @@ public class ConfigServerBootstrapTest extends TestWithTenant {
         assertTrue(rpc.stopped);
     }
 
+    @Test
+    public void testTenantRedeployment() throws Exception {
+        MockDeployer deployer = new MockDeployer();
+        Tenant tenant = tenants.getTenant(tenant1);
+        ApplicationId id = ApplicationId.from(tenant1, ApplicationName.defaultName(), InstanceName.defaultName());
+        tenant.getApplicationRepo().createPutApplicationTransaction(id, 3).commit();
+        applicationRepository.redeployAllApplications(deployer);
+        assertThat(deployer.lastDeployed, Is.is(id));
+    }
+
     private void waitUntilStarted(MockRpc server, long timeout) throws InterruptedException {
         long start = System.currentTimeMillis();
         while ((System.currentTimeMillis() - start) < timeout) {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java
index 31e8d545b7a..a2b4335d4ac 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java
@@ -37,7 +37,7 @@ import static org.junit.Assert.fail;
 
 public class TenantsTestCase extends TestWithCurator {
     private Tenants tenants;
-    TestComponentRegistry globalComponentRegistry;
+    private TestComponentRegistry globalComponentRegistry;
     private TenantRequestHandlerTest.MockReloadListener listener;
     private MockTenantListener tenantListener;
     private final TenantName tenant1 = TenantName.from("tenant1");
@@ -80,19 +80,19 @@ public class TenantsTestCase extends TestWithCurator {
     @Test
     public void testTenantListenersNotified() throws Exception {
         tenants.writeTenantPath(tenant3);
-        assertThat("tenant3 not the last created tenant. Tenants: " + tenants.getAllTenants() + ", /config/v2/tenants: " + readZKChildren("/config/v2/tenants"), tenantListener.tenantCreatedName, is(tenant3));
+        assertThat("tenant3 not the last created tenant. Tenants: " + tenants.getAllTenantNames() + ", /config/v2/tenants: " + readZKChildren("/config/v2/tenants"), tenantListener.tenantCreatedName, is(tenant3));
         tenants.deleteTenant(tenant2);
-        assertFalse(tenants.getAllTenants().contains(tenant2));
+        assertFalse(tenants.getAllTenantNames().contains(tenant2));
         assertThat(tenantListener.tenantDeletedName, is(tenant2));
     }
 
     @Test
     public void testAddTenant() throws Exception {
-        Set<TenantName> allTenants = tenants.getAllTenants();
+        Set<TenantName> allTenants = tenants.getAllTenantNames();
         assertTrue(allTenants.contains(tenant1));
         assertTrue(allTenants.contains(tenant2));
         tenants.writeTenantPath(tenant3);
-        allTenants = tenants.getAllTenants();
+        allTenants = tenants.getAllTenantNames();
         assertTrue(allTenants.contains(tenant1));
         assertTrue(allTenants.contains(tenant2));
         assertTrue(allTenants.contains(tenant3));
@@ -108,7 +108,7 @@ public class TenantsTestCase extends TestWithCurator {
     public void testRemove() throws Exception {
         assertNotNull(globalComponentRegistry.getCurator().framework().checkExists().forPath(tenants.tenantZkPath(tenant1)));
         tenants.deleteTenant(tenant1);
-        assertFalse(tenants.getAllTenants().contains(tenant1));
+        assertFalse(tenants.getAllTenantNames().contains(tenant1));
     }
     
     @Test
@@ -120,14 +120,14 @@ public class TenantsTestCase extends TestWithCurator {
         newTenants.add(tenant2);
         newTenants.add(defaultTenant);
         tenants.tenantsChanged(newTenants);
-        Set<TenantName> allTenants = tenants.getAllTenants();
+        Set<TenantName> allTenants = tenants.getAllTenantNames();
         assertTrue(allTenants.contains(tenant2));
         assertEquals("default", defaultTenant.value());
         assertTrue(allTenants.contains(defaultTenant));
         assertFalse(allTenants.contains(tenant1));
         newTenants.clear();
         tenants.tenantsChanged(newTenants);
-        allTenants = tenants.getAllTenants();
+        allTenants = tenants.getAllTenantNames();
         assertFalse(allTenants.contains(tenant1));
         assertFalse(allTenants.contains(tenant2));
         assertFalse(allTenants.contains(defaultTenant));
@@ -138,7 +138,7 @@ public class TenantsTestCase extends TestWithCurator {
         newTenants.add(foo);
         newTenants.add(bar);
         tenants.tenantsChanged(newTenants);
-        allTenants = tenants.getAllTenants();
+        allTenants = tenants.getAllTenantNames();
         assertTrue(allTenants.contains(tenant2));
         assertTrue(allTenants.contains(foo));
         assertTrue(allTenants.contains(bar));
@@ -149,13 +149,13 @@ public class TenantsTestCase extends TestWithCurator {
         TestComponentRegistry reg = new TestComponentRegistry.Builder().curator(curator).build();
         Tenants t = new Tenants(reg, Metrics.createTestMetrics());
         try {
-            assertTrue(t.getAllTenants().contains(TenantName.defaultName()));
+            assertTrue(t.getAllTenantNames().contains(TenantName.defaultName()));
             reg.getCurator().framework().create().forPath(tenants.tenantZkPath(TenantName.from("newTenant")));
             // Poll for the watcher to pick up the tenant from zk, and add it
             int tries=0;
             while(true) {
                 if (tries > 500) fail("Didn't react on watch");
-                if (t.getAllTenants().contains(TenantName.from("newTenant"))) {
+                if (t.getAllTenantNames().contains(TenantName.from("newTenant"))) {
                     return;
                 }
                 tries++;
@@ -166,14 +166,5 @@ public class TenantsTestCase extends TestWithCurator {
         }
     }
 
-    @Test
-    public void testTenantRedeployment() throws Exception {
-        MockDeployer deployer = new MockDeployer();
-        Tenant tenant = tenants.getTenant(tenant1);
-        ApplicationId id = ApplicationId.from(tenant1, ApplicationName.defaultName(), InstanceName.defaultName());
-        tenant.getApplicationRepo().createPutApplicationTransaction(id, 3).commit();
-        tenants.redeployApplications(deployer);
-        assertThat(deployer.lastDeployed, is(id));
-    }
 
 }