From 40395ff12dd52300db74ccf6503156eee45a262a Mon Sep 17 00:00:00 2001 From: gjoranv Date: Sat, 26 Oct 2019 21:13:52 +0200 Subject: Schedule deconstruct of all components with one call. --- .../testutil/HandlersConfigurerTestWrapper.java | 14 +++++----- .../yahoo/container/di/ComponentDeconstructor.java | 4 ++- .../java/com/yahoo/container/di/Container.java | 4 +-- .../di/componentgraph/core/ComponentGraph.java | 2 +- .../java/com/yahoo/container/di/ContainerTest.java | 29 ++++++++++---------- .../container/jdisc/component/Deconstructor.java | 31 ++++++++++++---------- .../jdisc/component/DeconstructorTest.java | 9 ++++--- 7 files changed, 49 insertions(+), 44 deletions(-) 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 1fd90125cde..684a45aeac1 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 @@ -19,6 +19,7 @@ import com.yahoo.osgi.MockOsgi; import java.io.File; import java.io.IOException; +import java.util.Collection; import java.util.LinkedHashSet; import java.util.Random; import java.util.Set; @@ -102,15 +103,12 @@ public class HandlersConfigurerTestWrapper { } private ComponentDeconstructor getTestDeconstructor() { - return new ComponentDeconstructor() { - @Override - public void deconstruct(Object component) { - if (component instanceof AbstractComponent) { - AbstractComponent abstractComponent = (AbstractComponent) component; - if (abstractComponent.isDeconstructable()) - ((AbstractComponent) component).deconstruct(); + return components -> components.forEach(component -> { + if (component instanceof AbstractComponent) { + AbstractComponent abstractComponent = (AbstractComponent) component; + if (abstractComponent.isDeconstructable()) abstractComponent.deconstruct(); } - }}; + }); } public void reloadConfig() { diff --git a/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java b/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java index dbfb842204d..09f72c9d86d 100644 --- a/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java +++ b/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java @@ -1,10 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.di; +import java.util.Collection; + /** * @author gjoranv * @author Tony Vaagenes */ public interface ComponentDeconstructor { - void deconstruct(Object component); + void deconstruct(Collection components); } diff --git a/container-di/src/main/java/com/yahoo/container/di/Container.java b/container-di/src/main/java/com/yahoo/container/di/Container.java index 0c534b0673c..67ba25d5e49 100644 --- a/container-di/src/main/java/com/yahoo/container/di/Container.java +++ b/container-di/src/main/java/com/yahoo/container/di/Container.java @@ -69,7 +69,7 @@ public class Container { IdentityHashMap oldComponents = new IdentityHashMap<>(); oldGraph.allConstructedComponentsAndProviders().forEach(c -> oldComponents.put(c, null)); newGraph.allConstructedComponentsAndProviders().forEach(oldComponents::remove); - oldComponents.keySet().forEach(componentDeconstructor::deconstruct); + componentDeconstructor.deconstruct(oldComponents.keySet()); } public ComponentGraph getNewComponentGraph(ComponentGraph oldGraph, Injector fallbackInjector, boolean restartOnRedeploy) { @@ -244,7 +244,7 @@ public class Container { } private void deconstructAllComponents(ComponentGraph graph, ComponentDeconstructor deconstructor) { - graph.allConstructedComponentsAndProviders().forEach(deconstructor::deconstruct); + deconstructor.deconstruct(graph.allConstructedComponentsAndProviders()); } public static T getConfig(ConfigKey key, diff --git a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentGraph.java b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentGraph.java index f55d68ad708..67143c60f62 100644 --- a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentGraph.java +++ b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentGraph.java @@ -165,7 +165,7 @@ public class ComponentGraph { } } - public Collection allConstructedComponentsAndProviders() { + public Collection allConstructedComponentsAndProviders() { return nodes().stream().map(node -> node.constructedInstance().get()).collect(Collectors.toList()); } diff --git a/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java b/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java index 3996dff2811..9c4891c7db2 100644 --- a/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java +++ b/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java @@ -15,6 +15,7 @@ import com.yahoo.container.di.config.RestApiContext; import org.junit.Ignore; import org.junit.Test; +import java.util.Collection; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -282,17 +283,13 @@ public class ContainerTest extends ContainerTestBase { public void providers_are_destructed() { writeBootstrapConfigs("id1", DestructableProvider.class); - ComponentDeconstructor deconstructor = new ComponentDeconstructor() { - @Override - public void deconstruct(Object component) { - if (component instanceof AbstractComponent) { - ((AbstractComponent) component).deconstruct(); - ; - } else if (component instanceof Provider) { - ((Provider) component).deconstruct(); - } + ComponentDeconstructor deconstructor = components -> components.forEach(component -> { + if (component instanceof AbstractComponent) { + ((AbstractComponent) component).deconstruct(); + } else if (component instanceof Provider) { + ((Provider) component).deconstruct(); } - }; + }); Container container = newContainer(dirConfigSource, deconstructor); @@ -376,11 +373,13 @@ public class ContainerTest extends ContainerTestBase { public static class TestDeconstructor implements ComponentDeconstructor { @Override - public void deconstruct(Object component) { - if (component instanceof DestructableComponent) { - DestructableComponent vespaComponent = (DestructableComponent) component; - vespaComponent.deconstruct(); - } + public void deconstruct(Collection components) { + components.forEach(component -> { + if (component instanceof DestructableComponent) { + DestructableComponent vespaComponent = (DestructableComponent) component; + vespaComponent.deconstruct(); + } + }); } } 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 4fe7a07e281..4920051cbee 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,7 @@ import com.yahoo.jdisc.SharedResource; import com.yahoo.log.LogLevel; import java.time.Duration; +import java.util.Collection; import java.util.Random; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -36,21 +37,23 @@ public class Deconstructor implements ComponentDeconstructor { @Override - public void deconstruct(Object component) { - if (component instanceof AbstractComponent) { - AbstractComponent abstractComponent = (AbstractComponent) component; - if (abstractComponent.isDeconstructable()) { - executor.schedule(new DestructComponentTask(abstractComponent), delay.getSeconds(), TimeUnit.SECONDS); + public void deconstruct(Collection components) { + for (var component : components) { + if (component instanceof AbstractComponent) { + AbstractComponent abstractComponent = (AbstractComponent) component; + if (abstractComponent.isDeconstructable()) { + executor.schedule(new DestructComponentTask(abstractComponent), delay.getSeconds(), TimeUnit.SECONDS); + } + } else if (component instanceof Provider) { + // TODO Providers should most likely be deconstructed similarily to AbstractComponent + log.info("Starting deconstruction of provider " + component); + ((Provider) component).deconstruct(); + log.info("Finished deconstruction of provider " + component); + } else if (component instanceof SharedResource) { + log.info("Releasing container reference to resource " + component); + // No need to delay release, as jdisc does ref-counting + ((SharedResource) component).release(); } - } else if (component instanceof Provider) { - // TODO Providers should most likely be deconstructed similarily to AbstractComponent - log.info("Starting deconstruction of provider " + component); - ((Provider)component).deconstruct(); - log.info("Finished deconstruction of provider " + component); - } else if (component instanceof SharedResource) { - log.info("Releasing container reference to resource " + component); - // No need to delay release, as jdisc does ref-counting - ((SharedResource)component).release(); } } 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 ff61127f617..395b1aa7c44 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 @@ -8,6 +8,9 @@ import com.yahoo.jdisc.SharedResource; import org.junit.Before; import org.junit.Test; +import java.util.Collections; + +import static java.util.Collections.singleton; import static org.junit.Assert.assertTrue; /** @@ -25,7 +28,7 @@ public class DeconstructorTest { 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(abstractComponent); + deconstructor.deconstruct(singleton(abstractComponent)); int cnt = 0; while (! abstractComponent.destructed && (cnt++ < 12000)) { Thread.sleep(10); @@ -36,14 +39,14 @@ public class DeconstructorTest { @Test public void require_provider_destructed() { TestProvider provider = new TestProvider(); - deconstructor.deconstruct(provider); + deconstructor.deconstruct(singleton(provider)); assertTrue(provider.destructed); } @Test public void require_shared_resource_released() { TestSharedResource sharedResource = new TestSharedResource(); - deconstructor.deconstruct(sharedResource); + deconstructor.deconstruct(singleton(sharedResource)); assertTrue(sharedResource.released); } -- cgit v1.2.3