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-core/src | |
parent | 73cbfef0434123f59584f9ed5f5cceac6715adbd (diff) |
Reapply "Gjoranv/allow duplicate bundles"
This reverts commit 2ef1e922a1d845b3cd79e9fb329925e7e9896919.
Diffstat (limited to 'container-core/src')
8 files changed, 181 insertions, 64 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 2b3a272fadc..4b8f21469d1 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,7 +10,6 @@ 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; @@ -26,11 +25,18 @@ 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 { - private final List<Bundle> initialBundles; - + /* 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 Map<FileReference, List<Bundle>> reference2Bundles = new LinkedHashMap<>(); private final Logger log = Logger.getLogger(BundleLoader.class.getName()); @@ -38,7 +44,6 @@ 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 { @@ -46,17 +51,19 @@ public class BundleLoader { return osgi.install(file.getAbsolutePath()); } - /** Returns the number of bundles installed by this call. */ - private int install(List<FileReference> references) { + private void 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) { @@ -97,6 +104,7 @@ public class BundleLoader { } List<Bundle> bundles = osgi.install(file.getAbsolutePath()); + reference2Bundles.put(reference, bundles); } @@ -113,13 +121,16 @@ public class BundleLoader { } } - // All bundles must have been started first to ensure correct package resolution. + /** + * 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. + */ private void startBundles() { for (List<Bundle> bundles : reference2Bundles.values()) { for (Bundle bundle : bundles) { try { if ( ! isFragment(bundle)) - bundle.start(); + bundle.start(); // NOP for already ACTIVE bundles } catch(Exception e) { throw new RuntimeException("Could not start bundle '" + bundle.getSymbolicName() + "'", e); } @@ -135,38 +146,44 @@ public class BundleLoader { return (bundleRevision.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0; } - /** Returns the number of uninstalled bundles */ - private int retainOnly(List<FileReference> newReferences) { - Set<Bundle> bundlesToRemove = new HashSet<>(Arrays.asList(osgi.getBundles())); + /** + * 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()); for (FileReference fileReferenceToKeep: newReferences) { if (reference2Bundles.containsKey(fileReferenceToKeep)) bundlesToRemove.removeAll(reference2Bundles.get(fileReferenceToKeep)); } - bundlesToRemove.removeAll(initialBundles); - for (Bundle bundle : bundlesToRemove) { - log.info("Removing bundle '" + bundle.toString() + "'"); - osgi.uninstall(bundle); - } + bundlesToRemove.removeAll(osgi.getInitialBundles()); + removeInactiveFileReferences(newReferences); + return bundlesToRemove; + } + + private void removeInactiveFileReferences(List<FileReference> newReferences) { + // Clean up the map of active bundles Set<FileReference> fileReferencesToRemove = new HashSet<>(reference2Bundles.keySet()); fileReferencesToRemove.removeAll(newReferences); - - for (FileReference fileReferenceToRemove : fileReferencesToRemove) { - reference2Bundles.remove(fileReferenceToRemove); - } - return bundlesToRemove.size(); + fileReferencesToRemove.forEach(reference2Bundles::remove); } - public synchronized int use(List<FileReference> bundles) { - int removedBundles = retainOnly(bundles); - int installedBundles = install(bundles); + /** + * 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); startBundles(); - log.info(removedBundles + " bundles were removed, and " + installedBundles + " bundles were installed."); log.info(installedBundlesMessage()); - return removedBundles + installedBundles; + return bundlesToUninstall; } 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 dd8b2605820..ef132694e10 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,6 +9,7 @@ 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; @@ -20,10 +21,9 @@ 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,19 +81,30 @@ public class HandlersConfigurerDi { Injector discInjector, OsgiFramework osgiFramework) { - this.vespaContainer = vespaContainer; - osgiWrapper = new OsgiWrapper(osgiFramework, new BundleLoader(new OsgiImpl(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; container = new Container(subscriberFactory, configId, deconstructor, osgiWrapper); getNewComponentGraph(discInjector, false); } - private static class OsgiWrapper extends OsgiImpl implements com.yahoo.container.di.Osgi { + private static class ContainerAndDiOsgi extends OsgiImpl implements OsgiWrapper { private final OsgiFramework osgiFramework; private final BundleLoader bundleLoader; - public OsgiWrapper(OsgiFramework osgiFramework, BundleLoader bundleLoader) { + public ContainerAndDiOsgi(OsgiFramework osgiFramework, BundleLoader bundleLoader) { super(osgiFramework); this.osgiFramework = osgiFramework; this.bundleLoader = bundleLoader; @@ -121,14 +132,9 @@ public class HandlersConfigurerDi { } @Override - public void useBundles(Collection<FileReference> bundles) { + public Set<Bundle> useBundles(Collection<FileReference> bundles) { log.info("Installing bundles from the latest application"); - - int bundlesRemovedOrInstalled = bundleLoader.use(new ArrayList<>(bundles)); - - if (bundlesRemovedOrInstalled > 0) { - refreshPackages(); - } + return bundleLoader.use(new ArrayList<>(bundles)); } } 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 684a45aeac1..9f49b016b68 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,7 +19,6 @@ 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; @@ -90,7 +89,7 @@ public class HandlersConfigurerTestWrapper { public HandlersConfigurerTestWrapper(Container container, String configId) { createFiles(configId); - MockOsgi mockOsgi = new MockOsgi(); + MockOsgiWrapper mockOsgiWrapper = new MockOsgiWrapper(); ComponentDeconstructor testDeconstructor = getTestDeconstructor(); configurer = new HandlersConfigurerDi( new CloudSubscriberFactory(configSources), @@ -98,16 +97,17 @@ public class HandlersConfigurerTestWrapper { configId, testDeconstructor, guiceInjector(), - mockOsgi); + mockOsgiWrapper); this.container = container; } private ComponentDeconstructor getTestDeconstructor() { - return components -> components.forEach(component -> { + return (components, bundles) -> 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 new file mode 100644 index 00000000000..a1f564f5682 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java @@ -0,0 +1,44 @@ +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 45ad02d2cef..d809c493565 100644 --- a/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java +++ b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java @@ -16,26 +16,28 @@ import java.util.List; public class MockOsgi extends NonWorkingOsgiFramework implements Osgi { @Override - public Bundle[] getBundles() { - return new Bundle[0]; + public List<Bundle> getInitialBundles() { + return Collections.emptyList(); } @Override - public Bundle getBundle(ComponentSpecification bundleId) { - return null; + public Bundle[] getBundles() { + return new Bundle[0]; } @Override - public List<Bundle> install(String absolutePath) { + public List<Bundle> getCurrentBundles() { return Collections.emptyList(); } @Override - public void uninstall(Bundle bundle) { + public Bundle getBundle(ComponentSpecification bundleId) { + return null; } @Override - public void refreshPackages() { + public List<Bundle> install(String absolutePath) { + return Collections.emptyList(); } } 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 c94eaf43deb..8f0acf41f30 100644 --- a/container-core/src/main/java/com/yahoo/osgi/Osgi.java +++ b/container-core/src/main/java/com/yahoo/osgi/Osgi.java @@ -4,6 +4,7 @@ package com.yahoo.osgi; import com.yahoo.component.ComponentSpecification; import org.osgi.framework.Bundle; +import java.util.Collection; import java.util.List; /** @@ -11,14 +12,17 @@ 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 uninstall(Bundle bundle); - - void refreshPackages(); + void allowDuplicateBundles(Collection<Bundle> bundles); } 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 8b2f20a1c13..ed93d15c975 100644 --- a/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java +++ b/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java @@ -7,19 +7,43 @@ 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 @@ -28,6 +52,10 @@ 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); @@ -86,8 +114,9 @@ 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 : getBundles()) { + for (Bundle bundle : getCurrentBundles()) { assert bundle.getSymbolicName() != null : "ensureHasBundleSymbolicName not called during installation"; if ( ! bundle.getSymbolicName().equals(id.getName())) continue; @@ -122,17 +151,16 @@ public class OsgiImpl implements Osgi { } @Override - public void uninstall(Bundle bundle) { - try { - bundle.uninstall(); - } catch (BundleException e) { - throw new RuntimeException(e); - } + public void allowDuplicateBundles(Collection<Bundle> bundles) { + jdiscOsgi.allowDuplicateBundles(bundles); } - @Override - public void refreshPackages() { - jdiscOsgi.refreshPackages(); + private static Bundle firstNonFrameworkBundle(List<Bundle> bundles) { + for (Bundle b : bundles) { + if (! (b instanceof Framework)) + return b; + } + return null; } } diff --git a/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java b/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java new file mode 100644 index 00000000000..58e19e52ee3 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java @@ -0,0 +1,16 @@ +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; + } + +} |