diff options
author | gjoranv <gv@verizonmedia.com> | 2019-11-05 10:53:44 +0100 |
---|---|---|
committer | gjoranv <gv@verizonmedia.com> | 2019-11-05 10:53:44 +0100 |
commit | d75825d503b8566e66d45b0ef726a1d5834970d7 (patch) | |
tree | d6da7f8af45a6bef83823f1ae7068317a5d17e8d /container-di | |
parent | 73cbfef0434123f59584f9ed5f5cceac6715adbd (diff) |
Reapply "Gjoranv/allow duplicate bundles"
This reverts commit 2ef1e922a1d845b3cd79e9fb329925e7e9896919.
Diffstat (limited to 'container-di')
5 files changed, 57 insertions, 22 deletions
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 09f72c9d86d..61497cf71bc 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,6 +1,8 @@ // 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 org.osgi.framework.Bundle; + import java.util.Collection; /** @@ -8,5 +10,7 @@ import java.util.Collection; * @author Tony Vaagenes */ public interface ComponentDeconstructor { - void deconstruct(Collection<Object> components); + + void deconstruct(Collection<Object> components, Collection<Bundle> bundles); + } 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 f82fd22f40d..9a9245f4ba2 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 @@ -18,9 +18,13 @@ import com.yahoo.container.di.componentgraph.core.Node; import com.yahoo.container.di.config.RestApiContext; import com.yahoo.container.di.config.SubscriberFactory; import com.yahoo.vespa.config.ConfigKey; +import org.osgi.framework.Bundle; +import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.IdentityHashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.logging.Level; @@ -65,19 +69,22 @@ public class Container { }); } - private void deconstructObsoleteComponents(ComponentGraph oldGraph, ComponentGraph newGraph) { + private void deconstructObsoleteComponents(ComponentGraph oldGraph, + ComponentGraph newGraph, + Collection<Bundle> obsoleteBundles) { IdentityHashMap<Object, Object> oldComponents = new IdentityHashMap<>(); oldGraph.allConstructedComponentsAndProviders().forEach(c -> oldComponents.put(c, null)); newGraph.allConstructedComponentsAndProviders().forEach(oldComponents::remove); - componentDeconstructor.deconstruct(oldComponents.keySet()); + componentDeconstructor.deconstruct(oldComponents.keySet(), obsoleteBundles); } public ComponentGraph getNewComponentGraph(ComponentGraph oldGraph, Injector fallbackInjector, boolean restartOnRedeploy) { try { - ComponentGraph newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy); + Collection<Bundle> obsoleteBundles = new HashSet<>(); + ComponentGraph newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy, obsoleteBundles); newGraph.reuseNodes(oldGraph); constructComponents(newGraph); - deconstructObsoleteComponents(oldGraph, newGraph); + deconstructObsoleteComponents(oldGraph, newGraph, obsoleteBundles); return newGraph; } catch (Throwable t) { // TODO: Wrap ComponentConstructorException in an Error when generation==0 (+ unit test that Error is thrown) @@ -122,8 +129,13 @@ public class Container { } } - private ComponentGraph getConfigAndCreateGraph(ComponentGraph graph, Injector fallbackInjector, boolean restartOnRedeploy) { + private ComponentGraph getConfigAndCreateGraph(ComponentGraph graph, + Injector fallbackInjector, + boolean restartOnRedeploy, + Collection<Bundle> obsoleteBundles) // NOTE: Return value + { ConfigSnapshot snapshot; + while (true) { snapshot = configurer.getConfigs(graph.configKeys(), leastGeneration, restartOnRedeploy); @@ -131,7 +143,6 @@ public class Container { graph.configKeys(), graph.generation(), snapshot)); if (snapshot instanceof BootstrapConfigs) { - // TODO: remove require when proven unnecessary if (getBootstrapGeneration() <= previousConfigGeneration) { throw new IllegalStateException(String.format( "Got bootstrap configs out of sequence for old config generation %d.\n" + "Previous config generation is %d", @@ -142,8 +153,12 @@ public class Container { "Got new bootstrap generation\n" + "bootstrap generation = %d\n" + "components generation: %d\n" + "previous generation: %d\n", getBootstrapGeneration(), getComponentsGeneration(), previousConfigGeneration)); - installBundles(snapshot.configs()); + + Collection<Bundle> bundlesToRemove = installBundles(snapshot.configs()); + obsoleteBundles.addAll(bundlesToRemove); + graph = createComponentsGraph(snapshot.configs(), getBootstrapGeneration(), fallbackInjector); + // Continues loop } else if (snapshot instanceof ConfigRetriever.ComponentsConfigs) { @@ -184,9 +199,9 @@ public class Container { } } - private void installBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) { + private Set<Bundle> installBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) { BundlesConfig bundlesConfig = getConfig(bundlesConfigKey, configsIncludingBootstrapConfigs); - osgi.useBundles(bundlesConfig.bundle()); + return osgi.useBundles(bundlesConfig.bundle()); } private ComponentGraph createComponentsGraph(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs, @@ -244,7 +259,8 @@ public class Container { } private void deconstructAllComponents(ComponentGraph graph, ComponentDeconstructor deconstructor) { - deconstructor.deconstruct(graph.allConstructedComponentsAndProviders()); + // This is only used for shutdown, so no need to uninstall any bundles. + deconstructor.deconstruct(graph.allConstructedComponentsAndProviders(), Collections.emptyList()); } public static <T extends ConfigInstance> T getConfig(ConfigKey<T> key, diff --git a/container-di/src/main/java/com/yahoo/container/di/Osgi.java b/container-di/src/main/java/com/yahoo/container/di/Osgi.java index 7095180dfc5..ab7da7665b6 100644 --- a/container-di/src/main/java/com/yahoo/container/di/Osgi.java +++ b/container-di/src/main/java/com/yahoo/container/di/Osgi.java @@ -13,6 +13,8 @@ import java.util.Collections; import java.util.Set; import java.util.stream.Collectors; +import static java.util.Collections.emptySet; + /** * @author gjoranv * @author Tony Vaagenes @@ -23,8 +25,13 @@ public interface Osgi { return new BundleClasses(new MockBundle(), Collections.emptySet()); } - default void useBundles(Collection<FileReference> bundles) { + /** + * Returns the set of bundles that is not used by the current application generation, + * and therefore should be scheduled for uninstalling. + */ + default Set<Bundle> useBundles(Collection<FileReference> bundles) { System.out.println("useBundles " + bundles.stream().map(Object::toString).collect(Collectors.joining(", "))); + return emptySet(); } default Class<?> resolveClass(BundleInstantiationSpecification spec) { 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 9c4891c7db2..90bda0ef8a8 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 @@ -14,6 +14,8 @@ import com.yahoo.container.di.componentgraph.core.Node; import com.yahoo.container.di.config.RestApiContext; import org.junit.Ignore; import org.junit.Test; +import org.osgi.framework.Bundle; +import org.osgi.framework.BundleException; import java.util.Collection; import java.util.concurrent.ExecutionException; @@ -283,13 +285,16 @@ public class ContainerTest extends ContainerTestBase { public void providers_are_destructed() { writeBootstrapConfigs("id1", DestructableProvider.class); - ComponentDeconstructor deconstructor = components -> components.forEach(component -> { - if (component instanceof AbstractComponent) { - ((AbstractComponent) component).deconstruct(); - } else if (component instanceof Provider) { - ((Provider<?>) component).deconstruct(); - } - }); + ComponentDeconstructor deconstructor = (components, bundles) -> { + components.forEach(component -> { + if (component instanceof AbstractComponent) { + ((AbstractComponent) component).deconstruct(); + } else if (component instanceof Provider) { + ((Provider<?>) component).deconstruct(); + } + }); + if (! bundles.isEmpty()) throw new IllegalArgumentException("This test should not use bundles"); + }; Container container = newContainer(dirConfigSource, deconstructor); @@ -373,13 +378,14 @@ public class ContainerTest extends ContainerTestBase { public static class TestDeconstructor implements ComponentDeconstructor { @Override - public void deconstruct(Collection<Object> components) { + public void deconstruct(Collection<Object> components, Collection<Bundle> bundles) { components.forEach(component -> { if (component instanceof DestructableComponent) { DestructableComponent vespaComponent = (DestructableComponent) component; vespaComponent.deconstruct(); } }); + if (! bundles.isEmpty()) throw new IllegalArgumentException("This test should not use bundles"); } } diff --git a/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java b/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java index 79cb080dfa4..18236a6bde9 100644 --- a/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java +++ b/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java @@ -5,7 +5,6 @@ import com.google.inject.Guice; import com.yahoo.component.ComponentSpecification; import com.yahoo.config.FileReference; import com.yahoo.container.bundle.BundleInstantiationSpecification; -import com.yahoo.container.di.CloudSubscriberFactory; import com.yahoo.container.di.ContainerTest.ComponentTakingConfig; import com.yahoo.container.di.componentgraph.core.ComponentGraph; import com.yahoo.container.di.osgi.BundleClasses; @@ -16,6 +15,8 @@ import org.osgi.framework.Bundle; import java.util.Collection; import java.util.Set; +import static java.util.Collections.emptySet; + /** * @author Tony Vaagenes * @author gjoranv @@ -60,7 +61,8 @@ public class ContainerTestBase { } @Override - public void useBundles(Collection<FileReference> bundles) { + public Set<Bundle> useBundles(Collection<FileReference> bundles) { + return emptySet(); } @Override |