diff options
7 files changed, 32 insertions, 51 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 716f7b496b1..bcd3665a3d6 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 @@ -143,9 +143,7 @@ public class HandlersConfigurerDi { return currentGraph.getInstance(componentClass); } - public void shutdown(ComponentDeconstructor deconstructor) { - container.shutdown(currentGraph, deconstructor); - } + public void shutdown() { container.shutdown(currentGraph); } /** Returns the currently active application configuration generation */ public long generation() { return currentGraph.generation(); } diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java index b9ff5a08785..eadb6b52294 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java @@ -125,7 +125,7 @@ public class HandlersConfigurerTestWrapper { } public void shutdown() { - configurer.shutdown(getTestDeconstructor()); + configurer.shutdown(); // TODO: Remove once tests use ConfigSet rather than dir: for (File f : createdFiles) { f.delete(); diff --git a/container-core/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java b/container-core/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java index bc5ddb3fa7a..95a15e12735 100644 --- a/container-core/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java +++ b/container-core/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java @@ -15,4 +15,7 @@ public interface ComponentDeconstructor { /** Deconstructs the given components in order, then the given bundles. */ void deconstruct(List<Object> components, Collection<Bundle> bundles); + /** Wait for all previous destruction tasks to complete */ + default void shutdown() {} + } 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 8fc72d8cddb..8d8a05408a9 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 @@ -245,10 +245,11 @@ public class Container { } } - public void shutdown(ComponentGraph graph, ComponentDeconstructor deconstructor) { + public void shutdown(ComponentGraph graph) { shutdownConfigurer(); if (graph != null) { - deconstructAllComponents(graph, deconstructor); + deconstructAllComponents(graph, componentDeconstructor); + componentDeconstructor.shutdown(); } } 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 cf81b39db06..d686bb9a3dd 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 @@ -351,7 +351,7 @@ public final class ConfiguredApplication implements Application { return new HandlersConfigurerDi(subscriberFactory, Container.get(), configId, - new Deconstructor(Deconstructor.Mode.RECONFIG), + new Deconstructor(), discInjector, osgiFramework); } @@ -385,7 +385,7 @@ public final class ConfiguredApplication implements Application { CountDownLatch latch = new CountDownLatch(1); activator.activateContainer(null) .notifyTermination(() -> { - configurer.shutdown(new Deconstructor(Deconstructor.Mode.SHUTDOWN)); + configurer.shutdown(); slobrokConfigSubscriber.ifPresent(SlobrokConfigSubscriber::shutdown); Container.get().shutdown(); unregisterInSlobrok(); 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 e224601498b..ee592c2e8d1 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 @@ -7,6 +7,7 @@ import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.container.di.ComponentDeconstructor; import com.yahoo.container.di.componentgraph.Provider; import com.yahoo.jdisc.SharedResource; +import com.yahoo.yolean.UncheckedInterruptedException; import org.osgi.framework.Bundle; import org.osgi.framework.BundleException; @@ -15,12 +16,9 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Random; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.logging.Level; import java.util.logging.Logger; @@ -37,32 +35,17 @@ public class Deconstructor implements ComponentDeconstructor { private static final Logger log = Logger.getLogger(Deconstructor.class.getName()); - private static final Duration RECONFIG_DECONSTRUCT_DELAY = Duration.ofSeconds(60); + private final ExecutorService executor = + Executors.newFixedThreadPool(1, ThreadFactoryFactory.getThreadFactory("component-deconstructor")); // This must be smaller than the shutdownDeadlineExecutor delay in ConfiguredApplication - private static final Duration SHUTDOWN_DECONSTRUCT_TIMEOUT = Duration.ofSeconds(45); + private final Duration shutdownTimeout; - 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. + public Deconstructor(Duration shutdownTimeout) { + this.shutdownTimeout = shutdownTimeout; } - private final ScheduledExecutorService executor = - new ScheduledThreadPoolExecutor(1, ThreadFactoryFactory.getThreadFactory("component-deconstructor")); - - private final Mode mode; - private final Duration delay; - - public Deconstructor(Mode mode) { - this(mode, (mode == Mode.RECONFIG) ? RECONFIG_DECONSTRUCT_DELAY : Duration.ZERO); - } - - // For testing only - Deconstructor(Mode mode, Duration reconfigDeconstructDelay) { - this.mode = mode; - this.delay = reconfigDeconstructDelay; - } + public Deconstructor() { this(Duration.ofSeconds(45)); } @Override public void deconstruct(List<Object> components, Collection<Bundle> bundles) { @@ -81,25 +64,21 @@ public class Deconstructor implements ComponentDeconstructor { } } if (!destructibleComponents.isEmpty() || !bundles.isEmpty()) { - var task = executor.schedule(new DestructComponentTask(destructibleComponents, bundles), - delay.getSeconds(), TimeUnit.SECONDS); - if (mode.equals(Mode.SHUTDOWN)) { - waitFor(task, SHUTDOWN_DECONSTRUCT_TIMEOUT); - } + executor.execute(new DestructComponentTask(destructibleComponents, bundles)); } } - private void waitFor(ScheduledFuture<?> task, Duration timeout) { + @Override + public void shutdown() { + executor.shutdown(); try { - log.info("Waiting up to " + timeout.toSeconds() + " seconds for all components to deconstruct."); - task.get(timeout.toMillis(), TimeUnit.MILLISECONDS); + log.info("Waiting up to " + shutdownTimeout.toSeconds() + " seconds for all previous components graphs to deconstruct."); + if (!executor.awaitTermination(shutdownTimeout.getSeconds(), TimeUnit.SECONDS)) { + log.warning("Waiting for deconstruction timed out."); + } } 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."); + throw new UncheckedInterruptedException(e, true); } } 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 0a7d3f45e82..579275deb0f 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 @@ -9,7 +9,6 @@ import com.yahoo.jdisc.SharedResource; import org.junit.Before; import org.junit.Test; -import java.time.Duration; import java.time.Instant; import java.util.List; import java.util.function.Supplier; @@ -26,15 +25,16 @@ public class DeconstructorTest { @Before public void init() { - deconstructor = new Deconstructor(Deconstructor.Mode.RECONFIG, Duration.ZERO); + deconstructor = new Deconstructor(); } @Test - public void deconstruct_is_synchronous_in_shutdown_mode() { - deconstructor = new Deconstructor(Deconstructor.Mode.SHUTDOWN); + public void deconstructor_waits_for_completion_on_shutdown() { + deconstructor = new Deconstructor(); var slowDeconstructComponent = new SlowDeconstructComponent(); deconstructor.deconstruct(List.of(slowDeconstructComponent), emptyList()); + deconstructor.shutdown(); assertTrue(slowDeconstructComponent.destructed); } |