diff options
author | gjoranv <gjoranv@gmail.com> | 2019-11-04 15:55:23 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-11-04 15:55:23 +0100 |
commit | 2ef1e922a1d845b3cd79e9fb329925e7e9896919 (patch) | |
tree | 429207fa364a2f6ecbc523b78c3bc4d7f967cf39 | |
parent | 8b0f9567b6f4baed6565174b68a356b4b8bdcd51 (diff) |
Revert "Gjoranv/allow duplicate bundles"
22 files changed, 97 insertions, 334 deletions
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java index 4b8f21469d1..2b3a272fadc 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java @@ -10,6 +10,7 @@ import org.osgi.framework.Bundle; import org.osgi.framework.wiring.BundleRevision; import java.io.File; +import java.util.Arrays; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; @@ -25,18 +26,11 @@ import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX * Manages the set of installed 3rd-party component bundles. * * @author Tony Vaagenes - * @author gjoranv */ public class BundleLoader { - /* Map of file refs of active bundles (not scheduled for uninstall) to a list of all bundles that were installed - * (pre-install directive) by the bundle pointed to by the file ref (including itself). - * - * Used to: - * 1. Avoid installing already installed bundles. Just an optimization, installing the same bundle location is a NOP - * 2. Start bundles (all are started every time) - * 3. Calculate the set of bundles to uninstall - */ + private final List<Bundle> initialBundles; + private final Map<FileReference, List<Bundle>> reference2Bundles = new LinkedHashMap<>(); private final Logger log = Logger.getLogger(BundleLoader.class.getName()); @@ -44,6 +38,7 @@ public class BundleLoader { public BundleLoader(Osgi osgi) { this.osgi = osgi; + initialBundles = Arrays.asList(osgi.getBundles()); } private List<Bundle> obtainBundles(FileReference reference, FileAcquirer fileAcquirer) throws InterruptedException { @@ -51,19 +46,17 @@ public class BundleLoader { return osgi.install(file.getAbsolutePath()); } - private void install(List<FileReference> references) { + /** Returns the number of bundles installed by this call. */ + private int install(List<FileReference> references) { Set<FileReference> bundlesToInstall = new HashSet<>(references); - - // This is just an optimization, as installing a bundle with the same location id returns the already installed bundle. - // It's ok that fileRefs pending uninstall are removed from the map, because they are never in the new set of bundles.. bundlesToInstall.removeAll(reference2Bundles.keySet()); PredicateSplit<FileReference> bundlesToInstall_isDisk = partition(bundlesToInstall, BundleLoader::isDiskBundle); installBundlesFromDisk(bundlesToInstall_isDisk.trueValues); installBundlesFromFileDistribution(bundlesToInstall_isDisk.falseValues); - // TODO: Remove. Bundles are also started in use() startBundles(); + return bundlesToInstall.size(); } private static boolean isDiskBundle(FileReference fileReference) { @@ -104,7 +97,6 @@ public class BundleLoader { } List<Bundle> bundles = osgi.install(file.getAbsolutePath()); - reference2Bundles.put(reference, bundles); } @@ -121,16 +113,13 @@ public class BundleLoader { } } - /** - * Resolves and starts (calls the Bundles BundleActivator) all bundles. Bundle resolution must take place - * after all bundles are installed to ensure that the framework can resolve dependencies between bundles. - */ + // All bundles must have been started first to ensure correct package resolution. private void startBundles() { for (List<Bundle> bundles : reference2Bundles.values()) { for (Bundle bundle : bundles) { try { if ( ! isFragment(bundle)) - bundle.start(); // NOP for already ACTIVE bundles + bundle.start(); } catch(Exception e) { throw new RuntimeException("Could not start bundle '" + bundle.getSymbolicName() + "'", e); } @@ -146,44 +135,38 @@ public class BundleLoader { return (bundleRevision.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0; } - /** - * Returns the bundles to schedule for uninstall after their components have been deconstructed - * and removes the same bundles from the map of active bundles. - */ - private Set<Bundle> getBundlesToUninstall(List<FileReference> newReferences) { - Set<Bundle> bundlesToRemove = new HashSet<>(osgi.getCurrentBundles()); + /** Returns the number of uninstalled bundles */ + private int retainOnly(List<FileReference> newReferences) { + Set<Bundle> bundlesToRemove = new HashSet<>(Arrays.asList(osgi.getBundles())); for (FileReference fileReferenceToKeep: newReferences) { if (reference2Bundles.containsKey(fileReferenceToKeep)) bundlesToRemove.removeAll(reference2Bundles.get(fileReferenceToKeep)); } - bundlesToRemove.removeAll(osgi.getInitialBundles()); - removeInactiveFileReferences(newReferences); - - return bundlesToRemove; - } + bundlesToRemove.removeAll(initialBundles); + for (Bundle bundle : bundlesToRemove) { + log.info("Removing bundle '" + bundle.toString() + "'"); + osgi.uninstall(bundle); + } - private void removeInactiveFileReferences(List<FileReference> newReferences) { - // Clean up the map of active bundles Set<FileReference> fileReferencesToRemove = new HashSet<>(reference2Bundles.keySet()); fileReferencesToRemove.removeAll(newReferences); - fileReferencesToRemove.forEach(reference2Bundles::remove); + + for (FileReference fileReferenceToRemove : fileReferencesToRemove) { + reference2Bundles.remove(fileReferenceToRemove); + } + return bundlesToRemove.size(); } - /** - * Installs the given set of bundles and returns the set of bundles that is no longer used - * by the application, and should therefore be scheduled for uninstall. - */ - public synchronized Set<Bundle> use(List<FileReference> newBundles) { - Set<Bundle> bundlesToUninstall = getBundlesToUninstall(newBundles); - osgi.allowDuplicateBundles(bundlesToUninstall); - log.info(() -> bundlesToUninstall.isEmpty() ? "Adding bundles to allowed duplicates: " + bundlesToUninstall : ""); - install(newBundles); + public synchronized int use(List<FileReference> bundles) { + int removedBundles = retainOnly(bundles); + int installedBundles = install(bundles); startBundles(); + log.info(removedBundles + " bundles were removed, and " + installedBundles + " bundles were installed."); log.info(installedBundlesMessage()); - return bundlesToUninstall; + return removedBundles + installedBundles; } private String installedBundlesMessage() { 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 ef132694e10..dd8b2605820 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 @@ -9,7 +9,6 @@ import com.yahoo.component.ComponentSpecification; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.config.FileReference; -import com.yahoo.container.core.config.testutil.MockOsgiWrapper; import com.yahoo.container.di.ComponentDeconstructor; import com.yahoo.container.di.Container; import com.yahoo.container.di.componentgraph.core.ComponentGraph; @@ -21,9 +20,10 @@ import com.yahoo.jdisc.application.OsgiFramework; import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.service.ClientProvider; import com.yahoo.jdisc.service.ServerProvider; +import com.yahoo.language.Linguistics; +import com.yahoo.language.simple.SimpleLinguistics; import com.yahoo.log.LogLevel; import com.yahoo.osgi.OsgiImpl; -import com.yahoo.osgi.OsgiWrapper; import com.yahoo.statistics.Statistics; import org.osgi.framework.Bundle; import org.osgi.framework.wiring.BundleWiring; @@ -81,30 +81,19 @@ public class HandlersConfigurerDi { Injector discInjector, OsgiFramework osgiFramework) { - this(subscriberFactory, vespaContainer, configId, deconstructor, discInjector, - new ContainerAndDiOsgi(osgiFramework, new BundleLoader(new OsgiImpl(osgiFramework)))); - } - - // Only public for testing - public HandlersConfigurerDi(SubscriberFactory subscriberFactory, - com.yahoo.container.Container vespaContainer, - String configId, - ComponentDeconstructor deconstructor, - Injector discInjector, - OsgiWrapper osgiWrapper) { - this.vespaContainer = vespaContainer; - this.osgiWrapper = osgiWrapper; + osgiWrapper = new OsgiWrapper(osgiFramework, new BundleLoader(new OsgiImpl(osgiFramework))); + container = new Container(subscriberFactory, configId, deconstructor, osgiWrapper); getNewComponentGraph(discInjector, false); } - private static class ContainerAndDiOsgi extends OsgiImpl implements OsgiWrapper { + private static class OsgiWrapper extends OsgiImpl implements com.yahoo.container.di.Osgi { private final OsgiFramework osgiFramework; private final BundleLoader bundleLoader; - public ContainerAndDiOsgi(OsgiFramework osgiFramework, BundleLoader bundleLoader) { + public OsgiWrapper(OsgiFramework osgiFramework, BundleLoader bundleLoader) { super(osgiFramework); this.osgiFramework = osgiFramework; this.bundleLoader = bundleLoader; @@ -132,9 +121,14 @@ public class HandlersConfigurerDi { } @Override - public Set<Bundle> useBundles(Collection<FileReference> bundles) { + public void useBundles(Collection<FileReference> bundles) { log.info("Installing bundles from the latest application"); - return bundleLoader.use(new ArrayList<>(bundles)); + + int bundlesRemovedOrInstalled = bundleLoader.use(new ArrayList<>(bundles)); + + if (bundlesRemovedOrInstalled > 0) { + refreshPackages(); + } } } 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 9f49b016b68..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; @@ -89,7 +90,7 @@ public class HandlersConfigurerTestWrapper { public HandlersConfigurerTestWrapper(Container container, String configId) { createFiles(configId); - MockOsgiWrapper mockOsgiWrapper = new MockOsgiWrapper(); + MockOsgi mockOsgi = new MockOsgi(); ComponentDeconstructor testDeconstructor = getTestDeconstructor(); configurer = new HandlersConfigurerDi( new CloudSubscriberFactory(configSources), @@ -97,17 +98,16 @@ public class HandlersConfigurerTestWrapper { configId, testDeconstructor, guiceInjector(), - mockOsgiWrapper); + mockOsgi); this.container = container; } private ComponentDeconstructor getTestDeconstructor() { - return (components, bundles) -> components.forEach(component -> { + return components -> components.forEach(component -> { if (component instanceof AbstractComponent) { AbstractComponent abstractComponent = (AbstractComponent) component; if (abstractComponent.isDeconstructable()) abstractComponent.deconstruct(); } - if (! bundles.isEmpty()) throw new IllegalArgumentException("This test should not use bundles"); }); } diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java deleted file mode 100644 index a1f564f5682..00000000000 --- a/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java +++ /dev/null @@ -1,44 +0,0 @@ -package com.yahoo.container.core.config.testutil; - -import com.yahoo.component.ComponentSpecification; -import com.yahoo.osgi.OsgiWrapper; -import org.osgi.framework.Bundle; - -import java.util.Collection; -import java.util.List; - -import static java.util.Collections.emptyList; - -/** - * @author gjoranv - */ -public class MockOsgiWrapper implements OsgiWrapper { - - @Override - public List<Bundle> getInitialBundles() { - return emptyList(); - } - - @Override - public Bundle[] getBundles() { - return new Bundle[0]; - } - - @Override - public List<Bundle> getCurrentBundles() { - return emptyList(); - } - - @Override - public Bundle getBundle(ComponentSpecification bundleId) { - return null; - } - - @Override - public List<Bundle> install(String absolutePath) { - return emptyList(); - } - - @Override - public void allowDuplicateBundles(Collection<Bundle> bundles) { } -} diff --git a/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java index d809c493565..45ad02d2cef 100644 --- a/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java +++ b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java @@ -16,28 +16,26 @@ import java.util.List; public class MockOsgi extends NonWorkingOsgiFramework implements Osgi { @Override - public List<Bundle> getInitialBundles() { - return Collections.emptyList(); + public Bundle[] getBundles() { + return new Bundle[0]; } @Override - public Bundle[] getBundles() { - return new Bundle[0]; + public Bundle getBundle(ComponentSpecification bundleId) { + return null; } @Override - public List<Bundle> getCurrentBundles() { + public List<Bundle> install(String absolutePath) { return Collections.emptyList(); } @Override - public Bundle getBundle(ComponentSpecification bundleId) { - return null; + public void uninstall(Bundle bundle) { } @Override - public List<Bundle> install(String absolutePath) { - return Collections.emptyList(); + public void refreshPackages() { } } diff --git a/container-core/src/main/java/com/yahoo/osgi/Osgi.java b/container-core/src/main/java/com/yahoo/osgi/Osgi.java index 8f0acf41f30..c94eaf43deb 100644 --- a/container-core/src/main/java/com/yahoo/osgi/Osgi.java +++ b/container-core/src/main/java/com/yahoo/osgi/Osgi.java @@ -4,7 +4,6 @@ package com.yahoo.osgi; import com.yahoo.component.ComponentSpecification; import org.osgi.framework.Bundle; -import java.util.Collection; import java.util.List; /** @@ -12,17 +11,14 @@ import java.util.List; */ public interface Osgi { - List<Bundle> getInitialBundles(); - Bundle[] getBundles(); - /** Returns all bundles that have not been scheduled for uninstall. */ - List<Bundle> getCurrentBundles(); - Bundle getBundle(ComponentSpecification bundleId); List<Bundle> install(String absolutePath); - void allowDuplicateBundles(Collection<Bundle> bundles); + void uninstall(Bundle bundle); + + void refreshPackages(); } diff --git a/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java b/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java index ed93d15c975..8b2f20a1c13 100644 --- a/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java +++ b/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java @@ -7,43 +7,19 @@ import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.jdisc.application.OsgiFramework; import org.osgi.framework.Bundle; import org.osgi.framework.BundleException; -import org.osgi.framework.launch.Framework; -import java.util.Collection; import java.util.List; -import java.util.logging.Logger; /** * @author Tony Vaagenes * @author bratseth */ public class OsgiImpl implements Osgi { - private static final Logger log = Logger.getLogger(OsgiImpl.class.getName()); private final OsgiFramework jdiscOsgi; - // The initial bundles are never scheduled for uninstall - private final List<Bundle> initialBundles; - - // An initial bundle that is not the framework, and can hence be used to look up current bundles - private final Bundle alwaysCurrentBundle; - public OsgiImpl(OsgiFramework jdiscOsgi) { this.jdiscOsgi = jdiscOsgi; - - this.initialBundles = jdiscOsgi.bundles(); - if (initialBundles.isEmpty()) - throw new IllegalStateException("No initial bundles!"); - - alwaysCurrentBundle = firstNonFrameworkBundle(initialBundles); - if (alwaysCurrentBundle == null) - throw new IllegalStateException("The initial bundles only contained the framework bundle!"); - log.info("Using " + alwaysCurrentBundle + " to lookup current bundles."); - } - - @Override - public List<Bundle> getInitialBundles() { - return initialBundles; } @Override @@ -52,10 +28,6 @@ public class OsgiImpl implements Osgi { return bundles.toArray(new Bundle[bundles.size()]); } - @Override - public List<Bundle> getCurrentBundles() { - return jdiscOsgi.getBundles(alwaysCurrentBundle); - } public Class<Object> resolveClass(BundleInstantiationSpecification spec) { Bundle bundle = getBundle(spec.bundle); @@ -114,9 +86,8 @@ public class OsgiImpl implements Osgi { * @return the bundle match having the highest version, or null if there was no matches */ public Bundle getBundle(ComponentSpecification id) { - log.fine(() -> "Getting bundle for component " + id + ". Set of current bundles: " + getCurrentBundles()); Bundle highestMatch = null; - for (Bundle bundle : getCurrentBundles()) { + for (Bundle bundle : getBundles()) { assert bundle.getSymbolicName() != null : "ensureHasBundleSymbolicName not called during installation"; if ( ! bundle.getSymbolicName().equals(id.getName())) continue; @@ -151,16 +122,17 @@ public class OsgiImpl implements Osgi { } @Override - public void allowDuplicateBundles(Collection<Bundle> bundles) { - jdiscOsgi.allowDuplicateBundles(bundles); + public void uninstall(Bundle bundle) { + try { + bundle.uninstall(); + } catch (BundleException e) { + throw new RuntimeException(e); + } } - private static Bundle firstNonFrameworkBundle(List<Bundle> bundles) { - for (Bundle b : bundles) { - if (! (b instanceof Framework)) - return b; - } - return null; + @Override + public void refreshPackages() { + jdiscOsgi.refreshPackages(); } } diff --git a/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java b/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java deleted file mode 100644 index 58e19e52ee3..00000000000 --- a/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java +++ /dev/null @@ -1,16 +0,0 @@ -package com.yahoo.osgi; - -import com.yahoo.component.ComponentSpecification; -import org.osgi.framework.Bundle; - -/** - * @author gjoranv - */ -public interface OsgiWrapper extends Osgi, com.yahoo.container.di.Osgi { - - @Override - default Bundle getBundle(ComponentSpecification bundleId) { - return null; - } - -} 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 61497cf71bc..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,8 +1,6 @@ // 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; /** @@ -10,7 +8,5 @@ import java.util.Collection; * @author Tony Vaagenes */ public interface ComponentDeconstructor { - - void deconstruct(Collection<Object> components, Collection<Bundle> bundles); - + void deconstruct(Collection<Object> 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 9a9245f4ba2..f82fd22f40d 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,13 +18,9 @@ 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; @@ -69,22 +65,19 @@ public class Container { }); } - private void deconstructObsoleteComponents(ComponentGraph oldGraph, - ComponentGraph newGraph, - Collection<Bundle> obsoleteBundles) { + private void deconstructObsoleteComponents(ComponentGraph oldGraph, ComponentGraph newGraph) { IdentityHashMap<Object, Object> oldComponents = new IdentityHashMap<>(); oldGraph.allConstructedComponentsAndProviders().forEach(c -> oldComponents.put(c, null)); newGraph.allConstructedComponentsAndProviders().forEach(oldComponents::remove); - componentDeconstructor.deconstruct(oldComponents.keySet(), obsoleteBundles); + componentDeconstructor.deconstruct(oldComponents.keySet()); } public ComponentGraph getNewComponentGraph(ComponentGraph oldGraph, Injector fallbackInjector, boolean restartOnRedeploy) { try { - Collection<Bundle> obsoleteBundles = new HashSet<>(); - ComponentGraph newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy, obsoleteBundles); + ComponentGraph newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy); newGraph.reuseNodes(oldGraph); constructComponents(newGraph); - deconstructObsoleteComponents(oldGraph, newGraph, obsoleteBundles); + deconstructObsoleteComponents(oldGraph, newGraph); return newGraph; } catch (Throwable t) { // TODO: Wrap ComponentConstructorException in an Error when generation==0 (+ unit test that Error is thrown) @@ -129,13 +122,8 @@ public class Container { } } - private ComponentGraph getConfigAndCreateGraph(ComponentGraph graph, - Injector fallbackInjector, - boolean restartOnRedeploy, - Collection<Bundle> obsoleteBundles) // NOTE: Return value - { + private ComponentGraph getConfigAndCreateGraph(ComponentGraph graph, Injector fallbackInjector, boolean restartOnRedeploy) { ConfigSnapshot snapshot; - while (true) { snapshot = configurer.getConfigs(graph.configKeys(), leastGeneration, restartOnRedeploy); @@ -143,6 +131,7 @@ 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", @@ -153,12 +142,8 @@ public class Container { "Got new bootstrap generation\n" + "bootstrap generation = %d\n" + "components generation: %d\n" + "previous generation: %d\n", getBootstrapGeneration(), getComponentsGeneration(), previousConfigGeneration)); - - Collection<Bundle> bundlesToRemove = installBundles(snapshot.configs()); - obsoleteBundles.addAll(bundlesToRemove); - + installBundles(snapshot.configs()); graph = createComponentsGraph(snapshot.configs(), getBootstrapGeneration(), fallbackInjector); - // Continues loop } else if (snapshot instanceof ConfigRetriever.ComponentsConfigs) { @@ -199,9 +184,9 @@ public class Container { } } - private Set<Bundle> installBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) { + private void installBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) { BundlesConfig bundlesConfig = getConfig(bundlesConfigKey, configsIncludingBootstrapConfigs); - return osgi.useBundles(bundlesConfig.bundle()); + osgi.useBundles(bundlesConfig.bundle()); } private ComponentGraph createComponentsGraph(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs, @@ -259,8 +244,7 @@ public class Container { } private void deconstructAllComponents(ComponentGraph graph, ComponentDeconstructor deconstructor) { - // This is only used for shutdown, so no need to uninstall any bundles. - deconstructor.deconstruct(graph.allConstructedComponentsAndProviders(), Collections.emptyList()); + deconstructor.deconstruct(graph.allConstructedComponentsAndProviders()); } 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 ab7da7665b6..7095180dfc5 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,8 +13,6 @@ import java.util.Collections; import java.util.Set; import java.util.stream.Collectors; -import static java.util.Collections.emptySet; - /** * @author gjoranv * @author Tony Vaagenes @@ -25,13 +23,8 @@ public interface Osgi { return new BundleClasses(new MockBundle(), Collections.emptySet()); } - /** - * 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) { + default void 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 90bda0ef8a8..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 @@ -14,8 +14,6 @@ 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; @@ -285,16 +283,13 @@ public class ContainerTest extends ContainerTestBase { public void providers_are_destructed() { writeBootstrapConfigs("id1", DestructableProvider.class); - 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"); - }; + 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); @@ -378,14 +373,13 @@ public class ContainerTest extends ContainerTestBase { public static class TestDeconstructor implements ComponentDeconstructor { @Override - public void deconstruct(Collection<Object> components, Collection<Bundle> bundles) { + public void deconstruct(Collection<Object> components) { 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 18236a6bde9..79cb080dfa4 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,6 +5,7 @@ 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; @@ -15,8 +16,6 @@ import org.osgi.framework.Bundle; import java.util.Collection; import java.util.Set; -import static java.util.Collections.emptySet; - /** * @author Tony Vaagenes * @author gjoranv @@ -61,8 +60,7 @@ public class ContainerTestBase { } @Override - public Set<Bundle> useBundles(Collection<FileReference> bundles) { - return emptySet(); + public void useBundles(Collection<FileReference> bundles) { } @Override diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/DisableOsgiFramework.java b/container-disc/src/main/java/com/yahoo/container/jdisc/DisableOsgiFramework.java index b81923c5c0a..362fefa405d 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/DisableOsgiFramework.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/DisableOsgiFramework.java @@ -6,7 +6,6 @@ import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.BundleException; -import java.util.Collection; import java.util.List; /** @@ -53,16 +52,6 @@ public final class DisableOsgiFramework implements OsgiFramework { } @Override - public List<Bundle> getBundles(Bundle requestingBundle) { - throw newException(); - } - - @Override - public void allowDuplicateBundles(Collection<Bundle> bundles) { - throw newException(); - } - - @Override public void start() throws BundleException { throw newException(); } 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 371f29e86a3..284e3a69b61 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,8 +7,6 @@ import com.yahoo.container.di.ComponentDeconstructor; import com.yahoo.container.di.componentgraph.Provider; import com.yahoo.jdisc.SharedResource; import com.yahoo.log.LogLevel; -import org.osgi.framework.Bundle; -import org.osgi.framework.BundleException; import java.time.Duration; import java.util.ArrayList; @@ -19,7 +17,6 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; -import static java.util.logging.Level.SEVERE; import static java.util.logging.Level.WARNING; /** @@ -40,7 +37,7 @@ public class Deconstructor implements ComponentDeconstructor { } @Override - public void deconstruct(Collection<Object> components, Collection<Bundle> bundles) { + public void deconstruct(Collection<Object> components) { Collection<AbstractComponent> destructibleComponents = new ArrayList<>(); for (var component : components) { if (component instanceof AbstractComponent) { @@ -60,24 +57,20 @@ public class Deconstructor implements ComponentDeconstructor { } } if (! destructibleComponents.isEmpty()) - executor.schedule(new DestructComponentTask(destructibleComponents, bundles), - delay.getSeconds(), TimeUnit.SECONDS); + executor.schedule(new DestructComponentTask(destructibleComponents), delay.getSeconds(), TimeUnit.SECONDS); } private static class DestructComponentTask implements Runnable { private final Random random = new Random(System.nanoTime()); private final Collection<AbstractComponent> components; - private final Collection<Bundle> bundles; - DestructComponentTask(Collection<AbstractComponent> components, - Collection<Bundle> bundles) { + DestructComponentTask(Collection<AbstractComponent> components) { this.components = components; - this.bundles = bundles; } /** - * Returns a random delay between 0 and 10 minutes which will be different across identical containers invoking this at the same time. + * Returns a random delay betweeen 0 and 10 minutes which will be different across identical containers invoking this at the same time. * Used to randomize restart to avoid simultaneous cluster restarts. */ private Duration getRandomizedShutdownDelay() { @@ -87,6 +80,7 @@ public class Deconstructor implements ComponentDeconstructor { @Override public void run() { + log.info("Starting deconstruction of " + components.size() + " components"); for (var component : components) { log.info("Starting deconstruction of component " + component); try { @@ -108,19 +102,7 @@ public class Deconstructor implements ComponentDeconstructor { log.log(WARNING, "Non-error not exception throwable thrown when deconstructing component " + component, e); } } - // It should now be safe to uninstall the old bundles. - for (var bundle : bundles) { - try { - log.info("Uninstalling bundle " + bundle); - bundle.uninstall(); - } catch (BundleException e) { - log.log(SEVERE, "Could not uninstall bundle " + bundle); - } - } - // NOTE: It could be tempting to refresh packages here, but if active bundles were using any of - // the removed ones, they would switch wiring in the middle of a generation's lifespan. - // This would happen if the dependent active bundle has not been rebuilt with a new version - // of its dependency(ies). + log.info("Finished deconstructing " + components.size() + " components"); } } 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 345f75f7eb6..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 @@ -10,7 +10,6 @@ import org.junit.Test; import java.util.Collections; -import static java.util.Collections.emptyList; import static java.util.Collections.singleton; import static org.junit.Assert.assertTrue; @@ -29,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(singleton(abstractComponent), emptyList()); + deconstructor.deconstruct(singleton(abstractComponent)); int cnt = 0; while (! abstractComponent.destructed && (cnt++ < 12000)) { Thread.sleep(10); @@ -40,14 +39,14 @@ public class DeconstructorTest { @Test public void require_provider_destructed() { TestProvider provider = new TestProvider(); - deconstructor.deconstruct(singleton(provider), emptyList()); + deconstructor.deconstruct(singleton(provider)); assertTrue(provider.destructed); } @Test public void require_shared_resource_released() { TestSharedResource sharedResource = new TestSharedResource(); - deconstructor.deconstruct(singleton(sharedResource), emptyList()); + deconstructor.deconstruct(singleton(sharedResource)); assertTrue(sharedResource.released); } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java index b1aceb81bc6..f5346a21a4f 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java @@ -5,7 +5,6 @@ import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.BundleException; -import java.util.Collection; import java.util.List; /** @@ -14,7 +13,6 @@ import java.util.List; * {@link BundleInstaller} since that provides common convenience methods. * * @author Simon Thoresen Hult - * @author gjoranv */ public interface OsgiFramework { @@ -60,8 +58,6 @@ public interface OsgiFramework { /** * Synchronously refresh all bundles currently loaded. Once this method returns, the * class loaders of all bundles will reflect on the current set of loaded bundles. - * - * NOTE: This method is no longer used by the Jdisc container framework, but kept for completeness. */ void refreshPackages(); @@ -85,20 +81,6 @@ public interface OsgiFramework { List<Bundle> bundles(); /** - * Returns all installed bundles that are visible to the requesting bundle. Bundle visibility - * is controlled via implementations of {@link org.osgi.framework.hooks.bundle.FindHook}; - */ - List<Bundle> getBundles(Bundle requestingBundle); - - /** - * Allows this framework to install duplicates of the given collection of bundles. Duplicate detection - * is handled by the {@link com.yahoo.jdisc.core.BundleCollisionHook}. - * - * @param bundles The bundles to allow duplicates of - */ - void allowDuplicateBundles(Collection<Bundle> bundles); - - /** * This method starts the framework instance. Before this method is called, any call to {@link * #installBundle(String)} or {@link #bundles()} will generate a {@link NullPointerException}. * diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/BundleCollisionHook.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/BundleCollisionHook.java index ae1c81195ce..58ad5df9b0d 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/BundleCollisionHook.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/BundleCollisionHook.java @@ -15,7 +15,6 @@ import java.util.HashSet; import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.logging.Logger; /** * A bundle {@link CollisionHook} that contains a set of bundles that are allowed to collide with @@ -27,7 +26,6 @@ import java.util.logging.Logger; * @author gjoranv */ public class BundleCollisionHook implements CollisionHook, EventHook, FindHook { - private static Logger log = Logger.getLogger(BundleCollisionHook.class.getName()); private ServiceRegistration<?> registration; private Map<Bundle, BsnVersion> allowedDuplicates = new HashMap<>(5); @@ -107,7 +105,6 @@ public class BundleCollisionHook implements CollisionHook, EventHook, FindHook { } } } - log.info("Hiding bundles from bundle '" + context.getBundle() + "': " + bundlesToHide); bundles.removeAll(bundlesToHide); } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java index c14e513fb98..19a1707e97c 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java @@ -117,9 +117,6 @@ public class FelixFramework implements OsgiFramework { return sb.toString(); } - /** - * NOTE: This method is no longer used by the Jdisc container framework, but kept for completeness. - */ @Override public void refreshPackages() { FrameworkWiring wiring = felix.adapt(FrameworkWiring.class); @@ -156,13 +153,8 @@ public class FelixFramework implements OsgiFramework { return Arrays.asList(felix.getBundleContext().getBundles()); } - @Override public List<Bundle> getBundles(Bundle requestingBundle) { - log.fine(() -> "All bundles: " + bundles()); - log.fine(() -> "Getting visible bundles for bundle " + requestingBundle); - List<Bundle> visibleBundles = Arrays.asList(requestingBundle.getBundleContext().getBundles()); - log.fine(() -> "Visible bundles: " + visibleBundles); - return visibleBundles; + return Arrays.asList(requestingBundle.getBundleContext().getBundles()); } public void allowDuplicateBundles(Collection<Bundle> bundles) { diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/test/NonWorkingOsgiFramework.java b/jdisc_core/src/main/java/com/yahoo/jdisc/test/NonWorkingOsgiFramework.java index 0f927aa97d3..6b129e82a45 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/test/NonWorkingOsgiFramework.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/test/NonWorkingOsgiFramework.java @@ -5,7 +5,6 @@ import com.yahoo.jdisc.application.OsgiFramework; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; -import java.util.Collection; import java.util.Collections; import java.util.List; @@ -40,14 +39,6 @@ public class NonWorkingOsgiFramework implements OsgiFramework { } @Override - public List<Bundle> getBundles(Bundle requestingBundle) { - return Collections.emptyList(); - } - - @Override - public void allowDuplicateBundles(Collection<Bundle> bundles) { } - - @Override public void start() { } diff --git a/standalone-container/src/main/java/com/yahoo/application/container/impl/ClassLoaderOsgiFramework.java b/standalone-container/src/main/java/com/yahoo/application/container/impl/ClassLoaderOsgiFramework.java index 0d5b4c85264..08a70db7562 100644 --- a/standalone-container/src/main/java/com/yahoo/application/container/impl/ClassLoaderOsgiFramework.java +++ b/standalone-container/src/main/java/com/yahoo/application/container/impl/ClassLoaderOsgiFramework.java @@ -63,7 +63,7 @@ public final class ClassLoaderOsgiFramework implements OsgiFramework { @Override public List<Bundle> installBundle(String bundleLocation) { - if (bundleLocation != null && ! bundleLocation.isEmpty()) { + if (bundleLocation != null && bundleLocation.isEmpty() == false) { try { URL url = new URL(bundleLocation); bundleLocations.add(url); @@ -106,14 +106,6 @@ public final class ClassLoaderOsgiFramework implements OsgiFramework { } @Override - public List<Bundle> getBundles(Bundle requestingBundle) { - return bundleList; - } - - @Override - public void allowDuplicateBundles(Collection<Bundle> bundles) { } - - @Override public void start() { } diff --git a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java index cfd5f753c4f..04c3396c95c 100644 --- a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java +++ b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java @@ -24,7 +24,6 @@ import java.net.InetSocketAddress; import java.nio.channels.ServerSocketChannel; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Hashtable; import java.util.List; @@ -143,14 +142,6 @@ public class StandaloneContainerActivator implements BundleActivator { } @Override - public List<Bundle> getBundles(Bundle requestingBundle) { - return Collections.emptyList(); - } - - @Override - public void allowDuplicateBundles(Collection<Bundle> bundles) { } - - @Override public void start() { } |