Merge pull request #23858 from vespa-engine/minor-ApplicationBundleLoader-improvements

Minor application bundle loader improvements

1 files changed, 14 insertions, 11 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 1e30b19a48d..c294a7dd3ec 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
@@ -18,15 +18,18 @@ import java.util.logging.Logger;
 /**
  * Manages the set of installed and active/inactive bundles.
  *
+ * TODO: This class and the CollisionHook currently only handles a "current" and "previous" generation.
+ *       In order to correctly handle rapid reconfiguration and hence multiple generations, we need to
+ *       consider the graph generation number for each bundle.
+ *
  * @author gjoranv
- * @author Tony Vaagenes
  */
 public class ApplicationBundleLoader {
 
     private static final Logger log = Logger.getLogger(ApplicationBundleLoader.class.getName());
 
     // The active bundles for the current application generation.
-    private final Map<FileReference, Bundle> reference2Bundle = new LinkedHashMap<>();
+    private final Map<FileReference, Bundle> activeBundles = new LinkedHashMap<>();
 
     // The bundles that are obsolete from the previous generation, but kept in case the generation is reverted.
     private Map<FileReference, Bundle> obsoleteBundles = Map.of();
@@ -55,7 +58,7 @@ public class ApplicationBundleLoader {
         osgi.allowDuplicateBundles(bundlesToUninstall);
 
         bundlesFromNewGeneration = installBundles(newFileReferences);
-        BundleStarter.startBundles(reference2Bundle.values());
+        BundleStarter.startBundles(activeBundles.values());
         log.info(installedBundlesMessage());
 
         return bundlesToUninstall;
@@ -67,8 +70,8 @@ public class ApplicationBundleLoader {
      * be done by the Deconstructor as they may still be needed by components from the failed gen.
      */
     public synchronized Collection<Bundle> revertToPreviousGeneration() {
-        reference2Bundle.putAll(obsoleteBundles);
-        bundlesFromNewGeneration.forEach(reference2Bundle::remove);
+        activeBundles.putAll(obsoleteBundles);
+        bundlesFromNewGeneration.forEach(activeBundles::remove);
         Collection<Bundle> ret = bundlesFromNewGeneration.values();
 
         // For correct operation of the CollisionHook (more specifically its FindHook implementation), the set of
@@ -90,10 +93,10 @@ 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) {
-        Map<FileReference, Bundle> obsoleteReferences = new LinkedHashMap<>(reference2Bundle);
+        Map<FileReference, Bundle> obsoleteReferences = new LinkedHashMap<>(activeBundles);
         newReferences.forEach(obsoleteReferences::remove);
 
-        obsoleteReferences.forEach(reference2Bundle::remove);
+        obsoleteReferences.forEach(activeBundles::remove);
         return obsoleteReferences;
     }
 
@@ -104,7 +107,7 @@ public class ApplicationBundleLoader {
         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.
-        bundlesToInstall.removeAll(reference2Bundle.keySet());
+        bundlesToInstall.removeAll(activeBundles.keySet());
 
         if (bundlesToInstall.isEmpty()) return Map.of();
 
@@ -117,7 +120,7 @@ public class ApplicationBundleLoader {
     }
 
     private Map<FileReference, Bundle> installWithFileDistribution(Set<FileReference> bundlesToInstall,
-                                                    FileAcquirerBundleInstaller bundleInstaller) {
+                                                                   FileAcquirerBundleInstaller bundleInstaller) {
         var newBundles = new LinkedHashMap<FileReference, Bundle>();
 
         for (FileReference reference : bundlesToInstall) {
@@ -131,7 +134,7 @@ public class ApplicationBundleLoader {
                 if (bundles.size() > 1  && osgi.hasFelixFramework()) {
                     throw new RuntimeException("Bundle '" + bundles.get(0).getSymbolicName() + "' tried to pre-install bundles from disk.");
                 }
-                reference2Bundle.put(reference, bundles.get(0));
+                activeBundles.put(reference, bundles.get(0));
                 newBundles.put(reference, bundles.get(0));
             }
             catch(Exception e) {
@@ -152,7 +155,7 @@ public class ApplicationBundleLoader {
 
     // Only for testing
     List<FileReference> getActiveFileReferences() {
-        return new ArrayList<>(reference2Bundle.keySet());
+        return new ArrayList<>(activeBundles.keySet());
     }
 
 }