Merge pull request #6082 from vespa-engine/hmusum/control-status.html-when-starting-config-server

Take config server out of rotation (status.html) until it is bootstra…

5 files changed, 82 insertions, 19 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 9793a441355..916fde97e35 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
@@ -3,56 +3,70 @@ package com.yahoo.vespa.config.server;
 
 import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
+import com.yahoo.concurrent.DaemonThreadFactory;
+import com.yahoo.container.handler.VipStatus;
 import com.yahoo.container.jdisc.state.StateMonitor;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.config.server.rpc.RpcServer;
 import com.yahoo.vespa.config.server.version.VersionState;
 
+import java.time.Duration;
+import java.time.Instant;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
 /**
  * Main component that bootstraps and starts config server threads.
  *
- * @author lulf
- * @since 5.1
+ * If config server has been upgraded to a new version since the last time it was running it will redeploy all
+ * applications. If that is done successfully the RPC server will start and the health status code will change from
+ * 'initializing' to 'up' and the config server will be put into rotation (start serving status.html with 200 OK)
+ *
+ * @author Ulf Lilleengen
+ * @author hmusum
  */
 public class ConfigServerBootstrap extends AbstractComponent implements Runnable {
 
     private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(ConfigServerBootstrap.class.getName());
+    private static final ExecutorService rpcServerExecutor  = Executors.newSingleThreadExecutor(new DaemonThreadFactory("config server RPC server"));
+    private static final String vipStatusClusterIdentifier = "configserver";
 
     private final ApplicationRepository applicationRepository;
     private final RpcServer server;
     private final Thread serverThread;
     private final VersionState versionState;
     private final StateMonitor stateMonitor;
+    private final VipStatus vipStatus;
 
     // The tenants object is injected so that all initial requests handlers are
     // added to the rpc server before it starts answering rpc requests.
     @SuppressWarnings("WeakerAccess")
     @Inject
     public ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
-                                 VersionState versionState, StateMonitor stateMonitor) {
-        this(applicationRepository, server, versionState, stateMonitor, true);
+                                 VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus) {
+        this(applicationRepository, server, versionState, stateMonitor, vipStatus, true);
     }
 
     // For testing only
-    ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
-                          VersionState versionState, StateMonitor stateMonitor, boolean startMainThread) {
+    ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server, VersionState versionState,
+                          StateMonitor stateMonitor, VipStatus vipStatus, boolean startMainThread) {
         this.applicationRepository = applicationRepository;
         this.server = server;
         this.versionState = versionState;
         this.stateMonitor = stateMonitor;
         this.serverThread = new Thread(this, "configserver main");
+        this.vipStatus = vipStatus;
+        initializing(); // Initially take server out of rotation
         if (startMainThread)
             start();
     }
 
-    private void start() {
-        serverThread.start();
-    }
-
     @Override
     public void deconstruct() {
         log.log(LogLevel.INFO, "Stopping config server");
+        down();
         server.stop();
+        rpcServerExecutor.shutdown();
         try {
             serverThread.join();
         } catch (InterruptedException e) {
@@ -74,9 +88,8 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
                 return; // Status will not be set to 'up' since we return here
             }
         }
-        stateMonitor.status(StateMonitor.Status.up);
-        log.log(LogLevel.INFO, "Starting RPC server");
-        server.run();
+        startRpcServer();
+        up();
         do {
             try {
                 Thread.sleep(1000);
@@ -85,13 +98,51 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
                 break;
             }
         } while (server.isRunning());
+        down();
         log.log(LogLevel.INFO, "RPC server stopped");
-        stateMonitor.status(StateMonitor.Status.down);
     }
 
     StateMonitor.Status status() {
         return stateMonitor.status();
     }
 
+    private void start() {
+        serverThread.start();
+    }
+
+    private void up() {
+        stateMonitor.status(StateMonitor.Status.up);
+        vipStatus.addToRotation(vipStatusClusterIdentifier);
+    }
+
+    private void down() {
+        stateMonitor.status(StateMonitor.Status.down);
+        vipStatus.removeFromRotation(vipStatusClusterIdentifier);
+    }
+
+    private void initializing() {
+        // This is default value (from config), so not strictly necessary
+        stateMonitor.status(StateMonitor.Status.initializing);
+        vipStatus.removeFromRotation(vipStatusClusterIdentifier);
+    }
+
+    private void startRpcServer() {
+        log.log(LogLevel.INFO, "Starting RPC server");
+        rpcServerExecutor.execute(server);
+
+        Instant end = Instant.now().plus(Duration.ofSeconds(10));
+        while (!server.isRunning() && Instant.now().isBefore(end)) {
+            try {
+                Thread.sleep(10);
+            } catch (InterruptedException e) {
+                log.log(LogLevel.ERROR, "Got interrupted", e);
+                break;
+            }
+        }
+        if (!server.isRunning())
+            throw new RuntimeException("RPC server not started in 10 seconds");
+        log.log(LogLevel.INFO, "RPC server started");
+    }
+
 }
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java
index 9de587ac17b..f1cf479a38a 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java
@@ -166,7 +166,7 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener {
     }
 
     public void run() {
-        log.log(LogLevel.INFO, "Rpc server listening on port " + spec.port());
+        log.log(LogLevel.INFO, "Rpc will listen on port " + spec.port());
         try {
             Acceptor acceptor = supervisor.listen(spec);
             isRunning = true;
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 3557d7bf9ab..8d21a1b8c03 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
@@ -11,7 +11,6 @@ import com.yahoo.vespa.config.server.session.LocalSessionRepo;
 import com.yahoo.vespa.config.server.session.RemoteSessionRepo;
 import com.yahoo.vespa.config.server.session.SessionFactory;
 import com.yahoo.vespa.curator.Curator;
-import org.apache.zookeeper.Op;
 import org.apache.zookeeper.data.Stat;
 
 import java.time.Instant;
diff --git a/configserver/src/main/resources/configserver-app/services.xml b/configserver/src/main/resources/configserver-app/services.xml
index 2eeefda63e7..8b69c3ad825 100644
--- a/configserver/src/main/resources/configserver-app/services.xml
+++ b/configserver/src/main/resources/configserver-app/services.xml
@@ -10,6 +10,10 @@
       <initialStatus>initializing</initialStatus>
     </config>
 
+    <config name="container.core.vip-status">
+      <initiallyInRotation>false</initiallyInRotation>
+    </config>
+
     <accesslog type="vespa" fileNamePattern="logs/vespa/configserver/access.log.%Y%m%d%H%M%S" rotationScheme="date" compressOnRotation="true" symlinkName="access.log" />
     <preprocess:include file='access-logging.xml' required='false' />
 
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 67cc87ae223..c0c3683bebb 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,6 +2,7 @@
 package com.yahoo.vespa.config.server;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
+import com.yahoo.container.handler.VipStatus;
 import com.yahoo.container.jdisc.config.HealthMonitorConfig;
 import com.yahoo.container.jdisc.state.StateMonitor;
 import com.yahoo.jdisc.core.SystemTimer;
@@ -43,13 +44,17 @@ public class ConfigServerBootstrapTest {
         assertTrue(versionState.isUpgraded());
 
         RpcServer rpcServer = createRpcServer(configserverConfig);
-        ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState, createStateMonitor());
+        VipStatus vipStatus = new VipStatus();
+        ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState, createStateMonitor(), vipStatus);
+        assertFalse(vipStatus.isInRotation());
         waitUntil(() -> bootstrap.status() == StateMonitor.Status.up, "failed waiting for status 'up'");
         waitUntil(rpcServer::isRunning, "failed waiting for Rpc server running");
+        assertTrue(vipStatus.isInRotation());
 
         bootstrap.deconstruct();
         assertEquals(StateMonitor.Status.down, bootstrap.status());
         assertFalse(rpcServer.isRunning());
+        assertFalse(vipStatus.isInRotation());
     }
 
     @Test
@@ -69,13 +74,17 @@ public class ConfigServerBootstrapTest {
                                            .resolve("sessions/2/services.xml"));
 
         RpcServer rpcServer = createRpcServer(configserverConfig);
+        VipStatus vipStatus = new VipStatus();
         ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState,
-                                                                    createStateMonitor(), false /* do not call run method */);
+                                                                    createStateMonitor(), vipStatus, false /* do not call run method */);
+        assertFalse(vipStatus.isInRotation());
         // Call method directly, to be sure that it is finished redeploying all applications and we can check status
         bootstrap.run();
-        // App is invalid, so bootstrapping was unsuccessful. Status should be 'initializing' and rpc server should not be running
+        // 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, bootstrap.status());
         assertFalse(rpcServer.isRunning());
+        assertFalse(vipStatus.isInRotation());
     }
 
     private void waitUntil(BooleanSupplier booleanSupplier, String messageIfWaitingFails) throws InterruptedException {