BUG-4514: do not retain old internal info

Retaining this information past second phase commit leads to it being
retained via dependencyResolver. Thus a series of reconfigurations will
invariably retain the complete history, which is completely wasteful and
constitutes a memory leak (with GC chains of ~1800 hops observed).

Change-Id: Id67b8813a1d55b36f0b55a1c96099b906bf313ad
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
index 39ea73be55a63788c17c0edcd5a9215e8550a1be..da6b83c86e8547b555eacd68bd01f857f5ff5b37 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
@@ -338,6 +338,11 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
 
                 // close old module jmx registrator
                 oldInternalInfo.getModuleJMXRegistrator().close();
 
                 // close old module jmx registrator
                 oldInternalInfo.getModuleJMXRegistrator().close();

+
+                // We no longer need old internal info. Clear it out, so we do not create a serial leak evidenced
+                // by BUG-4514. The reason is that modules retain their resolver, which retains modules. If we retain
+                // the old module, we would have the complete reconfiguration history held in heap for no good reason.
+                entry.clearOldInternalInfo();

             } else {
                 // new instance:
                 // wrap in readable dynamic mbean
             } else {
                 // new instance:
                 // wrap in readable dynamic mbean

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java
index 210b6d2aa702793fddb1d6f25f2c12c45074f6dc..fc9b645ea2b0220a301858edfef47d0474994c10 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java
@@ -8,6 +8,7 @@
 package org.opendaylight.controller.config.manager.impl.dependencyresolver;
 
 import com.google.common.base.Preconditions;
 package org.opendaylight.controller.config.manager.impl.dependencyresolver;
 
 import com.google.common.base.Preconditions;

+import javax.annotation.Nonnull;

 import javax.annotation.Nullable;
 import org.opendaylight.controller.config.api.ModuleIdentifier;
 import org.opendaylight.controller.config.manager.impl.ModuleInternalInfo;
 import javax.annotation.Nullable;
 import org.opendaylight.controller.config.api.ModuleIdentifier;
 import org.opendaylight.controller.config.manager.impl.ModuleInternalInfo;


@@ -22,11 +23,11 @@ public class ModuleInternalTransactionalInfo implements Identifiable<ModuleIdent
     private final ModuleIdentifier name;
     private final Module proxiedModule, realModule;
     private final ModuleFactory moduleFactory;
     private final ModuleIdentifier name;
     private final Module proxiedModule, realModule;
     private final ModuleFactory moduleFactory;

-    @Nullable
-    private final ModuleInternalInfo maybeOldInternalInfo;
+

     private final TransactionModuleJMXRegistration transactionModuleJMXRegistration;
     private final boolean isDefaultBean;
     private final BundleContext bundleContext;
     private final TransactionModuleJMXRegistration transactionModuleJMXRegistration;
     private final boolean isDefaultBean;
     private final BundleContext bundleContext;

+    @Nullable private ModuleInternalInfo maybeOldInternalInfo;

 
     public ModuleInternalTransactionalInfo(ModuleIdentifier name, Module proxiedModule,
                                            ModuleFactory moduleFactory,
 
     public ModuleInternalTransactionalInfo(ModuleIdentifier name, Module proxiedModule,
                                            ModuleFactory moduleFactory,


@@ -69,11 +70,15 @@ public class ModuleInternalTransactionalInfo implements Identifiable<ModuleIdent
         return moduleFactory;
     }
 
         return moduleFactory;
     }
 

-    @Nullable
-    public ModuleInternalInfo getOldInternalInfo() {
+    @Nonnull public ModuleInternalInfo getOldInternalInfo() {

         return Preconditions.checkNotNull(maybeOldInternalInfo);
     }
 
         return Preconditions.checkNotNull(maybeOldInternalInfo);
     }
 

+    public void clearOldInternalInfo() {
+        Preconditions.checkState(maybeOldInternalInfo != null, "No old internal info present");
+        maybeOldInternalInfo = null;
+    }
+

     public TransactionModuleJMXRegistration getTransactionModuleJMXRegistration() {
         return transactionModuleJMXRegistration;
     }
     public TransactionModuleJMXRegistration getTransactionModuleJMXRegistration() {
         return transactionModuleJMXRegistration;
     }