diff options
Diffstat (limited to 'container-core')
6 files changed, 42 insertions, 46 deletions
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java b/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java index c1cf18582d5..cbff119c0ba 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java @@ -50,51 +50,49 @@ public class ApplicationBundleLoader { } /** - * 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. - * - * TODO: return void, and instead return the bundles to remove from completeGeneration() + * Installs the given set of bundles and updates state for which bundles and file references + * that are active or should be uninstalled in case of success or failure. */ - public synchronized Set<Bundle> useBundles(List<FileReference> newFileReferences) { + public synchronized void useBundles(List<FileReference> newFileReferences) { if (! readyForNewBundles) throw new IllegalStateException("Bundles must be committed or reverted before using new bundles."); - obsoleteBundles = removeObsoleteBundles(newFileReferences); - Set<Bundle> bundlesToUninstall = new LinkedHashSet<>(obsoleteBundles.values()); - log.info("Bundles to schedule for uninstall: " + bundlesToUninstall); - - osgi.allowDuplicateBundles(bundlesToUninstall); + obsoleteBundles = removeObsoleteReferences(newFileReferences); + osgi.allowDuplicateBundles(obsoleteBundles.values()); bundlesFromNewGeneration = installBundles(newFileReferences); BundleStarter.startBundles(activeBundles.values()); log.info(installedBundlesMessage()); readyForNewBundles = false; - - return bundlesToUninstall; } public synchronized Collection<Bundle> completeGeneration(GenerationStatus status) { Collection<Bundle> ret = List.of(); if (readyForNewBundles) return ret; + readyForNewBundles = true; if (status == GenerationStatus.SUCCESS) { - commitBundles(); + return commitBundles(); } else { - ret = revertToPreviousGeneration(); + return revertToPreviousGeneration(); } - readyForNewBundles = true; - return ret; } /** * Commit to the current set of bundles. Must be called after the component graph creation proved successful, * to prevent uninstalling bundles unintentionally upon a future call to {@link #revertToPreviousGeneration()}. + * Returns the set of bundles that is no longer used by the application, and should therefore be scheduled + * for uninstall. */ - private void commitBundles() { + private Collection<Bundle> commitBundles() { + Set<Bundle> bundlesToUninstall = new LinkedHashSet<>(obsoleteBundles.values()); + log.info("Bundles to be uninstalled from previous generation: " + bundlesToUninstall); + bundlesFromNewGeneration = Map.of(); obsoleteBundles = Map.of(); readyForNewBundles = true; + return bundlesToUninstall; } /** @@ -128,7 +126,7 @@ public class ApplicationBundleLoader { * * Returns the map of bundles that are not needed by the new application generation. */ - private Map<FileReference, Bundle> removeObsoleteBundles(List<FileReference> newReferences) { + private Map<FileReference, Bundle> removeObsoleteReferences(List<FileReference> newReferences) { Map<FileReference, Bundle> obsoleteReferences = new LinkedHashMap<>(activeBundles); newReferences.forEach(obsoleteReferences::remove); 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 f573de1f4ac..f2620cc8baa 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 @@ -98,9 +98,9 @@ public class HandlersConfigurerDi { } @Override - public Set<Bundle> useApplicationBundles(Collection<FileReference> bundles, long generation) { + public void useApplicationBundles(Collection<FileReference> bundles, long generation) { log.info("Installing bundles for application generation " + generation); - return applicationBundleLoader.useBundles(new ArrayList<>(bundles)); + applicationBundleLoader.useBundles(new ArrayList<>(bundles)); } @Override diff --git a/container-core/src/main/java/com/yahoo/container/di/Container.java b/container-core/src/main/java/com/yahoo/container/di/Container.java index c34e393ac02..a3e690df258 100644 --- a/container-core/src/main/java/com/yahoo/container/di/Container.java +++ b/container-core/src/main/java/com/yahoo/container/di/Container.java @@ -73,7 +73,7 @@ public class Container { ComponentGraph newGraph; Collection<Bundle> obsoleteBundles = new HashSet<>(); try { - newGraph = waitForNewConfigGenAndCreateGraph(oldGraph, fallbackInjector, isInitializing, obsoleteBundles); + newGraph = waitForNewConfigGenAndCreateGraph(oldGraph, fallbackInjector, isInitializing); newGraph.reuseNodes(oldGraph); } catch (Throwable t) { log.warning("Failed to set up component graph - uninstalling latest bundles. Bootstrap generation: " + getBootstrapGeneration()); @@ -89,9 +89,8 @@ public class Container { deconstructFailedGraph(oldGraph, newGraph, newBundlesFromFailedGen); throw e; } - // TODO: take obsoleteBundles as return value here! - osgi.completeBundleGeneration(Osgi.GenerationStatus.SUCCESS); - Runnable cleanupTask = createPreviousGraphDeconstructionTask(oldGraph, newGraph, obsoleteBundles); + Collection<Bundle> unusedBundlesFromPreviousGen = osgi.completeBundleGeneration(Osgi.GenerationStatus.SUCCESS); + Runnable cleanupTask = createPreviousGraphDeconstructionTask(oldGraph, newGraph, unusedBundlesFromPreviousGen); return new ComponentGraphResult(newGraph, cleanupTask); } catch (Throwable t) { invalidateGeneration(oldGraph.generation(), t); @@ -108,7 +107,7 @@ public class Container { } private ComponentGraph waitForNewConfigGenAndCreateGraph( - ComponentGraph graph, Injector fallbackInjector, boolean isInitializing, Collection<Bundle> obsoleteBundles) // NOTE: Return value + ComponentGraph graph, Injector fallbackInjector, boolean isInitializing) { ConfigSnapshot snapshot; while (true) { @@ -132,8 +131,7 @@ public class Container { } else { throwIfPlatformBundlesChanged(snapshot); } - Collection<Bundle> bundlesToRemove = installApplicationBundles(snapshot.configs()); - obsoleteBundles.addAll(bundlesToRemove); + installApplicationBundles(snapshot.configs()); graph = createComponentGraph(snapshot.configs(), getBootstrapGeneration(), fallbackInjector); @@ -203,9 +201,9 @@ public class Container { return () -> destructor.deconstruct(oldGraph.generation(), obsoleteComponents, obsoleteBundles); } - private Set<Bundle> installApplicationBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) { + private void installApplicationBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) { ApplicationBundlesConfig applicationBundlesConfig = getConfig(applicationBundlesConfigKey, configsIncludingBootstrapConfigs); - return osgi.useApplicationBundles(applicationBundlesConfig.bundles(), getBootstrapGeneration()); + osgi.useApplicationBundles(applicationBundlesConfig.bundles(), getBootstrapGeneration()); } private ComponentGraph createComponentGraph(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs, diff --git a/container-core/src/main/java/com/yahoo/container/di/Osgi.java b/container-core/src/main/java/com/yahoo/container/di/Osgi.java index c13ebca6f62..463ac627a19 100644 --- a/container-core/src/main/java/com/yahoo/container/di/Osgi.java +++ b/container-core/src/main/java/com/yahoo/container/di/Osgi.java @@ -30,26 +30,23 @@ public interface Osgi { } /** - * TODO: return void and let all obsolete bundles be returned by completeBundleGeneration - * - * Returns the set of bundles that is not needed by the new application generation, - * and therefore should be scheduled for uninstalling. + * Installs the new set of application bundles. * * @param bundles The bundles for the new application. * @param generation The generation number of the new application. - * @return the set of bundles that is not needed by the new application generation, */ - default Set<Bundle> useApplicationBundles(Collection<FileReference> bundles, long generation) { - return emptySet(); + default void useApplicationBundles(Collection<FileReference> bundles, long generation) { } /** + * If the current generation is a success, the set of bundles that is not needed by the new application + * generation, and therefore should be scheduled for uninstalling, is returned. * If the current generation is a failure, all state related to application bundles is reverted to * the previous generation. The set of bundles that was exclusively needed by the new generation, * and therefore should be scheduled for uninstalling, is returned. * * @param status The success or failure of the new generation - * @return The set of bundles that are no longer needed by the latest good generation. + * @return The set of bundles that are no longer needed by the new or latest good generation. */ default Collection<Bundle> completeBundleGeneration(GenerationStatus status) { return emptySet(); diff --git a/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java b/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java index fdd44fe358d..8a3243ab1a9 100644 --- a/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java +++ b/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java @@ -60,7 +60,8 @@ public class ApplicationBundleLoaderTest { bundleLoader.useBundles(List.of(BUNDLE_1_REF)); bundleLoader.completeGeneration(GenerationStatus.SUCCESS); - Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_1_REF, BUNDLE_2_REF)); + bundleLoader.useBundles(List.of(BUNDLE_1_REF, BUNDLE_2_REF)); + Collection<Bundle> obsoleteBundles = bundleLoader.completeGeneration(GenerationStatus.SUCCESS); // No bundles are obsolete assertTrue(obsoleteBundles.isEmpty()); @@ -71,7 +72,8 @@ public class ApplicationBundleLoaderTest { bundleLoader.useBundles(List.of(BUNDLE_1_REF)); bundleLoader.completeGeneration(GenerationStatus.SUCCESS); - Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_1_REF, BUNDLE_2_REF)); + bundleLoader.useBundles(List.of(BUNDLE_1_REF, BUNDLE_2_REF)); + Collection<Bundle> obsoleteBundles = bundleLoader.completeGeneration(GenerationStatus.SUCCESS); // No bundles are obsolete assertTrue(obsoleteBundles.isEmpty()); @@ -97,7 +99,8 @@ public class ApplicationBundleLoaderTest { bundleLoader.useBundles(List.of(BUNDLE_1_REF)); bundleLoader.completeGeneration(GenerationStatus.SUCCESS); - Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_2_REF)); + bundleLoader.useBundles(List.of(BUNDLE_2_REF)); + Collection<Bundle> obsoleteBundles = bundleLoader.completeGeneration(GenerationStatus.SUCCESS); // The returned set of obsolete bundles contains bundle-1 assertEquals(1, obsoleteBundles.size()); @@ -123,10 +126,9 @@ public class ApplicationBundleLoaderTest { bundleLoader.useBundles(List.of(BUNDLE_1_REF)); bundleLoader.completeGeneration(GenerationStatus.SUCCESS); - Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_2_REF)); - assertEquals(BUNDLE_1.getSymbolicName(), obsoleteBundles.iterator().next().getSymbolicName()); + bundleLoader.useBundles(List.of(BUNDLE_2_REF)); - // Revert to the previous generation, as will be done upon a failed reconfig. + // Report the generation as a failure. Collection<Bundle> bundlesToUninstall = bundleLoader.completeGeneration(GenerationStatus.FAILURE); assertEquals(1, bundlesToUninstall.size()); @@ -153,7 +155,8 @@ public class ApplicationBundleLoaderTest { assertEquals(0, bundlesToUninstall.size()); assertEquals(1, osgi.getCurrentBundles().size()); - Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_1_REF)); + bundleLoader.useBundles(List.of(BUNDLE_1_REF)); + Collection<Bundle> obsoleteBundles = bundleLoader.completeGeneration(GenerationStatus.SUCCESS); assertTrue(obsoleteBundles.isEmpty()); assertEquals(1, osgi.getCurrentBundles().size()); } diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java b/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java index d634d7b7ce2..2bd3a325d48 100644 --- a/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java +++ b/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java @@ -65,8 +65,8 @@ public class TestOsgi extends MockOsgi implements com.yahoo.container.di.Osgi { } @Override - public Set<Bundle> useApplicationBundles(Collection<FileReference> bundles, long generation) { - return bundleLoader.useBundles(new ArrayList<>(bundles)); + public void useApplicationBundles(Collection<FileReference> bundles, long generation) { + bundleLoader.useBundles(new ArrayList<>(bundles)); } @Override |