Merge pull request #19916 from vespa-engine/hmusum/start-config-server-maintainers-in-2-phases

Start config server maintainers in 2 phases [run-systemtest]

5 files changed, 98 insertions, 61 deletions

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 c2de581d1d4..57e49ef3e8d 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
@@ -10,8 +10,11 @@ import com.yahoo.config.provision.Deployment;
 import com.yahoo.config.provision.TransientException;
 import com.yahoo.container.handler.VipStatus;
 import com.yahoo.container.jdisc.state.StateMonitor;
+import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker;
+import com.yahoo.vespa.config.server.maintenance.ConfigServerMaintenance;
 import com.yahoo.vespa.config.server.rpc.RpcServer;
 import com.yahoo.vespa.config.server.version.VersionState;
+import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.yolean.Exceptions;
 
 import java.time.Duration;
@@ -69,27 +72,32 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
     private final Duration sleepTimeWhenRedeployingFails;
     private final RedeployingApplicationsFails exitIfRedeployingApplicationsFails;
     private final ExecutorService rpcServerExecutor;
+    private final ConfigServerMaintenance configServerMaintenance;
 
+    @SuppressWarnings("unused") //  Injected component
     @Inject
     public ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
-                                 VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus) {
+                                 VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus,
+                                 FlagSource flagSource, ConfigConvergenceChecker convergence) {
         this(applicationRepository, server, versionState, stateMonitor, vipStatus, BOOTSTRAP_IN_CONSTRUCTOR, EXIT_JVM,
              applicationRepository.configserverConfig().hostedVespa()
                      ? VipStatusMode.VIP_STATUS_FILE
-                     : VipStatusMode.VIP_STATUS_PROGRAMMATICALLY);
+                     : VipStatusMode.VIP_STATUS_PROGRAMMATICALLY,
+             flagSource, convergence);
     }
 
     // For testing only
     ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server, VersionState versionState,
-                          StateMonitor stateMonitor, VipStatus vipStatus, VipStatusMode vipStatusMode) {
+                          StateMonitor stateMonitor, VipStatus vipStatus, VipStatusMode vipStatusMode,
+                          FlagSource flagSource, ConfigConvergenceChecker convergence) {
         this(applicationRepository, server, versionState, stateMonitor, vipStatus,
-             FOR_TESTING_NO_BOOTSTRAP_OF_APPS, CONTINUE, vipStatusMode);
+             FOR_TESTING_NO_BOOTSTRAP_OF_APPS, CONTINUE, vipStatusMode, flagSource, convergence);
     }
 
     private ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
                                   VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus,
                                   Mode mode, RedeployingApplicationsFails exitIfRedeployingApplicationsFails,
-                                  VipStatusMode vipStatusMode) {
+                                  VipStatusMode vipStatusMode, FlagSource flagSource, ConfigConvergenceChecker convergence) {
         this.applicationRepository = applicationRepository;
         this.server = server;
         this.versionState = versionState;
@@ -101,6 +109,12 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
         this.exitIfRedeployingApplicationsFails = exitIfRedeployingApplicationsFails;
         rpcServerExecutor = Executors.newSingleThreadExecutor(new DaemonThreadFactory("config server RPC server"));
 
+        configServerMaintenance = new ConfigServerMaintenance(configserverConfig,
+                                                              applicationRepository,
+                                                              applicationRepository.tenantRepository().getCurator(),
+                                                              flagSource,
+                                                              convergence);
+        configServerMaintenance.startBeforeBootstrap();
         log.log(Level.FINE, () -> "Bootstrap mode: " + mode + ", VIP status mode: " + vipStatusMode);
         initializing(vipStatusMode);
 
@@ -122,6 +136,7 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
         server.stop();
         log.log(Level.FINE, "RPC server stopped");
         rpcServerExecutor.shutdown();
+        configServerMaintenance.shutdown();
     }
 
     @Override
@@ -156,11 +171,14 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
         applicationRepository.bootstrappingDone();
         allowConfigRpcRequests(server);
         up();
+        configServerMaintenance.startAfterBootstrap();
     }
 
-    StateMonitor.Status status() {
-        return stateMonitor.status();
-    }
+    StateMonitor.Status status() { return stateMonitor.status(); }
+
+    VipStatus vipStatus() { return vipStatus; }
+
+    public ConfigServerMaintenance configServerMaintenance() { return configServerMaintenance; }
 
     private void up() {
         vipStatus.setInRotation(true);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java
index 2c0b7e090a6..d4d4a7fa7d3 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java
@@ -63,7 +63,7 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer {
             for (var applicationId : applicationRepository.listApplications()) {
                 log.fine(() -> "Verifying application package for " + applicationId);
                 Session session = applicationRepository.getActiveSession(applicationId);
-                if (session == null) continue;  // App might be deleted after call to listApplications()
+                if (session == null) continue;  // App might be deleted after call to listApplications() or not activated yet (bootstrap phase)
 
                 FileReference applicationPackage = session.getApplicationPackageReference();
                 long sessionId = session.getSessionId();
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
index be19e53115f..a6516ac361b 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
@@ -1,9 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.maintenance;
 
-import com.google.inject.Inject;
 import com.yahoo.cloud.config.ConfigserverConfig;
-import com.yahoo.component.AbstractComponent;
 import com.yahoo.concurrent.maintenance.Maintainer;
 import com.yahoo.vespa.config.server.ApplicationRepository;
 import com.yahoo.vespa.config.server.ConfigServerBootstrap;
@@ -24,31 +22,48 @@ import java.util.concurrent.CopyOnWriteArrayList;
  *
  * @author hmusum
  */
-public class ConfigServerMaintenance extends AbstractComponent {
+public class ConfigServerMaintenance {
 
     private final List<Maintainer> maintainers = new CopyOnWriteArrayList<>();
+    private final ConfigserverConfig configserverConfig;
+    private final ApplicationRepository applicationRepository;
+    private final Curator curator;
+    private final FlagSource flagSource;
+    private final ConfigConvergenceChecker convergenceChecker;
 
-    @Inject
-    public ConfigServerMaintenance(ConfigServerBootstrap configServerBootstrap,
-                                   ConfigserverConfig configserverConfig,
+    public ConfigServerMaintenance(ConfigserverConfig configserverConfig,
                                    ApplicationRepository applicationRepository,
                                    Curator curator,
                                    FlagSource flagSource,
-                                   ConfigConvergenceChecker convergence) {
-        DefaultTimes defaults = new DefaultTimes(configserverConfig);
-        maintainers.add(new TenantsMaintainer(applicationRepository, curator, flagSource, defaults.defaultInterval, Clock.systemUTC()));
-        maintainers.add(new FileDistributionMaintainer(applicationRepository, curator, defaults.defaultInterval, flagSource));
-        maintainers.add(new SessionsMaintainer(applicationRepository, curator, Duration.ofSeconds(30), flagSource));
+                                   ConfigConvergenceChecker convergenceChecker) {
+        this.configserverConfig = configserverConfig;
+        this.applicationRepository = applicationRepository;
+        this.curator = curator;
+        this.flagSource = flagSource;
+        this.convergenceChecker = convergenceChecker;
+    }
+
+    public void startBeforeBootstrap() {
         maintainers.add(new ApplicationPackageMaintainer(applicationRepository, curator, Duration.ofSeconds(30), flagSource));
-        maintainers.add(new ReindexingMaintainer(applicationRepository, curator, flagSource, Duration.ofMinutes(3), convergence, Clock.systemUTC()));
+        maintainers.add(new TenantsMaintainer(applicationRepository, curator, flagSource,
+                                              new DefaultTimes(configserverConfig).defaultInterval, Clock.systemUTC()));
     }
 
-    @Override
-    public void deconstruct() {
+    public void startAfterBootstrap() {
+        maintainers.add(new FileDistributionMaintainer(applicationRepository, curator,
+                                                       new DefaultTimes(configserverConfig).defaultInterval, flagSource));
+        maintainers.add(new SessionsMaintainer(applicationRepository, curator, Duration.ofSeconds(30), flagSource));
+        maintainers.add(new ReindexingMaintainer(applicationRepository, curator, flagSource,
+                                                 Duration.ofMinutes(3), convergenceChecker, Clock.systemUTC()));
+    }
+
+    public void shutdown() {
         maintainers.forEach(Maintainer::shutdown);
         maintainers.forEach(Maintainer::awaitShutdown);
     }
 
+    public List<Maintainer> maintainers() { return List.copyOf(maintainers); }
+
     /*
      * Default values from config. If one of the values needs to be changed, add the value to
      * configserver-config.xml in the config server application directory and restart the config server
diff --git a/configserver/src/main/resources/configserver-app/services.xml b/configserver/src/main/resources/configserver-app/services.xml
index ca6ed4cff28..729b50419d7 100644
--- a/configserver/src/main/resources/configserver-app/services.xml
+++ b/configserver/src/main/resources/configserver-app/services.xml
@@ -35,7 +35,6 @@
     <component id="com.yahoo.vespa.config.server.application.ConfigConvergenceChecker" bundle="configserver" />
     <component id="com.yahoo.vespa.config.server.application.HttpProxy" bundle="configserver" />
     <component id="com.yahoo.vespa.config.server.filedistribution.FileServer" bundle="configserver" />
-    <component id="com.yahoo.vespa.config.server.maintenance.ConfigServerMaintenance" bundle="configserver" />
     <component id="com.yahoo.vespa.config.server.rpc.RpcRequestHandlerProvider" bundle="configserver" />
     <component id="com.yahoo.vespa.config.server.rpc.security.DummyNodeIdentifierProvider" bundle="configserver" />
     <component id="com.yahoo.vespa.config.server.rpc.security.DefaultRpcAuthorizerProvider" bundle="configserver" />
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 69b9d67f84c..babc7b79b65 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
@@ -3,6 +3,7 @@ package com.yahoo.vespa.config.server;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.component.Version;
+import com.yahoo.concurrent.maintenance.Maintainer;
 import com.yahoo.config.model.provision.Host;
 import com.yahoo.config.model.provision.Hosts;
 import com.yahoo.config.model.provision.InMemoryProvisioner;
@@ -18,11 +19,13 @@ import com.yahoo.container.jdisc.state.StateMonitor;
 import com.yahoo.docproc.jdisc.metric.NullMetric;
 import com.yahoo.path.Path;
 import com.yahoo.text.Utf8;
+import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker;
 import com.yahoo.vespa.config.server.deploy.DeployTester;
 import com.yahoo.vespa.config.server.rpc.RpcServer;
 import com.yahoo.vespa.config.server.version.VersionState;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -37,6 +40,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.BooleanSupplier;
+import java.util.stream.Collectors;
 
 import static com.yahoo.vespa.config.server.ConfigServerBootstrap.VipStatusMode.VIP_STATUS_FILE;
 import static com.yahoo.vespa.config.server.ConfigServerBootstrap.VipStatusMode.VIP_STATUS_PROGRAMMATICALLY;
@@ -64,28 +68,31 @@ public class ConfigServerBootstrapTest {
                 .configserverConfig(configserverConfig).hostProvisioner(provisioner).build();
         tester.deployApp("src/test/apps/hosted/");
 
-        VersionState versionState = createVersionState();
-        assertTrue(versionState.isUpgraded());
-
         RpcServer rpcServer = createRpcServer(configserverConfig);
         // Take a host away so that there are too few for the application, to verify we can still bootstrap
         provisioner.allocations().values().iterator().next().remove(0);
-        StateMonitor stateMonitor = StateMonitor.createForTesting();
-        VipStatus vipStatus = createVipStatus(stateMonitor);
-        ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState,
-                                                                    stateMonitor, vipStatus, VIP_STATUS_PROGRAMMATICALLY);
-        assertFalse(vipStatus.isInRotation());
+        ConfigServerBootstrap bootstrap = createBootstrap(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
+        assertEquals(List.of("ApplicationPackageMaintainer", "TenantsMaintainer"),
+                     bootstrap.configServerMaintenance().maintainers().stream()
+                              .map(Maintainer::name)
+                              .sorted().collect(Collectors.toList()));
+        assertFalse(bootstrap.vipStatus().isInRotation());
+
         bootstrap.start();
         waitUntil(rpcServer::isRunning, "failed waiting for Rpc server running");
         assertTrue(rpcServer.isServingConfigRequests());
         waitUntil(() -> bootstrap.status() == StateMonitor.Status.up, "failed waiting for status 'up'");
-        waitUntil(vipStatus::isInRotation, "failed waiting for server to be in rotation");
+        waitUntil(() -> bootstrap.vipStatus().isInRotation(), "failed waiting for server to be in rotation");
+        assertEquals(List.of("ApplicationPackageMaintainer", "FileDistributionMaintainer", "ReindexingMaintainer", "SessionsMaintainer", "TenantsMaintainer"),
+                     bootstrap.configServerMaintenance().maintainers().stream()
+                              .map(Maintainer::name)
+                              .sorted().collect(Collectors.toList()));
 
         bootstrap.deconstruct();
         assertEquals(StateMonitor.Status.down, bootstrap.status());
         assertFalse(rpcServer.isRunning());
         assertTrue(rpcServer.isServingConfigRequests());
-        assertFalse(vipStatus.isInRotation());
+        assertFalse(bootstrap.vipStatus().isInRotation());
     }
 
     // Just tests setup, the actual response of accessing /status.html depends on the status
@@ -98,21 +105,15 @@ public class ConfigServerBootstrapTest {
                 .configserverConfig(configserverConfig).hostProvisioner(provisioner).build();
         tester.deployApp("src/test/apps/hosted/");
 
-        VersionState versionState = createVersionState();
-        assertTrue(versionState.isUpgraded());
-
         RpcServer rpcServer = createRpcServer(configserverConfig);
-        StateMonitor stateMonitor = StateMonitor.createForTesting();
-        VipStatus vipStatus = createVipStatus(stateMonitor);
-        ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState,
-                                                                    stateMonitor, vipStatus, VIP_STATUS_FILE);
-        assertTrue(vipStatus.isInRotation()); // default is in rotation when using status file
+        ConfigServerBootstrap bootstrap = createBootstrap(tester, rpcServer, VIP_STATUS_FILE);
+        assertTrue(bootstrap.vipStatus().isInRotation()); // default is in rotation when using status file
 
         bootstrap.start();
         waitUntil(rpcServer::isRunning, "failed waiting for Rpc server running");
         assertTrue(rpcServer.isServingConfigRequests());
         waitUntil(() -> bootstrap.status() == StateMonitor.Status.up, "failed waiting for status 'up'");
-        waitUntil(vipStatus::isInRotation, "failed waiting for server to be in rotation");
+        waitUntil(() -> bootstrap.vipStatus().isInRotation(), "failed waiting for server to be in rotation");
         bootstrap.deconstruct();
     }
 
@@ -123,9 +124,6 @@ public class ConfigServerBootstrapTest {
                 .configserverConfig(configserverConfig).build();
         tester.deployApp("src/test/apps/hosted/");
 
-        VersionState versionState = createVersionState();
-        assertTrue(versionState.isUpgraded());
-
         // Manipulate application package so that it will fail deployment when config server starts
         java.nio.file.Files.delete(Paths.get(configserverConfig.configServerDBDir())
                                            .resolve("tenants/")
@@ -133,19 +131,16 @@ public class ConfigServerBootstrapTest {
                                            .resolve("sessions/2/services.xml"));
 
         RpcServer rpcServer = createRpcServer(configserverConfig);
-        StateMonitor stateMonitor = StateMonitor.createForTesting();
-        VipStatus vipStatus = createVipStatus(stateMonitor);
-        ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState,
-                                                                    stateMonitor, vipStatus, VIP_STATUS_PROGRAMMATICALLY);
-        assertFalse(vipStatus.isInRotation());
+        ConfigServerBootstrap bootstrap = createBootstrap(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
+        assertFalse(bootstrap.vipStatus().isInRotation());
         // Call method directly, to be sure that it is finished redeploying all applications and we can check status
         bootstrap.start();
         // App is invalid, bootstrapping was unsuccessful. Status should be 'initializing',
         // rpc server should not be running and it should be out of rotation
-        assertEquals(StateMonitor.Status.initializing, stateMonitor.status());
+        assertEquals(StateMonitor.Status.initializing, bootstrap.status());
         assertTrue(rpcServer.isRunning());
         assertFalse(rpcServer.isServingConfigRequests());
-        assertFalse(vipStatus.isInRotation());
+        assertFalse(bootstrap.vipStatus().isInRotation());
 
         bootstrap.deconstruct();
     }
@@ -168,24 +163,34 @@ public class ConfigServerBootstrapTest {
         tester.deployApp("src/test/apps/app/", vespaVersion);
         ApplicationId applicationId = tester.applicationId();
 
-        VersionState versionState = createVersionState();
-        assertTrue(versionState.isUpgraded());
-
         // Ugly hack, but I see no other way of doing it:
         // Manipulate application version in zookeeper so that it is an older version than the model we know, which is
         // the case when upgrading on non-hosted installations
         curator.set(Path.fromString("/config/v2/tenants/" + applicationId.tenant().value() + "/sessions/2/version"), Utf8.toBytes("1.2.2"));
 
         RpcServer rpcServer = createRpcServer(configserverConfig);
-        StateMonitor stateMonitor = StateMonitor.createForTesting();
-        VipStatus vipStatus = createVipStatus(stateMonitor);
-        ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState,
-                                                                    stateMonitor, vipStatus, VIP_STATUS_PROGRAMMATICALLY);
+        ConfigServerBootstrap bootstrap = createBootstrap(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
         bootstrap.start();
         waitUntil(rpcServer::isRunning, "failed waiting for Rpc server running");
         assertTrue(rpcServer.isServingConfigRequests());
         waitUntil(() -> bootstrap.status() == StateMonitor.Status.up, "failed waiting for status 'up'");
-        waitUntil(vipStatus::isInRotation, "failed waiting for server to be in rotation");
+        waitUntil(() -> bootstrap.vipStatus().isInRotation(), "failed waiting for server to be in rotation");
+    }
+
+    private ConfigServerBootstrap createBootstrap(DeployTester tester, RpcServer rpcServer, ConfigServerBootstrap.VipStatusMode vipStatusProgrammatically) throws IOException {
+        VersionState versionState = createVersionState();
+        assertTrue(versionState.isUpgraded());
+
+        StateMonitor stateMonitor = StateMonitor.createForTesting();
+        VipStatus vipStatus = createVipStatus(stateMonitor);
+        return new ConfigServerBootstrap(tester.applicationRepository(),
+                                         rpcServer,
+                                         versionState,
+                                         stateMonitor,
+                                         vipStatus,
+                                         vipStatusProgrammatically,
+                                         new InMemoryFlagSource(),
+                                         new ConfigConvergenceChecker());
     }
 
     private void waitUntil(BooleanSupplier booleanSupplier, String messageIfWaitingFails) throws InterruptedException {