Redesign application bundle loading with a 'complete' phase.

- To be able to uninstall bundles from a config generation that has
  failed, without affecting the bundles from the last successful
  generation. (Component reconfig with unchanged bundles was
  not handled correctly.)

2 files changed, 24 insertions, 13 deletions

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 b1bf45290df..b4a22566eef 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
@@ -70,20 +70,26 @@ public class Container {
     // TODO: try to simplify by returning the result even when the graph failed, instead of throwing here.
     public ComponentGraphResult waitForNextGraphGeneration(ComponentGraph oldGraph, Injector fallbackInjector, boolean isInitializing) {
         try {
+            ComponentGraph newGraph;
             Collection<Bundle> obsoleteBundles = new HashSet<>();
-            ComponentGraph newGraph = waitForNewConfigGenAndCreateGraph(oldGraph, fallbackInjector, isInitializing, obsoleteBundles);
-            newGraph.reuseNodes(oldGraph);
+            try {
+                newGraph = waitForNewConfigGenAndCreateGraph(oldGraph, fallbackInjector, isInitializing, obsoleteBundles);
+                newGraph.reuseNodes(oldGraph);
+            } catch (Throwable t) {
+                log.warning("Failed to set up component graph - uninstalling latest bundles. Bootstrap generation: " + getBootstrapGeneration());
+                osgi.completeBundleGeneration(Osgi.GenerationStatus.FAILURE);
+                throw t;
+            }
             try {
                 constructComponents(newGraph);
             } catch (Throwable e) {
-                log.log(Level.WARNING, String.format(
-                        "Failed to construct graph for generation '%d' - scheduling partial graph for deconstruction",
-                        newGraph.generation()), e);
-
-                Collection<Bundle> newBundlesFromFailedGen = osgi.revertApplicationBundles();
+                log.warning("Failed to construct components for generation '" + newGraph.generation() + "' - scheduling partial graph for deconstruction");
+                Collection<Bundle> newBundlesFromFailedGen = osgi.completeBundleGeneration(Osgi.GenerationStatus.FAILURE);
                 deconstructFailedGraph(oldGraph, newGraph, newBundlesFromFailedGen);
                 throw e;
             }
+            // TODO: take obsoleteBundles as return value here!
+            osgi.completeBundleGeneration(Osgi.GenerationStatus.SUCCESS);
             Runnable cleanupTask = createPreviousGraphDeconstructionTask(oldGraph, newGraph, obsoleteBundles);
             return new ComponentGraphResult(newGraph, cleanupTask);
         } catch (Throwable t) {
@@ -194,7 +200,7 @@ public class Container {
 
     private Set<Bundle> installApplicationBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) {
         ApplicationBundlesConfig applicationBundlesConfig = getConfig(applicationBundlesConfigKey, configsIncludingBootstrapConfigs);
-        return osgi.useApplicationBundles(applicationBundlesConfig.bundles(), getComponentsGeneration());
+        return 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 73d39c50f02..c13ebca6f62 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
@@ -24,10 +24,14 @@ import static java.util.Collections.emptySet;
  */
 public interface Osgi {
 
+    enum GenerationStatus { SUCCESS, FAILURE }
+
     default void installPlatformBundles(Collection<String> bundlePaths) {
     }
 
     /**
+     * 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.
      *
@@ -40,13 +44,14 @@ public interface Osgi {
     }
 
     /**
-     * To be used when a new application generation fails, e.g. during component construction.
-     * Reverts all state related to application bundles to the previous generation.
+     * 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.
      *
-     * Returns the set of bundles that was exclusively used by the new generation,
-     * and therefore should be scheduled for uninstalling.
+     * @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.
      */
-    default Collection<Bundle> revertApplicationBundles() {
+    default Collection<Bundle> completeBundleGeneration(GenerationStatus status) {
         return emptySet();
     }