Fix ModuleFactory not found errors

https://git.opendaylight.org/gerrit/#/c/27874/ improvements to the
config system but had the side-effect of introducing timing issues where
a ModuleFactory wasn't found when trying to push a config. The reason is
that yang schemas load earlier and much quicker than ModuleFactory's,
which are scanned from ACTIVE bundles, so the capabilities may resolve
but a ModuleFactory may not be available yet. As a result, that patch
was partially reverted for the time being.

To fix the missing ModuleFactory issue, I added retries in the
ConfigPusherImpl when a ModuleFactory isn't found, similar to the
ConflictingVersionException retries. The backend now throws a new
checked exception, ModuleFactoryNotFoundException, which is caught to
trigger a retry after a delay. Prior, it threw an
InstanceNotFoundException which was wrapped in an
IllegalArgumentException. I didn't keep the InstanceNotFoundException
b/c it can be thrown for other reasons and I wanted to distinguish
missing ModuleFactoryNotFoundException.

I derived ModuleFactoryNotFoundException from RuntimeException to avoid
having to change signatures in the call chain and thus changing the API.
Prior it threw an unchecked IllegalArgumentException anyway so it's
consistent plus other areas of the code throw unchecked exceptions along
with checked exceptions.

Since the missing ModuleFactory issue is fixed, I re-enabled scanning of
RESOLVED bundles in the ModuleInfoBundleTracker.

Change-Id: I89ff346c0a89afdfa76ce402f2cf3211ac68b5c0
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>

diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ModuleFactoryNotFoundException.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ModuleFactoryNotFoundException.java
new file mode 100644 (file)
index 0000000..6d8f654
--- /dev/null
+++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ModuleFactoryNotFoundException.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2015 Brocade Communications Systems, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.controller.config.api;
+
+/**
+ * Exception thrown when a ModuleFactory is not found while pushing a config.
+ *
+ * @author Thomas Pantelis
+ */
+public class ModuleFactoryNotFoundException extends RuntimeException {
+    private static final long serialVersionUID = 1L;
+
+    private final String moduleName;
+
+    public ModuleFactoryNotFoundException(String moduleName) {
+        super("ModuleFactory not found for module name: " + moduleName);
+        this.moduleName = moduleName;
+    }
+
+    public String getModuleName() {
+        return moduleName;
+    }
+}

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java
index ca318f32c9efbf94c1fdd1683d5ebdaeced7d93f..b945bf6965d67acf33d17795b7c558debcc3971d 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java
@@ -9,7 +9,6 @@ package org.opendaylight.controller.config.manager.impl;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.lang.String.format;
-
 import com.google.common.collect.Lists;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -27,6 +26,7 @@ import javax.management.InstanceNotFoundException;
 import javax.management.MBeanServer;
 import javax.management.ObjectName;
 import org.opendaylight.controller.config.api.DependencyResolver;
+import org.opendaylight.controller.config.api.ModuleFactoryNotFoundException;
 import org.opendaylight.controller.config.api.ModuleIdentifier;
 import org.opendaylight.controller.config.api.ValidationException;
 import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnotation;
@@ -188,7 +188,8 @@ class ConfigTransactionControllerImpl implements
     }
 
 
-    private synchronized void copyExistingModule(ModuleInternalInfo oldConfigBeanInfo) throws InstanceAlreadyExistsException {
+    private synchronized void copyExistingModule(ModuleInternalInfo oldConfigBeanInfo)
+            throws InstanceAlreadyExistsException {
 
         transactionStatus.checkNotCommitStarted();
         transactionStatus.checkNotAborted();
@@ -200,7 +201,7 @@ class ConfigTransactionControllerImpl implements
         try {
             moduleFactory = factoriesHolder.findByModuleName(moduleIdentifier.getFactoryName());
             bc = getModuleFactoryBundleContext(moduleFactory.getImplementationName());
-        } catch (InstanceNotFoundException e) {
+        } catch (ModuleFactoryNotFoundException e) {
             throw new IllegalStateException(e);
         }
 
@@ -221,8 +222,8 @@ class ConfigTransactionControllerImpl implements
     }
 
     @Override
-    public synchronized ObjectName createModule(String factoryName,
-                                                String instanceName) throws InstanceAlreadyExistsException {
+    public synchronized ObjectName createModule(String factoryName, String instanceName)
+            throws InstanceAlreadyExistsException {
 
         transactionStatus.checkNotCommitStarted();
         transactionStatus.checkNotAborted();
@@ -230,12 +231,8 @@ class ConfigTransactionControllerImpl implements
         dependencyResolverManager.assertNotExists(moduleIdentifier);
 
         // find factory
-        ModuleFactory moduleFactory;
-        try {
-            moduleFactory = factoriesHolder.findByModuleName(factoryName);
-        } catch (InstanceNotFoundException e) {
-            throw new IllegalArgumentException(e);
-        }
+        ModuleFactory moduleFactory = factoriesHolder.findByModuleName(factoryName);
+
         DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(moduleIdentifier);
         BundleContext bundleContext = getModuleFactoryBundleContext(moduleFactory.getImplementationName());
         Module module = moduleFactory.createModule(instanceName, dependencyResolver,
@@ -452,6 +449,7 @@ class ConfigTransactionControllerImpl implements
         close();
     }
 
+    @Override
     public void close() {
         dependencyResolverManager.close();
         txLookupRegistry.close();

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java
index c212300fa6bb0642c3a665d7552b62857edff4d4..65e185e6166cc2b9def63aecbfa61d02425b1bf3 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java
@@ -15,7 +15,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
-import javax.management.InstanceNotFoundException;
+import org.opendaylight.controller.config.api.ModuleFactoryNotFoundException;
 import org.opendaylight.controller.config.spi.ModuleFactory;
 import org.osgi.framework.BundleContext;
 
@@ -55,11 +55,10 @@ public class HierarchicalConfigMBeanFactoriesHolder {
      * @throws IllegalArgumentException
      *             if factory is not found
      */
-    public ModuleFactory findByModuleName(String moduleName) throws InstanceNotFoundException {
+    public ModuleFactory findByModuleName(String moduleName) throws ModuleFactoryNotFoundException {
         Map.Entry<ModuleFactory, BundleContext> result = moduleNamesToConfigBeanFactories.get(moduleName);
         if (result == null) {
-            throw new InstanceNotFoundException(
-                    "ModuleFactory not found with module name: " + moduleName);
+            throw new ModuleFactoryNotFoundException(moduleName);
         }
         return result.getKey();
     }

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java
index 049f1a6814a288b056f40cf2e29ffe723f5c04a5..1f538e5c6dc848bb5f9ef334d8d12df0d50807c9 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java
@@ -68,7 +68,7 @@ public class ConfigManagerActivator implements BundleActivator {
             ModuleFactoryBundleTracker moduleFactoryTracker = new ModuleFactoryBundleTracker(
                     blankTransactionServiceTracker);
 
-            boolean scanResolvedBundlesForModuleInfo = false;
+            boolean scanResolvedBundlesForModuleInfo = true;
             BundleTracker<Collection<ObjectRegistration<YangModuleInfo>>> moduleInfoResolvedBundleTracker = null;
             ExtensibleBundleTracker<?> moduleFactoryBundleTracker;
             if(scanResolvedBundlesForModuleInfo) {

diff --git a/opendaylight/config/config-persister-impl/src/main/java/org/opendaylight/controller/config/persist/impl/ConfigPusherImpl.java b/opendaylight/config/config-persister-impl/src/main/java/org/opendaylight/controller/config/persist/impl/ConfigPusherImpl.java
index fbc86a43fdab99807aeb0688402a5193c5650881..77f355e0b25c27530c0d36e20bf6e46c5ebfd04b 100644 (file)
--- a/opendaylight/config/config-persister-impl/src/main/java/org/opendaylight/controller/config/persist/impl/ConfigPusherImpl.java
+++ b/opendaylight/config/config-persister-impl/src/main/java/org/opendaylight/controller/config/persist/impl/ConfigPusherImpl.java
@@ -9,7 +9,6 @@
 package org.opendaylight.controller.config.persist.impl;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-
 import com.google.common.base.Function;
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.Collections2;
@@ -29,6 +28,7 @@ import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 import javax.management.MBeanServerConnection;
 import org.opendaylight.controller.config.api.ConflictingVersionException;
+import org.opendaylight.controller.config.api.ModuleFactoryNotFoundException;
 import org.opendaylight.controller.config.api.ValidationException;
 import org.opendaylight.controller.config.facade.xml.ConfigExecution;
 import org.opendaylight.controller.config.facade.xml.ConfigSubsystemFacade;
@@ -57,9 +57,9 @@ public class ConfigPusherImpl implements ConfigPusher {
 
     private final long maxWaitForCapabilitiesMillis;
     private final long conflictingVersionTimeoutMillis;
-    private BlockingQueue<List<? extends ConfigSnapshotHolder>> queue = new LinkedBlockingQueue<>(QUEUE_SIZE);
+    private final BlockingQueue<List<? extends ConfigSnapshotHolder>> queue = new LinkedBlockingQueue<>(QUEUE_SIZE);
 
-    private ConfigSubsystemFacadeFactory facade;
+    private final ConfigSubsystemFacadeFactory facade;
     private ConfigPersisterNotificationHandler jmxNotificationHandler;
 
     public ConfigPusherImpl(ConfigSubsystemFacadeFactory facade, long maxWaitForCapabilitiesMillis,
@@ -96,6 +96,7 @@ public class ConfigPusherImpl implements ConfigPusher {
         }
     }
 
+    @Override
     public void pushConfigs(List<? extends ConfigSnapshotHolder> configs) throws InterruptedException {
         LOG.debug("Requested to push configs {}", configs);
         this.queue.put(configs);
@@ -217,7 +218,7 @@ public class ConfigPusherImpl implements ConfigPusher {
 
     static class NotEnoughCapabilitiesException extends ConfigPusherException {
         private static final long serialVersionUID = 1L;
-        private Set<String> missingCaps;
+        private final Set<String> missingCaps;
 
         NotEnoughCapabilitiesException(String message, Set<String> missingCaps) {
             super(message);
@@ -281,8 +282,8 @@ public class ConfigPusherImpl implements ConfigPusher {
         final ConfigSubsystemFacade currentFacade = this.facade.createFacade("config-push");
         try {
             ConfigExecution configExecution = createConfigExecution(xmlToBePersisted, currentFacade);
-            currentFacade.executeConfigExecution(configExecution);
-        } catch (ValidationException | DocumentedException e) {
+            executeWithMissingModuleFactoryRetries(currentFacade, configExecution);
+        } catch (ValidationException | DocumentedException | ModuleFactoryNotFoundException e) {
             LOG.trace("Validation for config: {} failed", configSnapshotHolder, e);
             throw new ConfigSnapshotFailureException(configSnapshotHolder.toString(), "edit", e);
         }
@@ -299,6 +300,24 @@ public class ConfigPusherImpl implements ConfigPusher {
         return true;
     }
 
+    private void executeWithMissingModuleFactoryRetries(ConfigSubsystemFacade facade, ConfigExecution configExecution)
+            throws DocumentedException, ValidationException, ModuleFactoryNotFoundException {
+        Stopwatch stopwatch = Stopwatch.createStarted();
+        ModuleFactoryNotFoundException lastException = null;
+        do {
+            try {
+                facade.executeConfigExecution(configExecution);
+                return;
+            } catch (ModuleFactoryNotFoundException e) {
+                LOG.debug("{} - will retry after timeout", e.toString());
+                lastException = e;
+                sleep();
+            }
+        } while (stopwatch.elapsed(TimeUnit.MILLISECONDS) < maxWaitForCapabilitiesMillis);
+
+        throw lastException;
+    }
+
     private ConfigExecution createConfigExecution(Element xmlToBePersisted, final ConfigSubsystemFacade currentFacade) throws DocumentedException {
         final Config configMapping = currentFacade.getConfigMapping();
         return currentFacade.getConfigExecution(configMapping, xmlToBePersisted);