diff options
author | jonmv <venstad@gmail.com> | 2023-11-30 14:41:03 +0100 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2023-11-30 14:41:03 +0100 |
commit | 946bcd749dc6ab4469aca245508cec8251d6053a (patch) | |
tree | 649f30f01a539dd2f2a3c10fbfc2cf6a72bf9f8b /jdisc_core | |
parent | 3491100f5bee54d0918f3f9ba33d9b0b44026f55 (diff) |
Keep old container alive until servers are switched
Diffstat (limited to 'jdisc_core')
9 files changed, 62 insertions, 43 deletions
diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/DeactivatedContainer.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/DeactivatedContainer.java index 88943617fef..af28d756fe6 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/DeactivatedContainer.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/DeactivatedContainer.java @@ -7,13 +7,14 @@ import com.yahoo.jdisc.Response; import com.yahoo.jdisc.handler.ContentChannel; /** - * <p>This interface represents a {@link Container} which has been deactivated. An instance of this class is returned by - * the {@link ContainerActivator#activateContainer(ContainerBuilder)} method, and is used to schedule a cleanup task - * that is executed once the the deactivated Container has terminated.</p> + * <p>This interface represents a {@link Container} which is in the process of being deactivated. Closing this + * releases the last non-request-response reference to the container, and enables its termination. + * An instance of this class is returned by the {@link ContainerActivator#activateContainer(ContainerBuilder)} method, + * and is used to schedule a cleanup task that is executed once the the deactivated Container has terminated. </p> * * @author Simon Thoresen Hult */ -public interface DeactivatedContainer { +public interface DeactivatedContainer extends AutoCloseable { /** * <p>Returns the context object that was previously attached to the corresponding {@link ContainerBuilder} through @@ -35,4 +36,11 @@ public interface DeactivatedContainer { */ void notifyTermination(Runnable task); + /** + * <p>Close this DeactivatedContainer. This releases the last non-request-response reference to the container, and + * enables its termination.</p> + */ + @Override + void close(); + } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/package-info.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/package-info.java index 007f8242ee4..179dadbb040 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/package-info.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/package-info.java @@ -34,22 +34,23 @@ MyApplication(ContainerActivator activator) { * com.yahoo.jdisc.application.DeactivatedContainer DeactivatedContainer} instance - an API that can be used by the * Application to asynchronously wait for Container termination in order to completely shut down components that are no * longer required. This activation pattern is used both for Application startup, runtime reconfigurations, as well as - * for Application shutdown. It allows all jDISC Application to continously serve Requests during reconfiguration, + * for Application shutdown. It allows all jDISC Application to continuously serve Requests during reconfiguration, * causing no down time other than what the Application itself explicitly enforces.</p> * <pre> void reconfigureApplication() { - (...) - reconfiguredContainerBuilder.handlers().install(myRetainedClients); - reconfiguredContainerBuilder.servers().install(myRetainedServers); - myExpiredServers.close(); - DeactivatedContainer deactivatedContainer = containerActivator.activateContainer(reconfiguredContainerBuilder); - deactivatedContainer.notifyTermination(new Runnable() { - void run() { - myExpiredClients.destroy(); - myExpiredServers.destroy(); - } - }); + (...) + reconfiguredContainerBuilder.handlers().install(myRetainedClients); + reconfiguredContainerBuilder.servers().install(myRetainedServers); + myExpiredServers.close(); + try (DeactivatedContainer deactivatedContainer = containerActivator.activateContainer(reconfiguredContainerBuilder)) { + deactivatedContainer.notifyTermination(new Runnable() { + void run() { + myExpiredClients.destroy(); + myExpiredServers.destroy(); + } + }); + } } </pre> * diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java index e2d2da660c7..d3abfa77fca 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java @@ -55,7 +55,7 @@ public class ActiveContainer extends AbstractResource implements CurrentContaine } }); guiceInjector = builder.guiceModules().activate(); - termination = new ContainerTermination(builder.appContext()); + termination = new ContainerTermination(builder.appContext(), refer()); } @Override diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java index 0ca32197d5e..d6618d50bc3 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java @@ -186,7 +186,7 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C synchronized (appLock) { application = null; } - activateContainer(null); + try (DeactivatedContainer deactivated = activateContainer(null)) { } synchronized (appLock) { this.applicationInUseTracker = null; } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerTermination.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerTermination.java index 4c7c26358d6..7177c337fdd 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerTermination.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerTermination.java @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.core; +import com.yahoo.jdisc.ResourceReference; import com.yahoo.jdisc.application.DeactivatedContainer; /** @@ -10,11 +11,14 @@ public class ContainerTermination implements DeactivatedContainer, Runnable { private final Object lock = new Object(); private final Object appContext; + private final ResourceReference containerReference; private Runnable task; private boolean done; + private boolean closed; - public ContainerTermination(Object appContext) { + public ContainerTermination(Object appContext, ResourceReference containerReference) { this.appContext = appContext; + this.containerReference = containerReference; } @Override @@ -38,6 +42,15 @@ public class ContainerTermination implements DeactivatedContainer, Runnable { } @Override + public void close() { + synchronized (lock) { + if (closed) return; + closed = true; + containerReference.close(); + } + } + + @Override public void run() { Runnable task; synchronized (lock) { diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/test/TestDriver.java b/jdisc_core/src/main/java/com/yahoo/jdisc/test/TestDriver.java index e3bc8aa3b42..d5b6265a1e2 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/test/TestDriver.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/test/TestDriver.java @@ -73,9 +73,10 @@ public class TestDriver implements ContainerActivator, CurrentContainer { return loader.newContainerBuilder(); } + /** Returns the deactivated container, with its container reference already released. */ @Override public DeactivatedContainer activateContainer(ContainerBuilder builder) { - return loader.activateContainer(builder); + try (DeactivatedContainer deactivated = loader.activateContainer(buionf } @Override diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ApplicationLoaderTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ApplicationLoaderTestCase.java index 34acf3d4f93..ca5eefb58fc 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ApplicationLoaderTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ApplicationLoaderTestCase.java @@ -22,6 +22,7 @@ import org.osgi.framework.BundleContext; import java.nio.ByteBuffer; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; @@ -31,6 +32,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -72,23 +74,14 @@ public class ApplicationLoaderTestCase { void requireThatApplicationStartExceptionUnsetsAndDestroysApplication() throws Exception { MyApplication app = MyApplication.newStartException(); ApplicationLoader loader = new ApplicationLoader(new NonWorkingOsgiFramework(), - Arrays.asList(new MyApplicationModule(app))); + List.of(new MyApplicationModule(app))); loader.init(null, false); - try { - loader.start(); - fail(); - } catch (MyException e) { - - } + assertThrows(MyException.class, loader::start); assertNull(loader.application()); assertFalse(app.stop.await(100, TimeUnit.MILLISECONDS)); assertTrue(app.destroy.await(600, TimeUnit.SECONDS)); - try { - loader.activateContainer(loader.newContainerBuilder()); - fail(); - } catch (ApplicationNotReadyException e) { - - } + assertThrows(ApplicationNotReadyException.class, + () -> loader.activateContainer(loader.newContainerBuilder())); loader.stop(); loader.destroy(); } @@ -97,14 +90,10 @@ public class ApplicationLoaderTestCase { void requireThatApplicationStopExceptionDestroysApplication() throws Exception { MyApplication app = MyApplication.newStopException(); ApplicationLoader loader = new ApplicationLoader(new NonWorkingOsgiFramework(), - Arrays.asList(new MyApplicationModule(app))); + List.of(new MyApplicationModule(app))); loader.init(null, false); loader.start(); - try { - loader.stop(); - } catch (MyException e) { - - } + loader.stop(); assertTrue(app.destroy.await(600, TimeUnit.SECONDS)); loader.destroy(); } diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerTerminationTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerTerminationTestCase.java index f91c7ccc3b0..e24c63487d9 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerTerminationTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerTerminationTestCase.java @@ -18,8 +18,12 @@ public class ContainerTerminationTestCase { @Test void requireThatAccessorsWork() { Object obj = new Object(); - ContainerTermination termination = new ContainerTermination(obj); + MyTask task = new MyTask(); + ContainerTermination termination = new ContainerTermination(obj, task::run); assertSame(obj, termination.appContext()); + assertFalse(task.done); + termination.close(); + assertTrue(task.done); } @Test @@ -36,7 +40,7 @@ public class ContainerTerminationTestCase { @Test void requireThatEarlyTerminationIsNotified() { - ContainerTermination termination = new ContainerTermination(null); + ContainerTermination termination = new ContainerTermination(null, null); termination.run(); MyTask task = new MyTask(); termination.notifyTermination(task); @@ -45,7 +49,7 @@ public class ContainerTerminationTestCase { @Test void requireThatLaterTerminationIsNotified() { - ContainerTermination termination = new ContainerTermination(null); + ContainerTermination termination = new ContainerTermination(null, null); MyTask task = new MyTask(); termination.notifyTermination(task); assertFalse(task.done); @@ -55,7 +59,7 @@ public class ContainerTerminationTestCase { @Test void requireThatNotifyCanOnlyBeCalledOnce() { - ContainerTermination termination = new ContainerTermination(null); + ContainerTermination termination = new ContainerTermination(null, null); termination.notifyTermination(new MyTask()); try { termination.notifyTermination(new MyTask()); diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerWatchdogTest.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerWatchdogTest.java index 2909c11a6c6..6d178567b75 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerWatchdogTest.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerWatchdogTest.java @@ -42,6 +42,9 @@ public class ContainerWatchdogTest { assertEquals(1, runMonitorStepAndGetStaleContainerCount(watchdog, metric)); containerWithoutRetainedResources.release(); + assertEquals(1, runMonitorStepAndGetStaleContainerCount(watchdog, metric)); + + containerWithoutRetainedResources.shutdown().close(); assertEquals(0, runMonitorStepAndGetStaleContainerCount(watchdog, metric)); } |