diff options
author | gjoranv <gv@verizonmedia.com> | 2020-12-15 13:39:32 +0100 |
---|---|---|
committer | gjoranv <gv@verizonmedia.com> | 2020-12-15 14:37:14 +0100 |
commit | 08d1933bd7a34d0fae3c4cf0dcd8ba28084d83d1 (patch) | |
tree | 4982fe89a8b60f409a98127b4ed06298c663ea2f /container-disc | |
parent | ad4a9a58f97d7558eff48c6cd857cae3b570d55d (diff) |
Wait for component deconstruction to finish when shutting down.
- Wait up to 10 minuts.
- Add separate modes for reconfig and shutdown.
- Add test ctor for setting the delay before deconstruction starts
in RECONFIG mode. (Tests are TBD.)
Diffstat (limited to 'container-disc')
3 files changed, 65 insertions, 8 deletions
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 835326e0904..f266e3782ef 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 @@ -335,7 +335,7 @@ public final class ConfiguredApplication implements Application { return new HandlersConfigurerDi(subscriberFactory, Container.get(), configId, - new Deconstructor(true), + new Deconstructor(Deconstructor.Mode.RECONFIG), discInjector, osgiFramework); } @@ -367,7 +367,7 @@ public final class ConfiguredApplication implements Application { } log.info("Stop: Shutting container down"); - configurer.shutdown(new Deconstructor(false)); + configurer.shutdown(new Deconstructor(Deconstructor.Mode.SHUTDOWN)); slobrokConfigSubscriber.ifPresent(SlobrokConfigSubscriber::shutdown); Container.get().shutdown(); diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java b/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java index 0bb60c8aca8..47e36809a48 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/component/Deconstructor.java @@ -9,6 +9,10 @@ import com.yahoo.container.di.componentgraph.Provider; import com.yahoo.jdisc.SharedResource; import java.util.List; + + +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; import java.util.logging.Level; import org.osgi.framework.Bundle; import org.osgi.framework.BundleException; @@ -35,13 +39,29 @@ public class Deconstructor implements ComponentDeconstructor { private static final Logger log = Logger.getLogger(Deconstructor.class.getName()); + private static final Duration SHUTDOWN_DECONSTRUCT_TIMEOUT = Duration.ofMinutes(10); + + public enum Mode { + RECONFIG, // Delay deconstruction to allow old components to finish processing in-flight requests. + SHUTDOWN // The container is shutting down. Start deconstructing immediately, and wait until all components + // are deconstructed, to prevent shutting down while deconstruct is in progress. + } + + // TODO: make private again final ScheduledExecutorService executor = - Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getThreadFactory("component-deconstructor")); + Executors.newScheduledThreadPool(2, ThreadFactoryFactory.getThreadFactory("component-deconstructor")); + private final Mode mode; private final Duration delay; - public Deconstructor(boolean delayDeconstruction) { - this.delay = delayDeconstruction ? Duration.ofSeconds(60) : Duration.ZERO; + public Deconstructor(Mode mode) { + this(mode, (mode == Mode.RECONFIG) ? Duration.ofSeconds(60) : Duration.ZERO); + } + + // For testing only + Deconstructor(Mode mode, Duration reconfigDeconstructDelay) { + this.mode = mode; + this.delay = reconfigDeconstructDelay; } @Override @@ -61,9 +81,24 @@ public class Deconstructor implements ComponentDeconstructor { ((SharedResource) component).release(); } } - if (! destructibleComponents.isEmpty() || ! bundles.isEmpty()) - executor.schedule(new DestructComponentTask(destructibleComponents, bundles), + if (!destructibleComponents.isEmpty() || !bundles.isEmpty()) { + var task = executor.schedule(new DestructComponentTask(destructibleComponents, bundles), delay.getSeconds(), TimeUnit.SECONDS); + if (mode.equals(Mode.SHUTDOWN)) { + // Wait for deconstruction to finish + try { + log.info("Waiting up to " + SHUTDOWN_DECONSTRUCT_TIMEOUT.toSeconds() + " seconds for all components to deconstruct."); + task.get(SHUTDOWN_DECONSTRUCT_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + log.info("Interrupted while waiting for component deconstruction to finish."); + Thread.currentThread().interrupt(); + } catch (ExecutionException e) { + log.warning("Component deconstruction threw an exception: " + e.getMessage()); + } catch (TimeoutException e) { + log.warning("Component deconstruction timed out."); + } + } + } } private static class DestructComponentTask implements Runnable { diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/component/DeconstructorTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/component/DeconstructorTest.java index 9ae05f5c193..dfc1d32938f 100644 --- a/container-disc/src/test/java/com/yahoo/container/jdisc/component/DeconstructorTest.java +++ b/container-disc/src/test/java/com/yahoo/container/jdisc/component/DeconstructorTest.java @@ -24,12 +24,13 @@ public class DeconstructorTest { @Before public void init() { - deconstructor = new Deconstructor(false); + deconstructor = new Deconstructor(Deconstructor.Mode.SHUTDOWN); } @Test public void require_abstract_component_destructed() throws InterruptedException { TestAbstractComponent abstractComponent = new TestAbstractComponent(); + // Done by executor, so it takes some time even with a 0 delay. deconstructor.deconstruct(List.of(abstractComponent), emptyList()); deconstructor.executor.shutdown(); @@ -47,6 +48,13 @@ public class DeconstructorTest { } @Test + public void deconstruct_is_synchronous_in_shutdown_mode() { + var slowDeconstructComponent = new SlowDeconstructComponent(); + deconstructor.deconstruct(List.of(slowDeconstructComponent), emptyList()); + assertTrue(slowDeconstructComponent.destructed); + } + + @Test public void require_shared_resource_released() { TestSharedResource sharedResource = new TestSharedResource(); deconstructor.deconstruct(List.of(sharedResource), emptyList()); @@ -68,6 +76,20 @@ public class DeconstructorTest { @Override public void deconstruct() { destructed = true; } } + private static class SlowDeconstructComponent extends AbstractComponent { + boolean destructed = false; + @Override + public void deconstruct() { + // Add delay to verify that the Deconstructor waits until this is complete before returning. + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + throw new RuntimeException("The delayed deconstruct was interrupted."); + } + destructed = true; + } + } + private static class TestProvider implements Provider<Void> { volatile boolean destructed = false; |