diff options
author | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2022-02-04 13:48:06 +0100 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2022-02-04 13:48:06 +0100 |
commit | 17dfbf4625386d1f37313f2da9f6f9b4c7e96c4c (patch) | |
tree | aed2ab71aba51a593d63f6962e03bca29fc3c33e | |
parent | bbbfcfa38c01142e8fd56b48fb08da30cd38383d (diff) |
Shutdown reconfiguration thread in a more controlled way
Use interrupts is a horrible mechanism as we cannot control which part of the code receives the signal.
5 files changed, 54 insertions, 48 deletions
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java index bcd3665a3d6..decb0513e26 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java @@ -145,6 +145,8 @@ public class HandlersConfigurerDi { public void shutdown() { container.shutdown(currentGraph); } + public void shutdownConfigRetriever() { container.shutdownConfigRetriever(); } + /** Returns the currently active application configuration generation */ public long generation() { return currentGraph.generation(); } diff --git a/container-core/src/main/java/com/yahoo/container/di/Container.java b/container-core/src/main/java/com/yahoo/container/di/Container.java index 5d5906c7a37..b8bc5fc6c99 100644 --- a/container-core/src/main/java/com/yahoo/container/di/Container.java +++ b/container-core/src/main/java/com/yahoo/container/di/Container.java @@ -254,14 +254,14 @@ public class Container { } public void shutdown(ComponentGraph graph) { - shutdownConfigurer(); + shutdownConfigRetriever(); if (graph != null) { scheduleGraphForDeconstruction(graph); destructor.shutdown(); } } - void shutdownConfigurer() { + public void shutdownConfigRetriever() { retriever.shutdown(); } diff --git a/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java b/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java index 1570c529cdf..74c08b1044b 100644 --- a/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java +++ b/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java @@ -44,7 +44,7 @@ public class ContainerTest extends ContainerTestBase { ComponentTakingConfig component = createComponentTakingConfig(getNewComponentGraph(container)); assertEquals("myString", component.config.stringVal()); - container.shutdownConfigurer(); + container.shutdownConfigRetriever(); } @Test @@ -66,7 +66,7 @@ public class ContainerTest extends ContainerTestBase { ComponentTakingConfig component2 = createComponentTakingConfig(newComponentGraph); assertEquals("reconfigured", component2.config.stringVal()); - container.shutdownConfigurer(); + container.shutdownConfigRetriever(); } @Test @@ -90,7 +90,7 @@ public class ContainerTest extends ContainerTestBase { assertNotNull(ComponentGraph.getNode(newGraph, "id1")); assertNotNull(ComponentGraph.getNode(newGraph, "id2")); - container.shutdownConfigurer(); + container.shutdownConfigRetriever(); } //@Test TODO diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java index 24dcf18b555..8f9e7a2bbda 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java @@ -94,14 +94,15 @@ public final class ConfiguredApplication implements Application { new ComponentRegistry<>()); private final OsgiFramework restrictedOsgiFramework; private final Phaser nonTerminatedContainerTracker = new Phaser(1); + private final Thread reconfigurerThread; private HandlersConfigurerDi configurer; - private Thread reconfigurerThread; private Thread portWatcher; private QrConfig qrConfig; private Register slobrokRegistrator = null; private Supervisor supervisor = null; private Acceptor acceptor = null; + private volatile boolean shutdownReconfiguration = false; static { LogSetup.initVespaLogging("Container"); @@ -135,6 +136,7 @@ public final class ConfiguredApplication implements Application { : Optional.empty(); this.restrictedOsgiFramework = new DisableOsgiFramework(new RestrictedBundleContext(osgiFramework.bundleContext())); this.shutdownDeadline = new ShutdownDeadline(configId); + this.reconfigurerThread = new Thread(this::doReconfigurationLoop, "configured-application-reconfigurer"); } @Override @@ -146,7 +148,7 @@ public final class ConfiguredApplication implements Application { ContainerBuilder builder = createBuilderWithGuiceBindings(); configurer = createConfigurer(builder.guiceModules().activate()); initializeAndActivateContainer(builder, () -> {}); - startReconfigurerThread(); + reconfigurerThread.start(); portWatcher = new Thread(this::watchPortChange, "configured-application-port-watcher"); portWatcher.setDaemon(true); portWatcher.start(); @@ -286,30 +288,27 @@ public final class ConfiguredApplication implements Application { } @SuppressWarnings("removal") // TODO Vespa 8: remove - private void startReconfigurerThread() { - reconfigurerThread = new Thread(() -> { - while ( ! Thread.interrupted()) { - try { - ContainerBuilder builder = createBuilderWithGuiceBindings(); - - // Block until new config arrives, and it should be applied - Runnable cleanupTask = configurer.waitForNextGraphGeneration(builder.guiceModules().activate(), false); - initializeAndActivateContainer(builder, cleanupTask); - } catch (UncheckedInterruptedException | ConfigInterruptedException e) { - break; - } catch (Exception | LinkageError e) { // LinkageError: OSGi problems - tryReportFailedComponentGraphConstructionMetric(configurer, e); - log.log(Level.SEVERE, - "Reconfiguration failed, your application package must be fixed, unless this is a " + - "JNI reload issue: " + Exceptions.toMessageString(e), e); - } catch (Error e) { - com.yahoo.protect.Process.logAndDie("java.lang.Error on reconfiguration: We are probably in " + - "a bad state and will terminate", e); - } + private void doReconfigurationLoop() { + while (!shutdownReconfiguration) { + try { + ContainerBuilder builder = createBuilderWithGuiceBindings(); + + // Block until new config arrives, and it should be applied + Runnable cleanupTask = configurer.waitForNextGraphGeneration(builder.guiceModules().activate(), false); + initializeAndActivateContainer(builder, cleanupTask); + } catch (UncheckedInterruptedException | ConfigInterruptedException e) { + break; + } catch (Exception | LinkageError e) { // LinkageError: OSGi problems + tryReportFailedComponentGraphConstructionMetric(configurer, e); + log.log(Level.SEVERE, + "Reconfiguration failed, your application package must be fixed, unless this is a " + + "JNI reload issue: " + Exceptions.toMessageString(e), e); + } catch (Error e) { + com.yahoo.protect.Process.logAndDie("java.lang.Error on reconfiguration: We are probably in " + + "a bad state and will terminate", e); } - log.fine("Shutting down HandlersConfigurerDi"); - }, "configured-application-reconfigurer"); - reconfigurerThread.start(); + } + log.fine("Reconfiguration loop exited"); } private static void tryReportFailedComponentGraphConstructionMetric(HandlersConfigurerDi configurer, Throwable error) { @@ -397,7 +396,7 @@ public final class ConfiguredApplication implements Application { } private void stopServersAndAwaitTermination(String logPrefix) { - shutdownReconfigurerThread(); + shutdownReconfigurer(); log.info(logPrefix + ": Closing servers"); startAndStopServers(List.of()); startAndRemoveClients(List.of()); @@ -407,20 +406,24 @@ public final class ConfiguredApplication implements Application { log.info(logPrefix + ": Finished"); } - // TODO Do more graceful shutdown of reconfigurer thread. The interrupt may leave the container in state where - // graceful shutdown is impossible or may hang. - private void shutdownReconfigurerThread() { + private void shutdownReconfigurer() { + if (!reconfigurerThread.isAlive()) { + log.info("Reconfiguration thread shutdown already completed"); + return; + } + log.info("Shutting down reconfiguration thread"); + long start = System.currentTimeMillis(); + shutdownReconfiguration = true; + configurer.shutdownConfigRetriever(); try { - //Workaround for component constructors masking InterruptedException. - while (reconfigurerThread != null && reconfigurerThread.isAlive()) { - reconfigurerThread.interrupt(); - long millis = 200; - reconfigurerThread.join(millis); - reconfigurerThread = null; - } + reconfigurerThread.join(); + log.info(String.format( + "Reconfiguration shutdown completed in %.3f seconds", (System.currentTimeMillis() - start) / 1000D)); } catch (InterruptedException e) { - log.info("Interrupted while joining on HandlersConfigurer reconfigure thread."); - Thread.currentThread().interrupt(); + String message = "Interrupted while waiting for reconfiguration shutdown"; + log.warning(message); + log.log(Level.FINE, e.getMessage(), e); + throw new UncheckedInterruptedException(message, true); } } diff --git a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java index 11f799fcfd4..907c9649912 100644 --- a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java +++ b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java @@ -31,6 +31,7 @@ public class StandaloneSubscriberFactory implements SubscriberFactory { private final Set<ConfigKey<ConfigInstance>> configKeys; private long generation = -1L; + private volatile boolean shutdown = false; StandaloneSubscriber(Set<ConfigKey<ConfigInstance>> configKeys) { this.configKeys = configKeys; @@ -41,9 +42,7 @@ public class StandaloneSubscriberFactory implements SubscriberFactory { return generation == 0; } - @Override - public void close() { - } + @Override public void close() { shutdown = true; } @Override public Map<ConfigKey<ConfigInstance>, ConfigInstance> config() { @@ -64,9 +63,11 @@ public class StandaloneSubscriberFactory implements SubscriberFactory { if (generation != 0) { try { - while (!Thread.interrupted()) { - Thread.sleep(10000); + while (!shutdown && !Thread.interrupted()) { + Thread.sleep(100); } + if (shutdown) // Same semantics as an actual interrupt + throw new ConfigInterruptedException(new InterruptedException()); } catch (InterruptedException e) { throw new ConfigInterruptedException(e); } |