From 4f27eb271720e9d7eaa2f720672a2e76de82c40e Mon Sep 17 00:00:00 2001 From: Tomas Olvecky Date: Fri, 14 Feb 2014 16:22:10 +0100 Subject: [PATCH] Add watchdog thread to detect illegal blocking calls during second phase commit. Add a thread that starts when second phase commit starts, that will monitor if a module instance occupies the commit thread for more than 5 seconds. If so, log a warning every second informing about possible misuse of config subsystem to block on external resources. Added a proxy module that caches Module#getInstance() call as this method is idempotent. Change-Id: Ia1e56386bdd8c6b7a6dc625cfc69253b927c92a9 Signed-off-by: Tomas Olvecky --- .../controller/config/spi/Module.java | 14 +- opendaylight/config/config-manager/pom.xml | 1 - .../config/manager/impl/CommitInfo.java | 2 + .../manager/impl/ConfigRegistryImpl.java | 76 ++++++----- .../impl/ConfigTransactionControllerImpl.java | 15 +-- .../ConfigTransactionControllerInternal.java | 6 +- .../config/manager/impl/DeadlockMonitor.java | 122 ++++++++++++++++++ .../manager/impl/ModuleInternalInfo.java | 1 + .../DependencyResolverImpl.java | 5 +- .../DependencyResolverManager.java | 88 ++++++++++--- .../DestroyedModule.java | 4 +- .../ModuleInternalTransactionalInfo.java | 26 ++-- .../dependencyresolver/ModulesHolder.java | 26 ++-- .../dependencyresolver/TransactionHolder.java | 41 ------ .../DependencyResolverManagerTest.java | 41 +++--- .../TestingParallelAPSPModule.java | 18 ++- .../test/SimpleConfigurationTest.java | 10 +- 17 files changed, 328 insertions(+), 168 deletions(-) create mode 100644 opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/DeadlockMonitor.java rename opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/{ => dependencyresolver}/DestroyedModule.java (94%) rename opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/{ => dependencyresolver}/ModuleInternalTransactionalInfo.java (75%) delete mode 100644 opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/TransactionHolder.java diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/Module.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/Module.java index dd11b7503f..1b16ec8284 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/Module.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/Module.java @@ -7,11 +7,11 @@ */ package org.opendaylight.controller.config.spi; -import javax.annotation.concurrent.NotThreadSafe; - import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.yangtools.concepts.Identifiable; +import javax.annotation.concurrent.NotThreadSafe; + /** * Represents one service that is to be configured. These methods need to be @@ -20,7 +20,7 @@ import org.opendaylight.yangtools.concepts.Identifiable; * ConfigBeans. *

* In order to guide dependency resolution, the setter method should be - * annotated with {@link RequireInterface}. + * annotated with {@link org.opendaylight.controller.config.api.annotations.RequireInterface}. *

*

* Thread safety note: implementations of this interface are not required to be @@ -43,10 +43,10 @@ public interface Module extends Identifiable{ * Returns 'live' object that was configured using this object. It is * allowed to call this method only after all ConfigBeans were validated. In * this method new resources might be opened or old instance might be - * modified. Note that when obtaining dependent Module using - * {@link org.opendaylight.controller.config.api.DependencyResolver#validateDependency(Class, javax.management.ObjectName, String)} - * a proxy will be created that will disallow calling this method before - * second commit phase begins. + * modified. This method must be implemented so that it returns same + * result for a single transaction. Since Module is created per transaction + * this means that it must be safe to cache result of first call. + * * * @return closeable instance: After bundle update the factory might be able * to copy old configuration into new one without being able to cast diff --git a/opendaylight/config/config-manager/pom.xml b/opendaylight/config/config-manager/pom.xml index 4857f2a201..7d7d9d697a 100644 --- a/opendaylight/config/config-manager/pom.xml +++ b/opendaylight/config/config-manager/pom.xml @@ -68,7 +68,6 @@ com.google.guava guava - test org.opendaylight.yangtools diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/CommitInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/CommitInfo.java index 9d086295e3..28732d8f68 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/CommitInfo.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/CommitInfo.java @@ -14,6 +14,8 @@ import java.util.Map; import javax.annotation.concurrent.Immutable; import org.opendaylight.controller.config.api.ModuleIdentifier; +import org.opendaylight.controller.config.manager.impl.dependencyresolver.DestroyedModule; +import org.opendaylight.controller.config.manager.impl.dependencyresolver.ModuleInternalTransactionalInfo; /** * Structure obtained during first phase commit, contains destroyed modules from 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 8f6a4654b2..dd510a1ed7 100644 --- 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 @@ -7,6 +7,7 @@ */ package org.opendaylight.controller.config.manager.impl; +import com.google.common.collect.Maps; import org.opendaylight.controller.config.api.ConflictingVersionException; import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.api.RuntimeBeanRegistratorAwareModule; @@ -14,6 +15,8 @@ import org.opendaylight.controller.config.api.ServiceReferenceWritableRegistry; import org.opendaylight.controller.config.api.ValidationException; import org.opendaylight.controller.config.api.jmx.CommitStatus; import org.opendaylight.controller.config.api.jmx.ObjectNameUtil; +import org.opendaylight.controller.config.manager.impl.dependencyresolver.DestroyedModule; +import org.opendaylight.controller.config.manager.impl.dependencyresolver.ModuleInternalTransactionalInfo; import org.opendaylight.controller.config.manager.impl.dynamicmbean.DynamicReadableWrapper; import org.opendaylight.controller.config.manager.impl.factoriesresolver.HierarchicalConfigMBeanFactoriesHolder; import org.opendaylight.controller.config.manager.impl.factoriesresolver.ModuleFactoriesResolver; @@ -119,13 +122,13 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe MBeanServer configMBeanServer, BaseJMXRegistrator baseJMXRegistrator, CodecRegistry codecRegistry) { this.resolver = resolver; - beanToOsgiServiceManager = new BeanToOsgiServiceManager(); + this.beanToOsgiServiceManager = new BeanToOsgiServiceManager(); this.configMBeanServer = configMBeanServer; this.baseJMXRegistrator = baseJMXRegistrator; this.codecRegistry = codecRegistry; - registryMBeanServer = MBeanServerFactory + this.registryMBeanServer = MBeanServerFactory .createMBeanServer("ConfigRegistry" + configMBeanServer.getDefaultDomain()); - transactionsMBeanServer = MBeanServerFactory + this.transactionsMBeanServer = MBeanServerFactory .createMBeanServer("ConfigTransactions" + configMBeanServer.getDefaultDomain()); } @@ -173,7 +176,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe throw new IllegalStateException(e); } transactionController.copyExistingModulesAndProcessFactoryDiff(currentConfig.getEntries(), lastListOfFactories); - transactionsHolder.add(transactionName, transactionController); + transactionsHolder.add(transactionName, transactionController, txLookupRegistry); return transactionController; } @@ -189,12 +192,13 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe logger.trace("About to commit {}. Current parentVersion: {}, versionCounter {}", transactionName, version, versionCounter); // find ConfigTransactionController - Map transactions = transactionsHolder.getCurrentTransactions(); - ConfigTransactionControllerInternal configTransactionController = transactions.get(transactionName); - if (configTransactionController == null) { + Map> transactions = transactionsHolder.getCurrentTransactions(); + Entry configTransactionControllerEntry = transactions.get(transactionName); + if (configTransactionControllerEntry == null) { throw new IllegalArgumentException(String.format( "Transaction with name '%s' not found", transactionName)); } + ConfigTransactionControllerInternal configTransactionController = configTransactionControllerEntry.getKey(); // check optimistic lock if (version != configTransactionController.getParentVersion()) { throw new ConflictingVersionException( @@ -209,8 +213,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe lastListOfFactories = Collections.unmodifiableList(configTransactionController.getCurrentlyRegisteredFactories()); // non recoverable from here: try { - return secondPhaseCommit( - configTransactionController, commitInfo); + return secondPhaseCommit(configTransactionController, commitInfo, configTransactionControllerEntry.getValue()); } catch (Throwable t) { // some libs throw Errors: e.g. // javax.xml.ws.spi.FactoryFinder$ConfigurationError isHealthy = false; @@ -220,13 +223,13 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe } else if (t instanceof Error) { throw (Error) t; } else { - throw new IllegalStateException(t); + throw new RuntimeException(t); } } } private CommitStatus secondPhaseCommit(ConfigTransactionControllerInternal configTransactionController, - CommitInfo commitInfo) { + CommitInfo commitInfo, ConfigTransactionLookupRegistry txLookupRegistry) { // close instances which were destroyed by the user, including // (hopefully) runtime beans @@ -255,7 +258,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe .getRuntimeBeanRegistrator(); } // set runtime jmx registrator if required - Module module = entry.getModule(); + Module module = entry.getProxiedModule(); if (module instanceof RuntimeBeanRegistratorAwareModule) { ((RuntimeBeanRegistratorAwareModule) module) .setRuntimeBeanRegistrator(runtimeBeanRegistrator); @@ -265,8 +268,9 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe } // can register runtime beans - List orderedModuleIdentifiers = configTransactionController - .secondPhaseCommit(); + List orderedModuleIdentifiers = configTransactionController.secondPhaseCommit(); + txLookupRegistry.close(); + configTransactionController.close(); // copy configuration to read only mode List newInstances = new LinkedList<>(); @@ -283,7 +287,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe throw new NullPointerException("Module not found " + moduleIdentifier); } - Module module = entry.getModule(); + ObjectName primaryReadOnlyON = ObjectNameUtil .createReadOnlyModuleON(moduleIdentifier); @@ -300,13 +304,14 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe .createModuleJMXRegistrator(); OsgiRegistration osgiRegistration = null; + AutoCloseable instance = entry.getProxiedModule().getInstance(); if (entry.hasOldModule()) { ModuleInternalInfo oldInternalInfo = entry.getOldInternalInfo(); DynamicReadableWrapper oldReadableConfigBean = oldInternalInfo.getReadableModule(); currentConfig.remove(entry.getIdentifier()); // test if old instance == new instance - if (oldReadableConfigBean.getInstance().equals(module.getInstance())) { + if (oldReadableConfigBean.getInstance().equals(instance)) { // reused old instance: // wrap in readable dynamic mbean reusedInstances.add(primaryReadOnlyON); @@ -328,9 +333,10 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe // wrap in readable dynamic mbean newInstances.add(primaryReadOnlyON); } + Module realModule = entry.getRealModule(); DynamicReadableWrapper newReadableConfigBean = new DynamicReadableWrapper( - module, module.getInstance(), moduleIdentifier, + realModule, instance, moduleIdentifier, registryMBeanServer, configMBeanServer); // register to JMX @@ -347,7 +353,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe if (moduleFactory != null) { BundleContext bc = configTransactionController. getModuleFactoryBundleContext(moduleFactory.getImplementationName()); - osgiRegistration = beanToOsgiServiceManager.registerToOsgi(module.getClass(), + osgiRegistration = beanToOsgiServiceManager.registerToOsgi(realModule.getClass(), newReadableConfigBean.getInstance(), entry.getIdentifier(), bc); } else { throw new NullPointerException(entry.getIdentifier().getFactoryName() + " ModuleFactory not found."); @@ -362,7 +368,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe runtimeBeanRegistrator, newModuleJMXRegistrator, orderingIdx, entry.isDefaultBean()); - newConfigEntries.put(module, newInfo); + newConfigEntries.put(realModule, newInfo); orderingIdx++; } currentConfig.addAll(newConfigEntries.values()); @@ -371,8 +377,8 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe version = configTransactionController.getVersion(); // switch readable Service Reference Registry - readableSRRegistry.close(); - readableSRRegistry = ServiceReferenceRegistryImpl.createSRReadableRegistry( + this.readableSRRegistry.close(); + this.readableSRRegistry = ServiceReferenceRegistryImpl.createSRReadableRegistry( configTransactionController.getWritableRegistry(), this, baseJMXRegistrator); return new CommitStatus(newInstances, reusedInstances, @@ -384,12 +390,12 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe */ @Override public synchronized List getOpenConfigs() { - Map transactions = transactionsHolder + Map> transactions = transactionsHolder .getCurrentTransactions(); List result = new ArrayList<>(transactions.size()); - for (ConfigTransactionControllerInternal configTransactionController : transactions + for (Entry configTransactionControllerEntry : transactions .values()) { - result.add(configTransactionController.getControllerObjectName()); + result.add(configTransactionControllerEntry.getKey().getControllerObjectName()); } return result; } @@ -403,11 +409,14 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe @Override public synchronized void close() { // abort transactions - Map transactions = transactionsHolder + Map> transactions = transactionsHolder .getCurrentTransactions(); - for (ConfigTransactionControllerInternal configTransactionController : transactions + for (Entry configTransactionControllerEntry : transactions .values()) { + + ConfigTransactionControllerInternal configTransactionController = configTransactionControllerEntry.getKey(); try { + configTransactionControllerEntry.getValue().close(); configTransactionController.abortConfig(); } catch (RuntimeException e) { logger.warn("Ignoring exception while aborting {}", @@ -652,15 +661,16 @@ class TransactionsHolder { * every time current transactions are requested. */ @GuardedBy("ConfigRegistryImpl.this") - private final Map transactions = new HashMap<>(); + private final Map> transactions = new HashMap<>(); /** * Can only be called from within synchronized method. */ public void add(String transactionName, - ConfigTransactionControllerInternal transactionController) { + ConfigTransactionControllerInternal transactionController, ConfigTransactionLookupRegistry txLookupRegistry) { Object oldValue = transactions.put(transactionName, - transactionController); + Maps.immutableEntry(transactionController, txLookupRegistry)); if (oldValue != null) { throw new IllegalStateException( "Error: two transactions with same name"); @@ -674,13 +684,13 @@ class TransactionsHolder { * * @return current view on transactions map. */ - public Map getCurrentTransactions() { + public Map> getCurrentTransactions() { // first, remove closed transaction - for (Iterator> it = transactions + for (Iterator>> it = transactions .entrySet().iterator(); it.hasNext(); ) { - Entry entry = it + Entry> entry = it .next(); - if (entry.getValue().isClosed()) { + if (entry.getValue().getKey().isClosed()) { it.remove(); } } 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 f7afded51f..84f76c9936 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 @@ -13,6 +13,7 @@ import org.opendaylight.controller.config.api.ServiceReferenceWritableRegistry; import org.opendaylight.controller.config.api.ValidationException; import org.opendaylight.controller.config.api.jmx.ObjectNameUtil; import org.opendaylight.controller.config.manager.impl.dependencyresolver.DependencyResolverManager; +import org.opendaylight.controller.config.manager.impl.dependencyresolver.ModuleInternalTransactionalInfo; import org.opendaylight.controller.config.manager.impl.dynamicmbean.DynamicWritableWrapper; import org.opendaylight.controller.config.manager.impl.dynamicmbean.ReadOnlyAtomicBoolean; import org.opendaylight.controller.config.manager.impl.dynamicmbean.ReadOnlyAtomicBoolean.ReadOnlyAtomicBooleanImpl; @@ -95,7 +96,8 @@ class ConfigTransactionControllerImpl implements this.currentlyRegisteredFactories = currentlyRegisteredFactories; this.factoriesHolder = new HierarchicalConfigMBeanFactoriesHolder(currentlyRegisteredFactories); this.transactionStatus = new TransactionStatus(); - this.dependencyResolverManager = new DependencyResolverManager(transactionName, transactionStatus, writableSRRegistry, codecRegistry); + this.dependencyResolverManager = new DependencyResolverManager(txLookupRegistry.getTransactionIdentifier(), + transactionStatus, writableSRRegistry, codecRegistry); this.transactionsMBeanServer = transactionsMBeanServer; this.configMBeanServer = configMBeanServer; this.blankTransaction = blankTransaction; @@ -231,11 +233,10 @@ class ConfigTransactionControllerImpl implements // put wrapper to jmx TransactionModuleJMXRegistration transactionModuleJMXRegistration = getTxModuleJMXRegistrator() .registerMBean(writableDynamicWrapper, writableON); - ModuleInternalTransactionalInfo moduleInternalTransactionalInfo = new ModuleInternalTransactionalInfo( + + dependencyResolverManager.put( moduleIdentifier, module, moduleFactory, maybeOldConfigBeanInfo, transactionModuleJMXRegistration, isDefaultBean); - - dependencyResolverManager.put(moduleInternalTransactionalInfo); return writableON; } @@ -394,8 +395,6 @@ class ConfigTransactionControllerImpl implements logger.trace("Committed configuration {}", getTransactionIdentifier()); transactionStatus.setCommitted(); - // unregister this and all modules from jmx - close(); return dependencyResolverManager.getSortedModuleIdentifiers(); } @@ -413,8 +412,7 @@ class ConfigTransactionControllerImpl implements } public void close() { - //FIXME: should not close object that was retrieved in constructor, a wrapper object should do that perhaps - txLookupRegistry.close(); + dependencyResolverManager.close(); } @Override @@ -572,6 +570,7 @@ class ConfigTransactionControllerImpl implements return writableSRRegistry; } + @Override public TransactionIdentifier getTransactionIdentifier() { return txLookupRegistry.getTransactionIdentifier(); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerInternal.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerInternal.java index 82bae44a01..f6164e3256 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerInternal.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerInternal.java @@ -23,7 +23,7 @@ import org.osgi.framework.BundleContext; * and {@link ConfigRegistryImpl} (consumer). */ interface ConfigTransactionControllerInternal extends - ConfigTransactionControllerImplMXBean { + ConfigTransactionControllerImplMXBean, AutoCloseable { @@ -75,4 +75,8 @@ interface ConfigTransactionControllerInternal extends ServiceReferenceWritableRegistry getWritableRegistry(); + TransactionIdentifier getTransactionIdentifier(); + + @Override + void close(); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/DeadlockMonitor.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/DeadlockMonitor.java new file mode 100644 index 0000000000..ba7ab7fcba --- /dev/null +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/DeadlockMonitor.java @@ -0,0 +1,122 @@ +package org.opendaylight.controller.config.manager.impl; + +import org.opendaylight.controller.config.api.ModuleIdentifier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; +import java.util.concurrent.TimeUnit; + +public class DeadlockMonitor implements AutoCloseable { + private static final Logger logger = LoggerFactory.getLogger(DeadlockMonitorRunnable.class); + + private static final long WARN_AFTER_MILLIS = 5000; + + private final TransactionIdentifier transactionIdentifier; + private final DeadlockMonitorRunnable thread; + @GuardedBy("this") + private ModuleIdentifierWithNanos moduleIdentifierWithNanos = new ModuleIdentifierWithNanos(); + + public DeadlockMonitor(TransactionIdentifier transactionIdentifier) { + this.transactionIdentifier = transactionIdentifier; + thread = new DeadlockMonitorRunnable(); + thread.start(); + } + + public synchronized void setCurrentlyInstantiatedModule(ModuleIdentifier currentlyInstantiatedModule) { + this.moduleIdentifierWithNanos = new ModuleIdentifierWithNanos(currentlyInstantiatedModule); + } + + public boolean isAlive() { + return thread.isAlive(); + } + + @Override + public void close() { + thread.interrupt(); + } + + @Override + public String toString() { + return "DeadlockMonitor{" + transactionIdentifier + '}'; + } + + private class DeadlockMonitorRunnable extends Thread { + + private DeadlockMonitorRunnable() { + super(DeadlockMonitor.this.toString()); + } + + @Override + public void run() { + ModuleIdentifierWithNanos old = new ModuleIdentifierWithNanos(); // null moduleId + while (this.isInterrupted() == false) { + ModuleIdentifierWithNanos copy = new ModuleIdentifierWithNanos(DeadlockMonitor.this.moduleIdentifierWithNanos); + if (old.moduleIdentifier == null) { + // started + old = copy; + } else if (old.moduleIdentifier != null && old.equals(copy)) { + // is the getInstance() running longer than WARN_AFTER_MILLIS ? + long runningTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - copy.nanoTime); + if (runningTime > WARN_AFTER_MILLIS) { + logger.warn("{} did not finish after {} ms", copy.moduleIdentifier, runningTime); + } + } + try { + sleep(1000); + } catch (InterruptedException e) { + interrupt(); + } + } + logger.trace("Exiting {}", this); + } + + @Override + public String toString() { + return "DeadLockMonitorRunnable{" + transactionIdentifier + "}"; + } + } + + private class ModuleIdentifierWithNanos { + @Nullable + private final ModuleIdentifier moduleIdentifier; + private final long nanoTime; + + private ModuleIdentifierWithNanos() { + moduleIdentifier = null; + nanoTime = System.nanoTime(); + } + + private ModuleIdentifierWithNanos(ModuleIdentifier moduleIdentifier) { + this.moduleIdentifier = moduleIdentifier; + nanoTime = System.nanoTime(); + } + + private ModuleIdentifierWithNanos(ModuleIdentifierWithNanos copy) { + moduleIdentifier = copy.moduleIdentifier; + nanoTime = copy.nanoTime; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + ModuleIdentifierWithNanos that = (ModuleIdentifierWithNanos) o; + + if (nanoTime != that.nanoTime) return false; + if (moduleIdentifier != null ? !moduleIdentifier.equals(that.moduleIdentifier) : that.moduleIdentifier != null) + return false; + + return true; + } + + @Override + public int hashCode() { + int result = moduleIdentifier != null ? moduleIdentifier.hashCode() : 0; + result = 31 * result + (int) (nanoTime ^ (nanoTime >>> 32)); + return result; + } + } +} diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java index 941aec1096..fd6262cb8c 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java @@ -10,6 +10,7 @@ package org.opendaylight.controller.config.manager.impl; import javax.annotation.Nullable; import org.opendaylight.controller.config.api.ModuleIdentifier; +import org.opendaylight.controller.config.manager.impl.dependencyresolver.DestroyedModule; import org.opendaylight.controller.config.manager.impl.dynamicmbean.DynamicReadableWrapper; import org.opendaylight.controller.config.manager.impl.jmx.ModuleJMXRegistrator; import org.opendaylight.controller.config.manager.impl.jmx.RootRuntimeBeanRegistratorImpl; diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverImpl.java index e3057a1179..c229450c30 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverImpl.java @@ -118,7 +118,7 @@ final class DependencyResolverImpl implements DependencyResolver, } } - // transalate from serviceref to module ON + // translate from serviceref to module ON private ObjectName translateServiceRefIfPossible(ObjectName dependentReadOnlyON) { if (ObjectNameUtil.isServiceReference(dependentReadOnlyON)) { String serviceQName = ObjectNameUtil.getServiceQName(dependentReadOnlyON); @@ -214,7 +214,7 @@ final class DependencyResolverImpl implements DependencyResolver, return maxDependencyDepth; } - public void countMaxDependencyDepth(DependencyResolverManager manager) { + void countMaxDependencyDepth(DependencyResolverManager manager) { transactionStatus.checkCommitted(); if (maxDependencyDepth == null) { maxDependencyDepth = getMaxDepth(this, manager, @@ -257,4 +257,5 @@ final class DependencyResolverImpl implements DependencyResolver, public ModuleIdentifier getIdentifier() { return name; } + } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManager.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManager.java index c115934d37..b99bf8330e 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManager.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManager.java @@ -7,45 +7,57 @@ */ package org.opendaylight.controller.config.manager.impl.dependencyresolver; +import com.google.common.reflect.AbstractInvocationHandler; +import com.google.common.reflect.Reflection; import org.opendaylight.controller.config.api.DependencyResolver; import org.opendaylight.controller.config.api.DependencyResolverFactory; import org.opendaylight.controller.config.api.JmxAttribute; import org.opendaylight.controller.config.api.ModuleIdentifier; +import org.opendaylight.controller.config.api.RuntimeBeanRegistratorAwareModule; import org.opendaylight.controller.config.api.ServiceReferenceReadableRegistry; import org.opendaylight.controller.config.manager.impl.CommitInfo; -import org.opendaylight.controller.config.manager.impl.ModuleInternalTransactionalInfo; +import org.opendaylight.controller.config.manager.impl.DeadlockMonitor; +import org.opendaylight.controller.config.manager.impl.ModuleInternalInfo; +import org.opendaylight.controller.config.manager.impl.TransactionIdentifier; import org.opendaylight.controller.config.manager.impl.TransactionStatus; +import org.opendaylight.controller.config.manager.impl.jmx.TransactionModuleJMXRegistrator.TransactionModuleJMXRegistration; import org.opendaylight.controller.config.spi.Module; import org.opendaylight.controller.config.spi.ModuleFactory; import org.opendaylight.yangtools.yang.data.impl.codec.CodecRegistry; import javax.annotation.concurrent.GuardedBy; import javax.management.InstanceAlreadyExistsException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import static com.google.common.base.Preconditions.checkState; + /** * Holds information about modules being created and destroyed within this * transaction. Observes usage of DependencyResolver within modules to figure * out dependency tree. */ -public class DependencyResolverManager implements TransactionHolder, DependencyResolverFactory { +public class DependencyResolverManager implements DependencyResolverFactory, AutoCloseable { @GuardedBy("this") private final Map moduleIdentifiersToDependencyResolverMap = new HashMap<>(); private final ModulesHolder modulesHolder; private final TransactionStatus transactionStatus; private final ServiceReferenceReadableRegistry readableRegistry; private final CodecRegistry codecRegistry; + private final DeadlockMonitor deadlockMonitor; - public DependencyResolverManager(String transactionName, + public DependencyResolverManager(TransactionIdentifier transactionIdentifier, TransactionStatus transactionStatus, ServiceReferenceReadableRegistry readableRegistry, CodecRegistry codecRegistry) { - this.modulesHolder = new ModulesHolder(transactionName); + this.modulesHolder = new ModulesHolder(transactionIdentifier); this.transactionStatus = transactionStatus; this.readableRegistry = readableRegistry; this.codecRegistry = codecRegistry; + this.deadlockMonitor = new DeadlockMonitor(transactionIdentifier); } @Override @@ -88,7 +100,6 @@ public class DependencyResolverManager implements TransactionHolder, DependencyR return result; } - @Override public ModuleInternalTransactionalInfo destroyModule( ModuleIdentifier moduleIdentifier) { transactionStatus.checkNotCommitted(); @@ -99,45 +110,85 @@ public class DependencyResolverManager implements TransactionHolder, DependencyR } // protect write access - @Override + public void put( - ModuleInternalTransactionalInfo moduleInternalTransactionalInfo) { + final ModuleIdentifier moduleIdentifier, + final Module module, + ModuleFactory moduleFactory, + ModuleInternalInfo maybeOldInternalInfo, + TransactionModuleJMXRegistration transactionModuleJMXRegistration, + boolean isDefaultBean) { transactionStatus.checkNotCommitted(); + + Class moduleClass = Module.class; + if (module instanceof RuntimeBeanRegistratorAwareModule) { + moduleClass = RuntimeBeanRegistratorAwareModule.class; + } + Module proxiedModule = Reflection.newProxy(moduleClass, new AbstractInvocationHandler() { + // optimization: subsequent calls to getInstance MUST return the same value during transaction, + // so it is safe to cache the response + private Object cachedInstance; + + @Override + protected Object handleInvocation(Object proxy, Method method, Object[] args) throws Throwable { + boolean isGetInstance = method.getName().equals("getInstance"); + if (isGetInstance) { + if (cachedInstance != null) { + return cachedInstance; + } + + checkState(deadlockMonitor.isAlive(), "Deadlock monitor is not alive"); + deadlockMonitor.setCurrentlyInstantiatedModule(moduleIdentifier); + } + try { + Object response = method.invoke(module, args); + if (isGetInstance) { + cachedInstance = response; + } + return response; + } catch(InvocationTargetException e) { + throw e.getCause(); + } finally { + if (isGetInstance) { + deadlockMonitor.setCurrentlyInstantiatedModule(null); + } + } + } + }); + + + ModuleInternalTransactionalInfo moduleInternalTransactionalInfo = new ModuleInternalTransactionalInfo( + moduleIdentifier, proxiedModule, moduleFactory, + maybeOldInternalInfo, transactionModuleJMXRegistration, isDefaultBean, module); modulesHolder.put(moduleInternalTransactionalInfo); } // wrapped methods: - @Override public CommitInfo toCommitInfo() { return modulesHolder.toCommitInfo(); } - @Override public Module findModule(ModuleIdentifier moduleIdentifier, - JmxAttribute jmxAttributeForReporting) { + JmxAttribute jmxAttributeForReporting) { return modulesHolder.findModule(moduleIdentifier, jmxAttributeForReporting); } - @Override public ModuleInternalTransactionalInfo findModuleInternalTransactionalInfo(ModuleIdentifier moduleIdentifier) { return modulesHolder.findModuleInternalTransactionalInfo(moduleIdentifier); } - @Override public ModuleFactory findModuleFactory(ModuleIdentifier moduleIdentifier, - JmxAttribute jmxAttributeForReporting) { + JmxAttribute jmxAttributeForReporting) { return modulesHolder.findModuleFactory(moduleIdentifier, jmxAttributeForReporting); } - @Override public Map getAllModules() { return modulesHolder.getAllModules(); } - @Override public void assertNotExists(ModuleIdentifier moduleIdentifier) throws InstanceAlreadyExistsException { modulesHolder.assertNotExists(moduleIdentifier); @@ -145,11 +196,16 @@ public class DependencyResolverManager implements TransactionHolder, DependencyR public List findAllByFactory(ModuleFactory factory) { List result = new ArrayList<>(); - for( ModuleInternalTransactionalInfo info : modulesHolder.getAllInfos()) { + for (ModuleInternalTransactionalInfo info : modulesHolder.getAllInfos()) { if (factory.equals(info.getModuleFactory())) { result.add(info.getIdentifier()); } } return result; } + + public void close() { + deadlockMonitor.close(); + } + } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/DestroyedModule.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DestroyedModule.java similarity index 94% rename from opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/DestroyedModule.java rename to opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DestroyedModule.java index e4652c9bb8..2aa74758d4 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/DestroyedModule.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DestroyedModule.java @@ -5,7 +5,7 @@ * 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.manager.impl; +package org.opendaylight.controller.config.manager.impl.dependencyresolver; import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.manager.impl.jmx.ModuleJMXRegistrator; @@ -31,7 +31,7 @@ public class DestroyedModule implements AutoCloseable, private final OsgiRegistration osgiRegistration; private final int orderingIdx; - DestroyedModule(ModuleIdentifier identifier, AutoCloseable instance, + public DestroyedModule(ModuleIdentifier identifier, AutoCloseable instance, ModuleJMXRegistrator oldJMXRegistrator, OsgiRegistration osgiRegistration, int orderingIdx) { this.identifier = identifier; diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java similarity index 75% rename from opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java rename to opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java index f598433433..e9f573a05d 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java @@ -5,9 +5,10 @@ * 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.manager.impl; +package org.opendaylight.controller.config.manager.impl.dependencyresolver; import org.opendaylight.controller.config.api.ModuleIdentifier; +import org.opendaylight.controller.config.manager.impl.ModuleInternalInfo; import org.opendaylight.controller.config.manager.impl.dynamicmbean.DynamicReadableWrapper; import org.opendaylight.controller.config.manager.impl.jmx.TransactionModuleJMXRegistrator.TransactionModuleJMXRegistration; import org.opendaylight.controller.config.spi.Module; @@ -18,24 +19,25 @@ import javax.annotation.Nullable; public class ModuleInternalTransactionalInfo implements Identifiable { private final ModuleIdentifier name; - private final Module module; + private final Module proxiedModule, realModule; private final ModuleFactory moduleFactory; @Nullable private final ModuleInternalInfo maybeOldInternalInfo; private final TransactionModuleJMXRegistration transactionModuleJMXRegistration; private final boolean isDefaultBean; - ModuleInternalTransactionalInfo(ModuleIdentifier name, Module module, - ModuleFactory moduleFactory, - ModuleInternalInfo maybeOldInternalInfo, - TransactionModuleJMXRegistration transactionModuleJMXRegistration, - boolean isDefaultBean) { + public ModuleInternalTransactionalInfo(ModuleIdentifier name, Module proxiedModule, + ModuleFactory moduleFactory, + ModuleInternalInfo maybeOldInternalInfo, + TransactionModuleJMXRegistration transactionModuleJMXRegistration, + boolean isDefaultBean, Module realModule) { this.name = name; - this.module = module; + this.proxiedModule = proxiedModule; this.moduleFactory = moduleFactory; this.maybeOldInternalInfo = maybeOldInternalInfo; this.transactionModuleJMXRegistration = transactionModuleJMXRegistration; this.isDefaultBean = isDefaultBean; + this.realModule = realModule; } @@ -56,8 +58,8 @@ public class ModuleInternalTransactionalInfo implements Identifiable commitMap = new HashMap<>(); @GuardedBy("this") private final Set unorderedDestroyedFromPreviousTransactions = new HashSet<>(); - ModulesHolder(String transactionName) { - this.transactionName = transactionName; + ModulesHolder(TransactionIdentifier transactionIdentifier) { + this.transactionIdentifier = transactionIdentifier; } - @Override + public CommitInfo toCommitInfo() { List orderedDestroyedFromPreviousTransactions = new ArrayList<>( unorderedDestroyedFromPreviousTransactions.size()); @@ -62,43 +61,38 @@ class ModulesHolder implements TransactionHolder { .get(moduleIdentifier); JmxAttributeValidationException.checkNotNull( moduleInternalTransactionalInfo, "Module " + moduleIdentifier - + "" + " not found in transaction " + transactionName, + + "" + " not found in transaction " + transactionIdentifier, jmxAttributeForReporting); return moduleInternalTransactionalInfo; } - @Override public Module findModule(ModuleIdentifier moduleIdentifier, JmxAttribute jmxAttributeForReporting) { return findModuleInternalTransactionalInfo(moduleIdentifier, - jmxAttributeForReporting).getModule(); + jmxAttributeForReporting).getProxiedModule(); } - @Override public ModuleFactory findModuleFactory(ModuleIdentifier moduleIdentifier, JmxAttribute jmxAttributeForReporting) { return findModuleInternalTransactionalInfo(moduleIdentifier, jmxAttributeForReporting).getModuleFactory(); } - @Override public Map getAllModules() { Map result = new HashMap<>(); for (ModuleInternalTransactionalInfo entry : commitMap.values()) { ModuleIdentifier name = entry.getIdentifier(); - result.put(name, entry.getModule()); + result.put(name, entry.getProxiedModule()); } return result; } - @Override public void put( ModuleInternalTransactionalInfo moduleInternalTransactionalInfo) { commitMap.put(moduleInternalTransactionalInfo.getIdentifier(), moduleInternalTransactionalInfo); } - @Override public ModuleInternalTransactionalInfo destroyModule( ModuleIdentifier moduleIdentifier) { ModuleInternalTransactionalInfo found = commitMap.remove(moduleIdentifier); @@ -111,7 +105,6 @@ class ModulesHolder implements TransactionHolder { return found; } - @Override public void assertNotExists(ModuleIdentifier moduleIdentifier) throws InstanceAlreadyExistsException { if (commitMap.containsKey(moduleIdentifier)) { @@ -124,7 +117,6 @@ class ModulesHolder implements TransactionHolder { return commitMap.values(); } - @Override public ModuleInternalTransactionalInfo findModuleInternalTransactionalInfo(ModuleIdentifier moduleIdentifier) { ModuleInternalTransactionalInfo found = commitMap.get(moduleIdentifier); if (found == null) { diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/TransactionHolder.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/TransactionHolder.java deleted file mode 100644 index bccd453af0..0000000000 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/TransactionHolder.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright (c) 2013 Cisco 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.manager.impl.dependencyresolver; - -import org.opendaylight.controller.config.api.JmxAttribute; -import org.opendaylight.controller.config.api.ModuleIdentifier; -import org.opendaylight.controller.config.manager.impl.CommitInfo; -import org.opendaylight.controller.config.manager.impl.ModuleInternalTransactionalInfo; -import org.opendaylight.controller.config.spi.Module; -import org.opendaylight.controller.config.spi.ModuleFactory; - -import javax.management.InstanceAlreadyExistsException; -import java.util.Map; - -interface TransactionHolder { - CommitInfo toCommitInfo(); - - Module findModule(ModuleIdentifier moduleIdentifier, - JmxAttribute jmxAttributeForReporting); - - ModuleFactory findModuleFactory(ModuleIdentifier moduleIdentifier, - JmxAttribute jmxAttributeForReporting); - - Map getAllModules(); - - void put(ModuleInternalTransactionalInfo moduleInternalTransactionalInfo); - - ModuleInternalTransactionalInfo destroyModule( - ModuleIdentifier moduleIdentifier); - - void assertNotExists(ModuleIdentifier moduleIdentifier) - throws InstanceAlreadyExistsException; - - ModuleInternalTransactionalInfo findModuleInternalTransactionalInfo(ModuleIdentifier moduleIdentifier); - -} diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java index 63a66e96eb..123e52f675 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java @@ -7,24 +7,27 @@ */ package org.opendaylight.controller.config.manager.impl.dependencyresolver; -import static org.junit.Assert.assertEquals; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.reset; - -import java.util.Arrays; -import java.util.List; - import org.junit.Before; import org.junit.Test; import org.opendaylight.controller.config.api.JmxAttribute; import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.api.ServiceReferenceReadableRegistry; import org.opendaylight.controller.config.api.jmx.ObjectNameUtil; -import org.opendaylight.controller.config.manager.impl.ModuleInternalTransactionalInfo; +import org.opendaylight.controller.config.manager.impl.ModuleInternalInfo; +import org.opendaylight.controller.config.manager.impl.TransactionIdentifier; import org.opendaylight.controller.config.manager.impl.TransactionStatus; +import org.opendaylight.controller.config.manager.impl.jmx.TransactionModuleJMXRegistrator.TransactionModuleJMXRegistration; import org.opendaylight.controller.config.spi.Module; +import org.opendaylight.controller.config.spi.ModuleFactory; + +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; public class DependencyResolverManagerTest { @@ -42,7 +45,7 @@ public class DependencyResolverManagerTest { public void setUp() { transactionStatus = mock(TransactionStatus.class); ServiceReferenceReadableRegistry mockedRegistry = mock(ServiceReferenceReadableRegistry.class); - tested = new DependencyResolverManager("txName", transactionStatus, mockedRegistry, null); + tested = new DependencyResolverManager(new TransactionIdentifier("txName"), transactionStatus, mockedRegistry, null); doNothing().when(transactionStatus).checkCommitStarted(); doNothing().when(transactionStatus).checkNotCommitted(); } @@ -87,10 +90,18 @@ public class DependencyResolverManagerTest { private static void mockGetInstance(DependencyResolverManager tested, ModuleIdentifier moduleIdentifier) { - ModuleInternalTransactionalInfo mock = mock(ModuleInternalTransactionalInfo.class); - doReturn(moduleIdentifier).when(mock).getIdentifier(); - doReturn(mockedModule()).when(mock).getModule(); - tested.put(mock); + + ModuleFactory moduleFactory = mock(ModuleFactory.class); + ModuleInternalInfo maybeOldInternalInfo = null; + TransactionModuleJMXRegistration transactionModuleJMXRegistration = null; + boolean isDefaultBean = false; + + tested.put(moduleIdentifier, + mockedModule(), + moduleFactory, + maybeOldInternalInfo, + transactionModuleJMXRegistration, + isDefaultBean); } private static Module mockedModule() { diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/parallelapsp/TestingParallelAPSPModule.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/parallelapsp/TestingParallelAPSPModule.java index f4ba5ef887..df6dce1243 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/parallelapsp/TestingParallelAPSPModule.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/parallelapsp/TestingParallelAPSPModule.java @@ -7,15 +7,7 @@ */ package org.opendaylight.controller.config.manager.testingservices.parallelapsp; -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; - -import java.io.Closeable; - -import javax.annotation.Nullable; -import javax.annotation.concurrent.NotThreadSafe; -import javax.management.ObjectName; - +import com.google.common.base.Strings; import org.opendaylight.controller.config.api.DependencyResolver; import org.opendaylight.controller.config.api.JmxAttribute; import org.opendaylight.controller.config.api.ModuleIdentifier; @@ -26,7 +18,13 @@ import org.opendaylight.controller.config.spi.Module; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Strings; +import javax.annotation.Nullable; +import javax.annotation.concurrent.NotThreadSafe; +import javax.management.ObjectName; +import java.io.Closeable; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; /** * Represents service that has dependency to thread pool. diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/SimpleConfigurationTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/SimpleConfigurationTest.java index 4e9ce009b4..28408abed2 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/SimpleConfigurationTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/SimpleConfigurationTest.java @@ -326,7 +326,7 @@ public class SimpleConfigurationTest extends AbstractConfigTest { } @Test - public void testAbort() { + public void testAbort() throws InstanceAlreadyExistsException, ValidationException { ConfigTransactionJMXClient transaction = configRegistryClient .createTransaction(); assertEquals(1, configRegistryClient.getOpenConfigs().size()); @@ -336,14 +336,14 @@ public class SimpleConfigurationTest extends AbstractConfigTest { transaction.createModule(TestingFixedThreadPoolModuleFactory.NAME, fixed1); fail(); - } catch (Exception e) { - assertTrue(e.getCause() instanceof InstanceNotFoundException); + } catch (IllegalStateException e) { + assertEquals("Configuration was aborted", e.getMessage()); } try { transaction.validateConfig(); fail(); - } catch (Exception e) { - assertTrue(e.getCause() instanceof InstanceNotFoundException); + } catch (IllegalStateException e) { + assertEquals("Configuration was aborted", e.getMessage()); } assertEquals(0, configRegistryClient.getOpenConfigs().size()); } -- 2.36.6