From: Tom Pantelis Date: Wed, 7 Oct 2015 18:24:40 +0000 (-0400) Subject: Fix ModuleFactory not found errors X-Git-Tag: release/beryllium~234 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=6998123adad9e88cd20f285dd48e433f71e90071 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 --- 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 index 0000000000..6d8f654463 --- /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 ca318f32c9..b945bf6965 100644 --- 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 c212300fa6..65e185e616 100644 --- 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 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 049f1a6814..1f538e5c6d 100644 --- 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>> 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 fbc86a43fd..77f355e0b2 100644 --- 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> queue = new LinkedBlockingQueue<>(QUEUE_SIZE); + private final BlockingQueue> 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 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 missingCaps; + private final Set missingCaps; NotEnoughCapabilitiesException(String message, Set 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);