Fix ModuleFactory not found errors 11/28211/5
authorTom Pantelis <tpanteli@brocade.com>
Wed, 7 Oct 2015 18:24:40 +0000 (14:24 -0400)
committerGerrit Code Review <gerrit@opendaylight.org>
Wed, 14 Oct 2015 07:05:12 +0000 (07:05 +0000)
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>
opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ModuleFactoryNotFoundException.java [new file with mode: 0644]
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java
opendaylight/config/config-persister-impl/src/main/java/org/opendaylight/controller/config/persist/impl/ConfigPusherImpl.java

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
@@ -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;
+    }
+}
index ca318f3..b945bf6 100644 (file)
@@ -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();
index c212300..65e185e 100644 (file)
@@ -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();
     }
index 049f1a6..1f538e5 100644 (file)
@@ -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) {
index fbc86a4..77f355e 100644 (file)
@@ -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);