ComponentsProviderImpl to implement deconstruct and shut down everything

6 files changed, 45 insertions, 42 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java
index e02f81e8f30..cf70963eee1 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java
@@ -62,5 +62,5 @@ public interface NodeAdmin {
     /**
      * Stop the NodeAgent. Will not delete the storage or stop the container.
      */
-    void shutdown();
+    void stop();
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java
index e38a17786cd..f227a166034 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java
@@ -179,24 +179,21 @@ public class NodeAdminImpl implements NodeAdmin {
     }
 
     @Override
-    public void shutdown() {
+    public void stop() {
         metricsScheduler.shutdown();
         aclScheduler.shutdown();
-        try {
-            boolean metricsSchedulerShutdown = metricsScheduler.awaitTermination(30, TimeUnit.SECONDS);
-            boolean aclSchedulerShutdown = aclScheduler.awaitTermination(30, TimeUnit.SECONDS);
-            if (! (metricsSchedulerShutdown && aclSchedulerShutdown)) {
-                throw new RuntimeException("Failed shutting down all scheduler(s), shutdown status:\n" +
-                        "\tMetrics Scheduler: " + metricsSchedulerShutdown + "\n" +
-                        "\tACL Scheduler: " + aclSchedulerShutdown);
-            }
-        } catch (InterruptedException e) {
-            throw new RuntimeException(e);
-        }
 
-        for (NodeAgent nodeAgent : nodeAgents.values()) {
-            nodeAgent.stop();
-        }
+        // Stop all node-agents in parallel, will block until the last NodeAgent is stopped
+        nodeAgents.values().parallelStream().forEach(NodeAgent::stop);
+
+        do {
+            try {
+                metricsScheduler.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
+                aclScheduler.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
+            } catch (InterruptedException e) {
+                logger.info("Was interrupted while waiting for metricsScheduler and aclScheduler to shutdown");
+            }
+        } while (!metricsScheduler.isTerminated() || !aclScheduler.isTerminated());
     }
 
     // Set-difference. Returns minuend minus subtrahend.
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java
index cf47e3c2da5..5adcc1d3e9c 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.nodeadmin;
 
-import com.yahoo.component.AbstractComponent;
 import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
@@ -36,7 +35,7 @@ import static com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdater.
  *
  * @author dybis, stiankri
  */
-public class NodeAdminStateUpdater extends AbstractComponent {
+public class NodeAdminStateUpdater {
     static final Duration FREEZE_CONVERGENCE_TIMEOUT = Duration.ofMinutes(5);
 
     private final AtomicBoolean terminated = new AtomicBoolean(false);
@@ -275,22 +274,27 @@ public class NodeAdminStateUpdater extends AbstractComponent {
         loopThread.start();
     }
 
-    @Override
-    public void deconstruct() {
+    public void stop() {
+        specVerifierScheduler.shutdown();
         if (!terminated.compareAndSet(false, true)) {
             throw new RuntimeException("Can not re-stop a node agent.");
         }
         log.log(LogLevel.INFO, objectToString() + ": Deconstruct called");
+
+        // First we need to stop NodeAdminStateUpdater thread to make sure no new NodeAgents are spawned
         signalWorkToBeDone();
-        try {
-            loopThread.join(10000);
-            if (loopThread.isAlive()) {
-                log.log(LogLevel.ERROR, "Could not stop tick thread");
+
+        do {
+            try {
+                loopThread.join(0);
+                specVerifierScheduler.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
+            } catch (InterruptedException e1) {
+                log.info("Interrupted while waiting for NodeAdminStateUpdater thread and specVerfierScheduler to shutdown");
             }
-        } catch (InterruptedException e1) {
-            log.log(LogLevel.ERROR, "Interrupted; Could not stop thread");
-        }
-        nodeAdmin.shutdown();
+        } while (loopThread.isAlive() || !specVerifierScheduler.isTerminated());
+
+        // Finally, stop NodeAdmin and all the NodeAgents
+        nodeAdmin.stop();
         log.log(LogLevel.INFO, objectToString() + ": Deconstruct complete");
     }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index 4f07658f320..d0c8b3b3837 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -210,19 +210,15 @@ public class NodeAgentImpl implements NodeAgent {
             throw new RuntimeException("Can not re-stop a node agent.");
         }
         signalWorkToBeDone();
-        try {
-            loopThread.join(10000);
-            if (loopThread.isAlive()) {
-                logger.error("Could not stop host thread " + hostname);
+
+        do {
+            try {
+                loopThread.join(0);
+                filebeatRestarter.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
+            } catch (InterruptedException e) {
+                logger.error("Interrupted while waiting for converge thread and filebeatRestarter scheduler to shutdown");
             }
-        } catch (InterruptedException e1) {
-            logger.error("Interrupted; Could not stop host thread " + hostname);
-        }
-        try {
-            filebeatRestarter.awaitTermination(10, TimeUnit.SECONDS);
-        } catch (InterruptedException e) {
-            logger.error("Interrupted; Could not stop filebeatrestarter thread");
-        }
+        } while (loopThread.isAlive() || !filebeatRestarter.isTerminated());
 
         logger.info("Stopped");
     }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
index 1e57c55d687..73b187b7e08 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.node.admin.provider;
 
 import com.google.inject.Inject;
+import com.yahoo.component.AbstractComponent;
 import com.yahoo.net.HostName;
 
 import com.yahoo.system.ProcessExecuter;
@@ -34,7 +35,7 @@ import static com.yahoo.vespa.defaults.Defaults.getDefaults;
  *
  * @author dybis
  */
-public class ComponentsProviderImpl implements ComponentsProvider {
+public class ComponentsProviderImpl extends AbstractComponent implements ComponentsProvider {
     private final NodeAdminStateUpdater nodeAdminStateUpdater;
 
     private static final int WEB_SERVICE_PORT = getDefaults().vespaWebServicePort();
@@ -71,4 +72,9 @@ public class ComponentsProviderImpl implements ComponentsProvider {
     public NodeAdminStateUpdater getNodeAdminStateUpdater() {
         return nodeAdminStateUpdater;
     }
+
+    @Override
+    public void deconstruct() {
+        nodeAdminStateUpdater.stop();
+    }
 }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
index 59cd1b653f9..ab752bbe4c0 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
@@ -76,6 +76,6 @@ public class DockerTester implements AutoCloseable {
 
     @Override
     public void close() {
-        nodeAdminStateUpdater.deconstruct();
+        nodeAdminStateUpdater.stop();
     }
 }