From: Tony Tkacik Date: Tue, 20 Jan 2015 15:05:37 +0000 (+0000) Subject: Merge topic 'archetype' X-Git-Tag: release/lithium~690 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=e003fee1268ac5cb49234d412eccd150558d85e8;hp=aeffd82f2034222f48d2425d4fb98870f88649c4 Merge topic 'archetype' * changes: Fixed missing uses of ${artifactId} in urn Fixed copyrights in opendaylight-startup archetype --- diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/DependencyResolver.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/DependencyResolver.java index 40ff7e1703..466f1ed60c 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/DependencyResolver.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/DependencyResolver.java @@ -51,6 +51,7 @@ public interface DependencyResolver extends Identifiable { T resolveInstance(Class expectedType, ObjectName objectName, JmxAttribute jmxAttribute); + /** * To be used during commit phase to resolve identity-ref config attributes. * @@ -86,4 +87,12 @@ public interface DependencyResolver extends Identifiable { */ T newMXBeanProxy(ObjectName objectName, Class interfaceClass); + /** + * Check whether a dependency will be reused or (re)created. Useful when deciding if current module could be also reused. + * + * @param objectName ObjectName ID of a dependency + * @param jmxAttribute JMXAttribute ID of a dependency + * @return true if the dependency will be reused false otherwise + */ + boolean canReuseDependency(ObjectName objectName, JmxAttribute jmxAttribute); } diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/AbstractModule.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/AbstractModule.java new file mode 100644 index 0000000000..772bacce89 --- /dev/null +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/AbstractModule.java @@ -0,0 +1,119 @@ +package org.opendaylight.controller.config.spi; + +import org.opendaylight.controller.config.api.DependencyResolver; +import org.opendaylight.controller.config.api.ModuleIdentifier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Base implementation of Module. This implementation contains base logic for Module reconfiguration with associated fields. + * @param Type of module implementation. Enables easier implementation for the {@link #isSame} method + */ +public abstract class AbstractModule> implements org.opendaylight.controller.config.spi.Module { + + private static final Logger LOG = LoggerFactory.getLogger(AbstractModule.class); + + private final M oldModule; + private final AutoCloseable oldInstance; + protected final ModuleIdentifier identifier; + private AutoCloseable instance; + protected final DependencyResolver dependencyResolver; + + /** + * Called when module is configured. + * + * @param identifier id of current instance. + * @param dependencyResolver resolver used in dependency injection and validation. + */ + public AbstractModule(ModuleIdentifier identifier, DependencyResolver dependencyResolver) { + this(identifier, dependencyResolver, null, null); + } + + /** + * Called when module is reconfigured. + * + * @param identifier id of current instance. + * @param dependencyResolver resolver used in dependency injection and validation. + * @param oldModule old instance of module that is being reconfigred(replaced) by current instance. The old instance can be examined for reuse. + * @param oldInstance old instance wrapped by the old module. This is the resource that is actually being reused if possible or closed otherwise. + */ + public AbstractModule(ModuleIdentifier identifier, DependencyResolver dependencyResolver, M oldModule, AutoCloseable oldInstance) { + this.identifier = identifier; + this.dependencyResolver = dependencyResolver; + this.oldModule = oldModule; + this.oldInstance = oldInstance; + } + + @Override + public ModuleIdentifier getIdentifier() { + return identifier; + } + + /** + * + * General algorithm for spawning/closing and reusing wrapped instances. + * + * @return current instance of wrapped resource either by reusing the old one (if present) or constructing a brand new. + */ + @Override + public final AutoCloseable getInstance() { + if(instance==null) { + if(oldInstance!=null && canReuseInstance(oldModule)) { + resolveDependencies(); + instance = reuseInstance(oldInstance); + } else { + if(oldInstance!=null) { + try { + oldInstance.close(); + } catch(Exception e) { + LOG.error("An error occurred while closing old instance {} for module {}", oldInstance, getIdentifier(), e); + } + } + resolveDependencies(); + instance = createInstance(); + if (instance == null) { + throw new IllegalStateException("Error in createInstance - null is not allowed as return value. Module: " + getIdentifier()); + } + } + } + return instance; + } + + /** + * @return Brand new instance of wrapped class in case no previous instance is present or reconfiguration is impossible. + */ + protected abstract AutoCloseable createInstance(); + + @Override + public final boolean canReuse(Module oldModule) { + // Just cast into a specific instance + // TODO unify this method with canReuseInstance (required Module interface to be generic which requires quite a lot of changes) + return getClass().isInstance(oldModule) ? canReuseInstance((M) oldModule) : false; + } + + /** + * + * Users are welcome to override this method to provide custom logic for advanced reusability detection. + * + * @param oldModule old instance of a Module + * @return true if the old instance is reusable false if a new one should be spawned + */ + protected abstract boolean canReuseInstance(final M oldModule); + + /** + * By default the oldInstance is returned since this method is by default called only if the oldModule had the same configuration and dependencies configured. + * Users are welcome to override this method to provide custom logic for advanced reusability. + * + * @param oldInstance old instance of a class wrapped by the module + * @return reused instance + */ + protected AutoCloseable reuseInstance(AutoCloseable oldInstance) { + // implement if instance reuse should be supported. Override canReuseInstance to change the criteria. + return oldInstance; + } + + /** + * Inject all the dependencies using dependency resolver instance. + */ + protected abstract void resolveDependencies(); +} 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 b557108d4d..53f03a2398 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 @@ -54,4 +54,17 @@ public interface Module extends Identifiable{ */ AutoCloseable getInstance(); + + /** + * Compare current module with oldModule and if the instance/live object + * produced by the old module can be reused in this module as well return true. + * Typically true should be returned if the old module had the same configuration. + * + * + * @param oldModule old instance of Module + * @return true if the instance produced by oldModule can be reused with current instance as well. + */ + public boolean canReuse(Module oldModule); + + } 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 96739fb822..91c67b8b02 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 @@ -430,8 +430,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe } } - // destroy all live objects one after another in order of the dependency - // hierarchy + // destroy all live objects one after another in order of the dependency hierarchy, from top to bottom List destroyedModules = currentConfig .getModulesToBeDestroyed(); for (DestroyedModule destroyedModule : destroyedModules) { 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 bb61c4acc7..37c2e2d777 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 @@ -10,6 +10,7 @@ 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; import java.util.HashSet; @@ -383,32 +384,34 @@ class ConfigTransactionControllerImpl implements LOG.trace("Committing transaction {}", getTransactionIdentifier()); - // call getInstance() - for (Entry entry : dependencyResolverManager - .getAllModules().entrySet()) { - Module module = entry.getValue(); - ModuleIdentifier name = entry.getKey(); + Map allModules = dependencyResolverManager.getAllModules(); + + // call getInstance() on all Modules from top to bottom (from source to target of the dependency relation) + // The source of a dependency closes itself and calls getInstance recursively on the dependencies (in case of reconfiguration) + // This makes close() calls from top to bottom while createInstance() calls are performed bottom to top + List sortedModuleIdentifiers = Lists.reverse(dependencyResolverManager.getSortedModuleIdentifiers()); + for (ModuleIdentifier moduleIdentifier : sortedModuleIdentifiers) { + Module module = allModules.get(moduleIdentifier); + try { LOG.debug("About to commit {} in transaction {}", - name, getTransactionIdentifier()); + moduleIdentifier, getTransactionIdentifier()); AutoCloseable instance = module.getInstance(); - checkNotNull(instance, "Instance is null:{} in transaction {}", name, getTransactionIdentifier()); + checkNotNull(instance, "Instance is null:{} in transaction {}", moduleIdentifier, getTransactionIdentifier()); } catch (Exception e) { - LOG.error("Commit failed on {} in transaction {}", name, + LOG.error("Commit failed on {} in transaction {}", moduleIdentifier, getTransactionIdentifier(), e); internalAbort(); throw new IllegalStateException( format("Error - getInstance() failed for %s in transaction %s", - name, getTransactionIdentifier()), e); + moduleIdentifier, getTransactionIdentifier()), e); } } - // count dependency order - LOG.trace("Committed configuration {}", getTransactionIdentifier()); transactionStatus.setCommitted(); - return dependencyResolverManager.getSortedModuleIdentifiers(); + return sortedModuleIdentifiers; } @Override 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 0741dab69d..024518ca98 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 @@ -9,6 +9,7 @@ package org.opendaylight.controller.config.manager.impl.dependencyresolver; import static java.lang.String.format; +import com.google.common.base.Preconditions; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Set; @@ -151,28 +152,17 @@ final class DependencyResolverImpl implements DependencyResolver, @Override public T resolveInstance(Class expectedType, ObjectName dependentReadOnlyON, JmxAttribute jmxAttribute) { - if (expectedType == null || dependentReadOnlyON == null || jmxAttribute == null) { - throw new IllegalArgumentException(format( - "Null parameters not allowed, got %s %s %s", expectedType, - dependentReadOnlyON, jmxAttribute)); - } - ObjectName translatedDependentReadOnlyON = translateServiceRefIfPossible(dependentReadOnlyON); - transactionStatus.checkCommitStarted(); - transactionStatus.checkNotCommitted(); + Module module = resolveModuleInstance(dependentReadOnlyON, jmxAttribute); - ModuleIdentifier dependentModuleIdentifier = ObjectNameUtil.fromON( - translatedDependentReadOnlyON, ObjectNameUtil.TYPE_MODULE); - Module module = modulesHolder.findModule(dependentModuleIdentifier, - jmxAttribute); synchronized (this) { - dependencies.add(dependentModuleIdentifier); + dependencies.add(module.getIdentifier()); } AutoCloseable instance = module.getInstance(); if (instance == null) { String message = format( "Error while %s resolving instance %s. getInstance() returned null. " + "Expected type %s , attribute %s", name, - dependentModuleIdentifier, expectedType, jmxAttribute + module.getIdentifier(), expectedType, jmxAttribute ); throw new JmxAttributeValidationException(message, jmxAttribute); } @@ -188,6 +178,36 @@ final class DependencyResolverImpl implements DependencyResolver, } } + private Module resolveModuleInstance(ObjectName dependentReadOnlyON, + JmxAttribute jmxAttribute) { + Preconditions.checkArgument(dependentReadOnlyON != null ,"dependentReadOnlyON"); + Preconditions.checkArgument(jmxAttribute != null, "jmxAttribute"); + ObjectName translatedDependentReadOnlyON = translateServiceRefIfPossible(dependentReadOnlyON); + transactionStatus.checkCommitStarted(); + transactionStatus.checkNotCommitted(); + + ModuleIdentifier dependentModuleIdentifier = ObjectNameUtil.fromON( + translatedDependentReadOnlyON, ObjectNameUtil.TYPE_MODULE); + + return Preconditions.checkNotNull(modulesHolder.findModule(dependentModuleIdentifier, jmxAttribute)); + } + + @Override + public boolean canReuseDependency(ObjectName objectName, JmxAttribute jmxAttribute) { + Preconditions.checkNotNull(objectName); + Preconditions.checkNotNull(jmxAttribute); + + Module currentModule = resolveModuleInstance(objectName, jmxAttribute); + ModuleIdentifier identifier = currentModule.getIdentifier(); + ModuleInternalTransactionalInfo moduleInternalTransactionalInfo = modulesHolder.findModuleInternalTransactionalInfo(identifier); + + if(moduleInternalTransactionalInfo.hasOldModule()) { + Module oldModule = moduleInternalTransactionalInfo.getOldInternalInfo().getReadableModule().getModule(); + return currentModule.canReuse(oldModule); + } + return false; + } + @Override public Class resolveIdentity(IdentityAttributeRef identityRef, Class expectedBaseClass) { final QName qName = QName.create(identityRef.getqNameOfIdentity()); @@ -217,7 +237,7 @@ final class DependencyResolverImpl implements DependencyResolver, @Override public int compareTo(DependencyResolverImpl o) { - transactionStatus.checkCommitted(); + transactionStatus.checkCommitStarted(); return Integer.compare(getMaxDependencyDepth(), o.getMaxDependencyDepth()); } @@ -232,7 +252,11 @@ final class DependencyResolverImpl implements DependencyResolver, } void countMaxDependencyDepth(DependencyResolverManager manager) { - transactionStatus.checkCommitted(); + // We can calculate the dependency after second phase commit was started + // Second phase commit starts after validation and validation adds the dependencies into the dependency resolver, which are necessary for the calculation + // FIXME generated code for abstract module declares validate method as non-final + // Overriding the validate would cause recreate every time instead of reuse + also possibly wrong close order if there is another module depending + transactionStatus.checkCommitStarted(); if (maxDependencyDepth == null) { maxDependencyDepth = getMaxDepth(this, manager, new LinkedHashSet()); 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 da6349ac53..0014a5924d 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 @@ -88,7 +88,7 @@ public class DependencyResolverManager implements DependencyResolverFactory, Aut * things? */ private List getAllSorted() { - transactionStatus.checkCommitted(); + transactionStatus.checkCommitStarted(); List sorted = new ArrayList<>( moduleIdentifiersToDependencyResolverMap.values()); for (DependencyResolverImpl dri : sorted) { diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java index 26d8c74fb3..210b6d2aa7 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java @@ -7,6 +7,7 @@ */ package org.opendaylight.controller.config.manager.impl.dependencyresolver; +import com.google.common.base.Preconditions; import javax.annotation.Nullable; import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.manager.impl.ModuleInternalInfo; @@ -70,10 +71,7 @@ public class ModuleInternalTransactionalInfo implements Identifiable sortedModuleIdentifiers = tested .getSortedModuleIdentifiers(); assertEquals( @@ -109,6 +112,7 @@ public class DependencyResolverManagerTest extends AbstractLockedPlatformMBeanSe private static Module mockedModule() { Module mockedModule = mock(Module.class); doReturn(mock(AutoCloseable.class)).when(mockedModule).getInstance(); + doReturn(new ModuleIdentifier("fact", "instance")).when(mockedModule).getIdentifier(); return 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 d25956b774..1857aaa22f 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 @@ -146,6 +146,11 @@ public class TestingParallelAPSPModule implements Module, return instance; } + @Override + public boolean canReuse(final Module oldModule) { + return false; + } + @Override public ModuleIdentifier getIdentifier() { return identifier; diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/parallelapsp/test/MockedDependenciesTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/parallelapsp/test/MockedDependenciesTest.java index 7deade5de2..95de90d3af 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/parallelapsp/test/MockedDependenciesTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/parallelapsp/test/MockedDependenciesTest.java @@ -73,6 +73,11 @@ public class MockedDependenciesTest extends AbstractParallelAPSPTest { } + @Override + public boolean canReuse(Module oldModule) { + return false; + } + @Override public Closeable getInstance() { return new MockedThreadPool(threadCount); diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/scheduledthreadpool/TestingScheduledThreadPoolModule.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/scheduledthreadpool/TestingScheduledThreadPoolModule.java index 1811e7dd94..e35509d090 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/scheduledthreadpool/TestingScheduledThreadPoolModule.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/scheduledthreadpool/TestingScheduledThreadPoolModule.java @@ -61,6 +61,11 @@ public class TestingScheduledThreadPoolModule implements Module, "Parameter 'ThreadCount' must be greater than 0"); } + @Override + public boolean canReuse(final Module oldModule) { + return false; + } + @Override public int getThreadCount() { return threadCount; diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/TestingFixedThreadPoolModule.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/TestingFixedThreadPoolModule.java index 464f786e6f..c773dd232b 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/TestingFixedThreadPoolModule.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/TestingFixedThreadPoolModule.java @@ -69,6 +69,11 @@ public class TestingFixedThreadPoolModule implements "Parameter 'threadCount' must be greater than 0"); } + @Override + public boolean canReuse(final Module oldModule) { + return isReusable() && triggerNewInstanceCreation == false; + } + @Override public Closeable getInstance() { if (instance == null) { diff --git a/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/fixed/FixedThreadPoolConfigBeanTest.java b/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/fixed/FixedThreadPoolConfigBeanTest.java index 64e83a544f..5bf4ac2d7b 100644 --- a/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/fixed/FixedThreadPoolConfigBeanTest.java +++ b/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/fixed/FixedThreadPoolConfigBeanTest.java @@ -40,6 +40,7 @@ public class FixedThreadPoolConfigBeanTest extends AbstractConfigTest { private FixedThreadPoolModuleFactory factory; private final String nameInstance = "fixedInstance"; + private ObjectName threadFactoryON; @Before public void setUp() { @@ -73,10 +74,12 @@ public class FixedThreadPoolConfigBeanTest extends AbstractConfigTest { assertBeanCount(1, factory.getImplementationName()); transaction = configRegistryClient.createTransaction(); + NamingThreadFactoryModuleMXBean namingThreadFactoryModuleMXBean = transaction.newMXBeanProxy(threadFactoryON, NamingThreadFactoryModuleMXBean.class); + namingThreadFactoryModuleMXBean.setNamePrefix("newPrefix"); CommitStatus status = transaction.commit(); assertBeanCount(1, factory.getImplementationName()); - assertStatus(status, 0, 0, 2); + assertStatus(status, 0, 2, 0); } @Test @@ -161,7 +164,7 @@ public class FixedThreadPoolConfigBeanTest extends AbstractConfigTest { FixedThreadPoolModuleMXBean mxBean = transaction.newMXBeanProxy(nameCreated, FixedThreadPoolModuleMXBean.class); mxBean.setMaxThreadCount(numberOfThreads); - ObjectName threadFactoryON = transaction.createModule(NamingThreadFactoryModuleFactory.NAME, "naming"); + threadFactoryON = transaction.createModule(NamingThreadFactoryModuleFactory.NAME, "naming"); NamingThreadFactoryModuleMXBean namingThreadFactoryModuleMXBean = transaction.newMXBeanProxy(threadFactoryON, NamingThreadFactoryModuleMXBean.class); namingThreadFactoryModuleMXBean.setNamePrefix(prefix); diff --git a/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/fixed/TestingFixedThreadPoolModule.java b/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/fixed/TestingFixedThreadPoolModule.java index d6171263af..bd57fe98f6 100644 --- a/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/fixed/TestingFixedThreadPoolModule.java +++ b/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/fixed/TestingFixedThreadPoolModule.java @@ -32,5 +32,4 @@ public class TestingFixedThreadPoolModule extends AbstractMockedModule implement doReturn(mock(ExecutorService.class)).when(pool).getExecutor(); return pool; } - } diff --git a/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/naming/TestingNamingThreadPoolFactoryModule.java b/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/naming/TestingNamingThreadPoolFactoryModule.java index 37fb839070..f91ec47ad9 100644 --- a/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/naming/TestingNamingThreadPoolFactoryModule.java +++ b/opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/naming/TestingNamingThreadPoolFactoryModule.java @@ -62,4 +62,9 @@ public class TestingNamingThreadPoolFactoryModule implements Module, ThreadFacto return fact; } + @Override + public boolean canReuse(Module oldModule) { + return false; + } + } diff --git a/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/AbstractModuleTemplate.java b/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/AbstractModuleTemplate.java index 2402014fd3..487bf63e62 100644 --- a/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/AbstractModuleTemplate.java +++ b/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/AbstractModuleTemplate.java @@ -22,13 +22,12 @@ public class AbstractModuleTemplate extends GeneralClassTemplate { private final String registratorType; public AbstractModuleTemplate(Header header, String packageName, - String abstractModuleName, List implementedIfcs, - List moduleFields, List methods, + String abstractModuleName, List extendedClasses, + List implementedIfcs, List moduleFields, List methods, boolean isRuntime, String registratorType) { - super(header, packageName, abstractModuleName, Collections - . emptyList(), implementedIfcs, Collections - . emptyList(), methods, true, false, Collections - . emptyList()); + super(header, packageName, abstractModuleName, extendedClasses, + implementedIfcs, Collections. emptyList(), methods, + true, false, Collections. emptyList()); this.moduleFields = moduleFields; this.runtime = isRuntime; this.registratorType = registratorType; diff --git a/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/GeneralClassTemplate.java b/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/GeneralClassTemplate.java index 3f4917f2ff..4adb2b0cf5 100644 --- a/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/GeneralClassTemplate.java +++ b/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/GeneralClassTemplate.java @@ -40,7 +40,7 @@ public class GeneralClassTemplate extends AbstractFtlTemplate { static List checkCardinality(List extendedClass) { if (extendedClass.size() > 1) { throw new IllegalArgumentException( - "Class cannot have more than one super " + "class"); + "Class cannot have more than one super class, found: " + extendedClass); } return extendedClass; } diff --git a/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/TemplateFactory.java b/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/TemplateFactory.java index 53ab4ef335..19e875f9b1 100644 --- a/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/TemplateFactory.java +++ b/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/TemplateFactory.java @@ -23,7 +23,7 @@ import org.opendaylight.controller.config.api.IdentityAttributeRef; import org.opendaylight.controller.config.api.RuntimeBeanRegistratorAwareModule; import org.opendaylight.controller.config.api.annotations.AbstractServiceInterface; import org.opendaylight.controller.config.api.runtime.RuntimeBean; -import org.opendaylight.controller.config.spi.Module; +import org.opendaylight.controller.config.spi.AbstractModule; import org.opendaylight.controller.config.yangjmxgenerator.AbstractEntry; import org.opendaylight.controller.config.yangjmxgenerator.ModuleMXBeanEntry; import org.opendaylight.controller.config.yangjmxgenerator.RuntimeBeanEntry; @@ -196,7 +196,6 @@ public class TemplateFactory { List moduleFields = attrProcessor.getModuleFields(); List implementedIfcs = Lists.newArrayList( - Module.class.getCanonicalName(), mbe.getFullyQualifiedName(mbe.getMXBeanInterfaceName())); for (String implementedService : mbe.getProvidedServices().keySet()) { @@ -218,9 +217,11 @@ public class TemplateFactory { .getCanonicalName()); } + List extendedClasses = Collections.singletonList(AbstractModule.class.getCanonicalName() + "<" + mbe.getAbstractModuleName() + ">"); + AbstractModuleTemplate abstractModuleTemplate = new AbstractModuleTemplate( getHeaderFromEntry(mbe), mbe.getPackageName(), - mbe.getAbstractModuleName(), implementedIfcs, moduleFields, + mbe.getAbstractModuleName(), extendedClasses, implementedIfcs, moduleFields, attrProcessor.getMethods(), generateRuntime, registratorFullyQualifiedName); diff --git a/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/gofactory/AbsModuleGeneratedObjectFactory.java b/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/gofactory/AbsModuleGeneratedObjectFactory.java index 64a13d3f80..92a0eec586 100644 --- a/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/gofactory/AbsModuleGeneratedObjectFactory.java +++ b/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/gofactory/AbsModuleGeneratedObjectFactory.java @@ -10,9 +10,13 @@ package org.opendaylight.controller.config.yangjmxgenerator.plugin.gofactory; import static com.google.common.base.Preconditions.checkState; import static java.lang.String.format; +import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.base.Optional; -import java.util.ArrayList; +import com.google.common.collect.Collections2; +import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -25,8 +29,10 @@ import org.opendaylight.controller.config.yangjmxgenerator.ModuleMXBeanEntry; import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.AbstractModuleTemplate; import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.TemplateFactory; import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.Annotation; +import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.Field; import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.IdentityRefModuleField; import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.Method; +import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.MethodDefinition; import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.ModuleField; import org.opendaylight.controller.config.yangjmxgenerator.plugin.java.FullyQualifiedName; import org.opendaylight.controller.config.yangjmxgenerator.plugin.java.GeneratedObject; @@ -39,16 +45,26 @@ import org.slf4j.LoggerFactory; public class AbsModuleGeneratedObjectFactory { + private static final Function FULLY_QUALIFIED_NAME_FUNCTION = new Function() { + @Override + public FullyQualifiedName apply(final String input) { + return FullyQualifiedName.fromString(input); + } + }; + public GeneratedObject toGeneratedObject(ModuleMXBeanEntry mbe, Optional copyright) { FullyQualifiedName abstractFQN = new FullyQualifiedName(mbe.getPackageName(), mbe.getAbstractModuleName()); Optional classJavaDoc = Optional.fromNullable(mbe.getNullableDescription()); AbstractModuleTemplate abstractModuleTemplate = TemplateFactory.abstractModuleTemplateFromMbe(mbe); Optional header = abstractModuleTemplate.getHeaderString(); - List implementedInterfaces = new ArrayList<>(); - for(String implemented: abstractModuleTemplate.getTypeDeclaration().getImplemented()) { - implementedInterfaces.add(FullyQualifiedName.fromString(implemented)); - } + List implementedInterfaces = Lists.transform(abstractModuleTemplate.getTypeDeclaration().getImplemented(), FULLY_QUALIFIED_NAME_FUNCTION); + + Optional extended = + Optional.fromNullable( + Iterables.getFirst( + Collections2.transform(abstractModuleTemplate.getTypeDeclaration().getExtended(), FULLY_QUALIFIED_NAME_FUNCTION), null)); + Optional maybeRegistratorType; if (abstractModuleTemplate.isRuntime()) { maybeRegistratorType = Optional.of(FullyQualifiedName.fromString(abstractModuleTemplate.getRegistratorType())); @@ -56,7 +72,7 @@ public class AbsModuleGeneratedObjectFactory { maybeRegistratorType = Optional.absent(); } - return toGeneratedObject(abstractFQN, copyright, header, classJavaDoc, implementedInterfaces, + return toGeneratedObject(abstractFQN, copyright, header, classJavaDoc, extended, implementedInterfaces, abstractModuleTemplate.getModuleFields(), maybeRegistratorType, abstractModuleTemplate.getMethods(), mbe.getYangModuleQName()); } @@ -65,6 +81,7 @@ public class AbsModuleGeneratedObjectFactory { Optional copyright, Optional header, Optional classJavaDoc, + Optional extended, List implementedInterfaces, List moduleFields, Optional maybeRegistratorType, @@ -84,6 +101,9 @@ public class AbsModuleGeneratedObjectFactory { for(FullyQualifiedName implemented: implementedInterfaces) { b.addImplementsFQN(implemented); } + if(extended.isPresent()) { + b.addExtendsFQN(extended.get()); + } if (classJavaDoc.isPresent()) { b.addClassAnnotation(format("@%s(value=\"%s\")", Description.class.getCanonicalName(), classJavaDoc.get())); } @@ -99,9 +119,6 @@ public class AbsModuleGeneratedObjectFactory { b.addToBody("//attributes end"); - b.addToBody(getCommonFields(abstractFQN)); - - b.addToBody(getNewConstructor(abstractFQN)); b.addToBody(getCopyFromOldConstructor(abstractFQN)); @@ -110,11 +127,12 @@ public class AbsModuleGeneratedObjectFactory { b.addToBody(getCachesOfResolvedDependencies(moduleFields)); b.addToBody(getCachesOfResolvedIdentityRefs(moduleFields)); - b.addToBody(getGetInstance(moduleFields)); + b.addToBody(getResolveDependencies(moduleFields)); b.addToBody(getReuseLogic(moduleFields, abstractFQN)); b.addToBody(getEqualsAndHashCode(abstractFQN)); b.addToBody(getMethods(methods)); + b.addToBody(getGetLogger()); return new GeneratedObjectBuilder(b.build()).toGeneratedObject(); } @@ -163,28 +181,31 @@ public class AbsModuleGeneratedObjectFactory { // loop through fields, do deep equals on each field for (ModuleField moduleField : moduleFields) { + result += format( + "if (java.util.Objects.deepEquals(%s, other.%1$s) == false) {\n"+ + "return false;\n"+ + "}\n", moduleField.getName()); + if (moduleField.isListOfDependencies()) { result += format( - "if (%1$sDependency.equals(other.%1$sDependency) == false) {\n"+ - "return false;\n"+ - "}\n"+ - "for (int idx = 0; idx < %1$sDependency.size(); idx++) {\n"+ - "if (%1$sDependency.get(idx) != other.%1$sDependency.get(idx)) {\n"+ - "return false;\n"+ - "}\n"+ - "}\n" ,moduleField.getName()); + "for (int idx = 0; idx < %1$s.size(); idx++) {\n"+ + "if (!dependencyResolver.canReuseDependency(%1$s.get(idx), %1$sJmxAttribute)) {\n"+ + "return false;\n"+ + "}\n"+ + "}\n" , moduleField.getName()); } else if (moduleField.isDependent()) { result += format( - "if (%sDependency != other.%1$sDependency) { // reference to dependency must be same\n"+ - "return false;\n"+ - "}\n",moduleField.getName()); - } else { - result += format( - "if (java.util.Objects.deepEquals(%s, other.%1$s) == false) {\n"+ - "return false;\n"+ - "}\n", moduleField.getName()); + // If a reference is null (ie optional reference) it makes no sens to call canReuse on it + // In such case we continue in the isSame method because if we have null here, the previous value was null as well + // If the previous value was not null and current is or vice verse, the deepEquals comparison would return false + "if(%1$s!= null) {\n" + + "if (!dependencyResolver.canReuseDependency(%1$s, %1$sJmxAttribute)) { // reference to dependency must be reusable as well\n" + + "return false;\n" + + "}\n" + + "}\n", moduleField.getName()); } } + result += "\n"+ "return true;\n"+ "}\n"; @@ -192,13 +213,7 @@ public class AbsModuleGeneratedObjectFactory { return result; } - private static String getGetInstance(List moduleFields) { - String result = "\n"+ - "@Override\n"+ - format("public final %s getInstance() {\n", AutoCloseable.class.getCanonicalName())+ - "if(instance==null) {\n"; - // create instance start - + private static String getResolveDependencies(final List moduleFields) { // loop through dependent fields, use dependency resolver to instantiate dependencies. Do it in loop in case field represents list of dependencies. Map resolveDependenciesMap = new HashMap<>(); for(ModuleField moduleField: moduleFields) { @@ -207,19 +222,21 @@ public class AbsModuleGeneratedObjectFactory { String osgi = moduleField.getDependency().getSie().getExportedOsgiClassName(); if (moduleField.isList()) { str = format( - "%sDependency = new java.util.ArrayList<%s>();\n"+ - "for(javax.management.ObjectName dep : %1$s) {\n"+ - "%1$sDependency.add(dependencyResolver.resolveInstance(%2$s.class, dep, %1$sJmxAttribute));\n"+ - "}\n", moduleField.getName(), osgi); + "%sDependency = new java.util.ArrayList<%s>();\n"+ + "for(javax.management.ObjectName dep : %1$s) {\n"+ + "%1$sDependency.add(dependencyResolver.resolveInstance(%2$s.class, dep, %1$sJmxAttribute));\n"+ + "}\n", moduleField.getName(), osgi); } else { str = format( - "%1$sDependency = dependencyResolver.resolveInstance(%2$s.class, %1$s, %1$sJmxAttribute);\n", - moduleField.getName(), osgi); + "%1$sDependency = dependencyResolver.resolveInstance(%2$s.class, %1$s, %1$sJmxAttribute);\n", + moduleField.getName(), osgi); } resolveDependenciesMap.put(moduleField, str); } } + String result = "\n" + + "protected final void resolveDependencies() {\n"; // wrap each field resolvation statement with if !=null when dependency is not mandatory for (Map.Entry entry : resolveDependenciesMap.entrySet()) { if (entry.getKey().getDependency().isMandatory() == false) { @@ -236,9 +253,9 @@ public class AbsModuleGeneratedObjectFactory { result += format("if (%s!=null){\n", moduleField.getName()); if (moduleField.isList()) { result += format( - "for(%s candidate : %s) {\n"+ - "candidate.injectDependencyResolver(dependencyResolver);\n"+ - "}\n", moduleField.getGenericInnerType(), moduleField.getName()); + "for(%s candidate : %s) {\n"+ + "candidate.injectDependencyResolver(dependencyResolver);\n"+ + "}\n", moduleField.getGenericInnerType(), moduleField.getName()); } else { result += format("%s.injectDependencyResolver(dependencyResolver);\n", moduleField.getName()); } @@ -256,44 +273,10 @@ public class AbsModuleGeneratedObjectFactory { result += "}\n"; } } - - // create instance end: reuse and recreate logic - result += "if(oldInstance!=null && canReuseInstance(oldModule)) {\n"+ - "instance = reuseInstance(oldInstance);\n"+ - "} else {\n"+ - "if(oldInstance!=null) {\n"+ - "try {\n"+ - "oldInstance.close();\n"+ - "} catch(Exception e) {\n"+ - "logger.error(\"An error occurred while closing old instance \" + oldInstance, e);\n"+ - "}\n"+ - "}\n"+ - "instance = createInstance();\n"+ - "if (instance == null) {\n"+ - "throw new IllegalStateException(\"Error in createInstance - null is not allowed as return value\");\n"+ - "}\n"+ - "}\n"+ - "}\n"+ - "return instance;\n"+ - "}\n"+ - format("public abstract %s createInstance();\n", AutoCloseable.class.getCanonicalName()); - + result += "}\n"; return result; } - private static String getCommonFields(FullyQualifiedName abstractFQN) { - return "\n"+ - format("private final %s oldModule;\n", abstractFQN.getTypeName())+ - format("private final %s oldInstance;\n", AutoCloseable.class.getCanonicalName())+ - format("private %s instance;\n", AutoCloseable.class.getCanonicalName())+ - format("protected final %s dependencyResolver;\n", DependencyResolver.class.getCanonicalName())+ - format("private final %s identifier;\n", ModuleIdentifier.class.getCanonicalName())+ - "@Override\n"+ - format("public %s getIdentifier() {\n", ModuleIdentifier.class.getCanonicalName())+ - "return identifier;\n"+ - "}\n"; - } - private static String getCachesOfResolvedIdentityRefs(List moduleFields) { StringBuilder result = new StringBuilder(); for (ModuleField moduleField : moduleFields) { @@ -355,7 +338,7 @@ public class AbsModuleGeneratedObjectFactory { "public void validate() {\n"; // validate each mandatory dependency for(ModuleField moduleField: moduleFields) { - if (moduleField.isDependent() && moduleField.getDependency().isMandatory()) { + if (moduleField.isDependent()) { if (moduleField.isList()) { result += "" + format("for(javax.management.ObjectName dep : %s) {\n", moduleField.getName()) + @@ -363,8 +346,14 @@ public class AbsModuleGeneratedObjectFactory { moduleField.getDependency().getSie().getFullyQualifiedName(), moduleField.getName()) + "}\n"; } else { - result += format("dependencyResolver.validateDependency(%s.class, %s, %sJmxAttribute);", + if(moduleField.getDependency().isMandatory() == false) { + result += format("if(%s != null) {\n", moduleField.getName()); + } + result += format("dependencyResolver.validateDependency(%s.class, %s, %sJmxAttribute);\n", moduleField.getDependency().getSie().getFullyQualifiedName(), moduleField.getName(), moduleField.getName()); + if(moduleField.getDependency().isMandatory() == false) { + result += "}\n"; + } } } } @@ -378,7 +367,7 @@ public class AbsModuleGeneratedObjectFactory { } private static String getLoggerDefinition(FullyQualifiedName fqn) { - return format("private static final %s logger = %s.getLogger(%s.class);", + return format("private static final %s LOGGER = %s.getLogger(%s.class);", Logger.class.getCanonicalName(), LoggerFactory.class.getCanonicalName(), fqn); } @@ -386,14 +375,9 @@ public class AbsModuleGeneratedObjectFactory { private static String getConstructorStart(FullyQualifiedName fqn, LinkedHashMap parameters, String after) { String paramString = Joiner.on(",").withKeyValueSeparator(" ").join(parameters); - String setters = ""; - for (String paramName : parameters.values()) { - setters += format("this.%s = %1$s;\n", paramName); - } return format("public %s(", fqn.getTypeName()) + paramString + ") {\n" + - setters + after + "}\n"; } @@ -402,10 +386,8 @@ public class AbsModuleGeneratedObjectFactory { LinkedHashMap parameters = new LinkedHashMap<>(); parameters.put(ModuleIdentifier.class.getCanonicalName(), "identifier"); parameters.put(DependencyResolver.class.getCanonicalName(), "dependencyResolver"); - - String setToNulls = "this.oldInstance=null;\n" + - "this.oldModule=null;\n"; - return getConstructorStart(abstractFQN, parameters, setToNulls); + String init = "super(identifier, dependencyResolver);\n"; + return getConstructorStart(abstractFQN, parameters, init); } private static String getCopyFromOldConstructor(FullyQualifiedName abstractFQN) { @@ -414,6 +396,11 @@ public class AbsModuleGeneratedObjectFactory { parameters.put(DependencyResolver.class.getCanonicalName(), "dependencyResolver"); parameters.put(abstractFQN.getTypeName(), "oldModule"); parameters.put(AutoCloseable.class.getCanonicalName(), "oldInstance"); - return getConstructorStart(abstractFQN, parameters, ""); + String init = "super(identifier, dependencyResolver, oldModule, oldInstance);\n"; + return getConstructorStart(abstractFQN, parameters, init); + } + + public String getGetLogger() { + return new MethodDefinition(Logger.class.getCanonicalName(), "getLogger", Collections.emptyList(), "return LOGGER;").toString(); } } diff --git a/opendaylight/config/yang-jmx-generator-plugin/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/JMXGeneratorTest.java b/opendaylight/config/yang-jmx-generator-plugin/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/JMXGeneratorTest.java index 8c2b5d69eb..a6cfc58c34 100644 --- a/opendaylight/config/yang-jmx-generator-plugin/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/JMXGeneratorTest.java +++ b/opendaylight/config/yang-jmx-generator-plugin/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/JMXGeneratorTest.java @@ -63,6 +63,7 @@ import org.opendaylight.controller.config.api.DynamicMBeanWithInstance; import org.opendaylight.controller.config.api.annotations.Description; import org.opendaylight.controller.config.api.annotations.RequireInterface; import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnotation; +import org.opendaylight.controller.config.spi.AbstractModule; import org.opendaylight.controller.config.spi.Module; import org.opendaylight.controller.config.spi.ModuleFactory; import org.opendaylight.controller.config.yangjmxgenerator.ConfigConstants; @@ -520,11 +521,11 @@ public class JMXGeneratorTest extends AbstractGeneratorTest { assertContains(visitor.implmts, PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX + ".threads.java.DynamicThreadPoolModuleMXBean", - Module.class.getCanonicalName(), PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX + ".threads.ScheduledThreadPoolServiceInterface", PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX + ".threads.ThreadPoolServiceInterface"); + assertContains(visitor.extnds, AbstractModule.class.getCanonicalName()); assertEquals(2, visitor.constructors.size()); Set fieldDeclarations = visitor.fieldDeclarations; assertDeclaredField(fieldDeclarations, @@ -538,7 +539,7 @@ public class JMXGeneratorTest extends AbstractGeneratorTest { assertDeclaredField(fieldDeclarations, "private java.lang.Long coreSize"); assertDeclaredField(fieldDeclarations, "private byte[] binary"); - assertEquals(22, fieldDeclarations.size()); + assertEquals(17, fieldDeclarations.size()); assertEquals(1, visitor.requireIfc.size()); String reqIfc = visitor.requireIfc.get("setThreadfactory"); @@ -546,7 +547,7 @@ public class JMXGeneratorTest extends AbstractGeneratorTest { assertContains(reqIfc, PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX + ".threads.ThreadFactoryServiceInterface"); - assertEquals("Incorrenct number of generated methods", 27, + assertEquals("Incorrenct number of generated methods", 26, visitor.methods.size()); assertEquals("Incorrenct number of generated method descriptions", 3, visitor.methodDescriptions.size()); diff --git a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java index ed727c9a13..0199e8cd17 100644 --- a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java +++ b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java @@ -11,7 +11,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static java.lang.String.format; import static org.opendaylight.controller.config.yangjmxgenerator.ConfigConstants.createConfigQName; - import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Optional; @@ -250,15 +249,10 @@ final class ModuleMXBeanEntryBuilder { String moduleLocalNameFromXPath = matcher.group(1); IdentitySchemaNode moduleIdentity = moduleIdentities.get(moduleLocalNameFromXPath); unaugmentedModuleIdentities.remove(moduleLocalNameFromXPath); - checkState(moduleIdentity != null, "Cannot find identity " + moduleLocalNameFromXPath - + " matching augmentation " + augmentation); + checkState(moduleIdentity != null, "Cannot find identity %s matching augmentation %s", moduleLocalNameFromXPath, augmentation); Map providedServices = findProvidedServices(moduleIdentity, currentModule, qNamesToSIEs, schemaContext); - if (moduleIdentity == null) { - throw new IllegalStateException("Cannot find identity specified by augmentation xpath constraint: " - + moduleLocalNameFromXPath + " of " + augmentation); - } String javaNamePrefix = TypeProviderWrapper.findJavaNamePrefix(moduleIdentity); Map yangToAttributes = null; @@ -346,7 +340,7 @@ final class ModuleMXBeanEntryBuilder { } } - private void checkUniqueAttributesWithGeneratedClass(final Map uniqueGeneratedClassNames, + private static void checkUniqueAttributesWithGeneratedClass(final Map uniqueGeneratedClassNames, final QName parentQName, final Map yangToAttributes) { for (Map.Entry attr : yangToAttributes.entrySet()) { if (attr.getValue() instanceof TOAttribute) { @@ -359,7 +353,7 @@ final class ModuleMXBeanEntryBuilder { } } - private void checkUniqueTOAttr(final Map uniqueGeneratedClassNames, final QName parentQName, final TOAttribute attr) { + private static void checkUniqueTOAttr(final Map uniqueGeneratedClassNames, final QName parentQName, final TOAttribute attr) { final String upperCaseCamelCase = attr.getUpperCaseCammelCase(); if (uniqueGeneratedClassNames.containsKey(upperCaseCamelCase)) { QName firstDefinedQName = uniqueGeneratedClassNames.get(upperCaseCamelCase); diff --git a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntry.java b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntry.java index 74981a9582..34e3c2e071 100644 --- a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntry.java +++ b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntry.java @@ -9,7 +9,6 @@ package org.opendaylight.controller.config.yangjmxgenerator; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; - import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Optional; @@ -95,7 +94,7 @@ public class RuntimeBeanEntry { final List children, final Set rpcs) { checkArgument(isRoot == false || keyYangName.isPresent() == false, - "Root RuntimeBeanEntry must not have key " + "set"); + "Root RuntimeBeanEntry must not have key set"); this.packageName = packageName; this.isRoot = isRoot; this.yangName = yangName; @@ -108,16 +107,14 @@ public class RuntimeBeanEntry { for (AttributeIfc a : attributes) { checkState(map.containsKey(a.getAttributeYangName()) == false, - "Attribute already defined: " + a.getAttributeYangName() - + " in " + nodeForReporting); + "Attribute already defined: %s in %s", a.getAttributeYangName(), nodeForReporting); map.put(a.getAttributeYangName(), a); } if (keyYangName.isPresent()) { AttributeIfc keyJavaName = map.get(keyYangName.get()); - checkArgument(keyJavaName != null, "Key " + keyYangName.get() - + " not found in attribute " + "list " + attributes - + " in " + nodeForReporting); + checkArgument(keyJavaName != null, "Key %s not found in attribute list %s in %s", keyYangName.get(), + attributes, nodeForReporting); this.keyJavaName = Optional .of(keyJavaName.getUpperCaseCammelCase()); } else { diff --git a/opendaylight/config/yang-jmx-generator/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryTest.java b/opendaylight/config/yang-jmx-generator/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryTest.java index 17d4d9a524..e116f480c5 100644 --- a/opendaylight/config/yang-jmx-generator/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryTest.java +++ b/opendaylight/config/yang-jmx-generator/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryTest.java @@ -8,6 +8,7 @@ package org.opendaylight.controller.config.yangjmxgenerator; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.isA; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -16,7 +17,6 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; - import com.google.common.collect.Sets; import java.net.URI; import java.net.URISyntaxException; @@ -113,7 +113,7 @@ public class ModuleMXBeanEntryTest extends AbstractYangTest { is("threadFactory")); assertThat(threadFactoryAttribute.getUpperCaseCammelCase(), is("ThreadFactory")); - assertThat(threadFactoryAttribute.getOpenType(), is(SimpleType.class)); + assertThat(threadFactoryAttribute.getOpenType(), isA(SimpleType.class)); assertNull(threadFactoryAttribute.getNullableDefault()); assertNull(threadFactoryAttribute.getNullableDescription()); assertThat(threadFactoryAttribute.getType().getName(), is("ObjectName")); @@ -261,7 +261,7 @@ public class ModuleMXBeanEntryTest extends AbstractYangTest { assertThat(toAttr.getAttributeYangName(), is("peer")); assertThat(toAttr.getLowerCaseCammelCase(), is("peer")); assertThat(toAttr.getUpperCaseCammelCase(), is("Peer")); - assertThat(toAttr.getOpenType(), is(CompositeType.class)); + assertThat(toAttr.getOpenType(), isA(CompositeType.class)); Set propsExpected = new HashSet(2); propsExpected.add("port"); propsExpected.add("core-size"); @@ -296,7 +296,7 @@ public class ModuleMXBeanEntryTest extends AbstractYangTest { is("innerStreamList")); assertThat(innerStream.getUpperCaseCammelCase(), is("InnerStreamList")); - assertThat(innerStream.getOpenType(), is(ArrayType.class)); + assertThat(innerStream.getOpenType(), isA(ArrayType.class)); } diff --git a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/.gitignore b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/.gitignore index 27d1535693..afd1e9d6dd 100644 --- a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/.gitignore +++ b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/.gitignore @@ -1 +1,2 @@ *.java +!CheckedAutoCloseable.java diff --git a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/CheckedAutoCloseable.java b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/CheckedAutoCloseable.java new file mode 100644 index 0000000000..74cf617d3f --- /dev/null +++ b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/CheckedAutoCloseable.java @@ -0,0 +1,17 @@ +package org.opendaylight.controller.config.yang.test.impl; + +import com.google.common.base.Preconditions; + +public class CheckedAutoCloseable implements AutoCloseable { + private boolean closed = false; + + @Override + public synchronized void close() throws Exception { + Preconditions.checkState(closed == false); + this.closed = true; + } + + public synchronized boolean isClosed() { + return this.closed; + } +} diff --git a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/DepTestImplModuleStub.txt b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/DepTestImplModuleStub.txt index 80c1e54a15..2359b50e0a 100644 --- a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/DepTestImplModuleStub.txt +++ b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/DepTestImplModuleStub.txt @@ -1,5 +1 @@ - return new AutoCloseable() { - @Override - public void close() throws Exception { - } - }; + return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable(); diff --git a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/IdentityTestModuleStub.txt b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/IdentityTestModuleStub.txt index 7fa0c10eb2..458ec01165 100644 --- a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/IdentityTestModuleStub.txt +++ b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/IdentityTestModuleStub.txt @@ -9,8 +9,4 @@ } logger.info("IdentityContainer Afi class: {}", getIdentitiesContainer().resolveAfi()); - return new AutoCloseable() { - @Override - public void close() throws Exception { - } - }; + return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable(); diff --git a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/MultipleDependenciesModuleStub.txt b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/MultipleDependenciesModuleStub.txt index 80c1e54a15..bbf2a71faa 100644 --- a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/MultipleDependenciesModuleStub.txt +++ b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/MultipleDependenciesModuleStub.txt @@ -1,5 +1,16 @@ - return new AutoCloseable() { + return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable() { @Override - public void close() throws Exception { + public synchronized void close() throws Exception { + if(getSingleDependency() != null && getSingleDependency().isClosed() == true) { + // Simulate a cleanup on dependencies that should not be closed yet + throw new java.lang.Error("Dependency was closed first"); + } + for (org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable autoCloseable : getTestingDepsDependency()) { + if(autoCloseable.isClosed() == true) { + // Simulate a cleanup on dependencies that should not be closed yet + throw new java.lang.Error("Dependency was closed first"); + } + } + super.close(); } - }; + }; \ No newline at end of file diff --git a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/NetconfTestImplModuleStub.txt b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/NetconfTestImplModuleStub.txt index 4b5b78f70a..7e155e05c7 100644 --- a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/NetconfTestImplModuleStub.txt +++ b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/NetconfTestImplModuleStub.txt @@ -1 +1,7 @@ - return org.opendaylight.controller.config.yang.test.util.NetconfTestImplModuleUtil.registerRuntimeBeans(this); + final AutoCloseable runtime = org.opendaylight.controller.config.yang.test.util.NetconfTestImplModuleUtil.registerRuntimeBeans(this); + return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable() { + @Override + public synchronized void close() throws Exception { + runtime.close(); + } + }; \ No newline at end of file diff --git a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/TestImplModuleStub.txt b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/TestImplModuleStub.txt index 80c1e54a15..dc933b4f3b 100644 --- a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/TestImplModuleStub.txt +++ b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/TestImplModuleStub.txt @@ -1,5 +1 @@ - return new AutoCloseable() { - @Override - public void close() throws Exception { - } - }; + return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable(); \ No newline at end of file diff --git a/opendaylight/config/yang-test/src/main/yang/config-test-impl.yang b/opendaylight/config/yang-test/src/main/yang/config-test-impl.yang index 093d7b3f13..f772a26898 100644 --- a/opendaylight/config/yang-test/src/main/yang/config-test-impl.yang +++ b/opendaylight/config/yang-test/src/main/yang/config-test-impl.yang @@ -468,7 +468,9 @@ module config-test-impl { } } } + } + } } @@ -485,6 +487,19 @@ module config-test-impl { } } } + container single { + uses config:service-ref { + refine type { + mandatory false; + config:required-identity test:testing; + } + } + } + + leaf simple { + type boolean; + default false; + } } } } diff --git a/opendaylight/config/yang-test/src/main/yang/config-test.yang b/opendaylight/config/yang-test/src/main/yang/config-test.yang index 2daf4055db..4843ebc895 100644 --- a/opendaylight/config/yang-test/src/main/yang/config-test.yang +++ b/opendaylight/config/yang-test/src/main/yang/config-test.yang @@ -18,6 +18,6 @@ module config-test { "Test api"; base "config:service-type"; - config:java-class "java.lang.AutoCloseable"; + config:java-class "org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable"; } } diff --git a/opendaylight/config/yang-test/src/test/java/org/opendaylight/controller/config/yang/test/impl/MultipleDependenciesModuleTest.java b/opendaylight/config/yang-test/src/test/java/org/opendaylight/controller/config/yang/test/impl/MultipleDependenciesModuleTest.java index 2df15d0dc1..997e4e3196 100644 --- a/opendaylight/config/yang-test/src/test/java/org/opendaylight/controller/config/yang/test/impl/MultipleDependenciesModuleTest.java +++ b/opendaylight/config/yang-test/src/test/java/org/opendaylight/controller/config/yang/test/impl/MultipleDependenciesModuleTest.java @@ -11,13 +11,17 @@ package org.opendaylight.controller.config.yang.test.impl; import static java.util.Arrays.asList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; import static org.opendaylight.controller.config.api.jmx.ObjectNameUtil.getInstanceName; import static org.opendaylight.controller.config.api.jmx.ObjectNameUtil.getTransactionName; import java.util.List; import javax.management.ObjectName; +import org.hamcrest.CoreMatchers; import org.junit.Before; import org.junit.Test; +import org.opendaylight.controller.config.api.ValidationException; import org.opendaylight.controller.config.manager.impl.AbstractConfigTest; import org.opendaylight.controller.config.manager.impl.factoriesresolver.HardcodedModuleFactoriesResolver; import org.opendaylight.controller.config.util.ConfigTransactionJMXClient; @@ -49,4 +53,53 @@ public class MultipleDependenciesModuleTest extends AbstractConfigTest { assertNull(getTransactionName(d1WithoutTxName)); transaction.commit(); } + + @Test + public void testCloseOrdering() throws Exception { + // Tests whether close is called in correct order on the module instances on following graph + // Each module tests whether its dependencies were closed before it (to simulate resource clean up failure) + // R1 + // | \ + // M1 M2 + // | + // L1 + ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction(); + ObjectName r1 = transaction.createModule(factory.getImplementationName(), "root1"); + ObjectName m1 = transaction.createModule(factory.getImplementationName(), "middle1"); + ObjectName m2 = transaction.createModule(factory.getImplementationName(), "middle2"); + ObjectName l1 = transaction.createModule(factory.getImplementationName(), "leaf1"); + + MultipleDependenciesModuleMXBean r1Proxy = transaction.newMXBeanProxy(r1, MultipleDependenciesModuleMXBean.class); + MultipleDependenciesModuleMXBean i1Proxy = transaction.newMXBeanProxy(m1, MultipleDependenciesModuleMXBean.class); + r1Proxy.setSingle(m1); + i1Proxy.setSingle(l1); + r1Proxy.setTestingDeps(asList(m2)); + transaction.commit(); + + configRegistryClient.createTransaction().commit(); + transaction = configRegistryClient.createTransaction(); + MultipleDependenciesModuleMXBean l1Proxy = transaction.newMXBeanProxy(l1, MultipleDependenciesModuleMXBean.class); + l1Proxy.setSimple(true); + transaction.commit(); + } + + @Test + public void testDestroyModuleDependency() throws Exception { + ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction(); + ObjectName r1 = transaction.createModule(factory.getImplementationName(), "root1"); + ObjectName m1 = transaction.createModule(factory.getImplementationName(), "middle1"); + + MultipleDependenciesModuleMXBean r1Proxy = transaction.newMXBeanProxy(r1, MultipleDependenciesModuleMXBean.class); + r1Proxy.setSingle(m1); + transaction.commit(); + + transaction = configRegistryClient.createTransaction(); + transaction.destroyModule(factory.getImplementationName(), "middle1"); + try { + transaction.commit(); + fail("Validation exception expected"); + } catch (ValidationException e) { + assertThat(e.getFailedValidations().keySet(), CoreMatchers.hasItem("multiple-dependencies")); + } + } } diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java index 3b84692077..132e09d6de 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java @@ -113,6 +113,8 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { private int currentRecoveryBatchCount; + + public RaftActor(String id, Map peerAddresses) { this(id, peerAddresses, Optional.absent()); } @@ -377,15 +379,18 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { if (oldBehavior != currentBehavior){ onStateChanged(); } - if (oldBehavior != null) { - // it can happen that the state has not changed but the leader has changed. - onLeaderChanged(oldBehavior.getLeaderId(), currentBehavior.getLeaderId()); - if (getRoleChangeNotifier().isPresent() && oldBehavior.state() != currentBehavior.state()) { - // we do not want to notify when the behavior/role is set for the first time (i.e follower) - getRoleChangeNotifier().get().tell(new RoleChanged(getId(), oldBehavior.state().name(), - currentBehavior.state().name()), getSelf()); - } + String oldBehaviorLeaderId = oldBehavior == null? null : oldBehavior.getLeaderId(); + String oldBehaviorState = oldBehavior == null? null : oldBehavior.state().name(); + + // it can happen that the state has not changed but the leader has changed. + onLeaderChanged(oldBehaviorLeaderId, currentBehavior.getLeaderId()); + + if (getRoleChangeNotifier().isPresent() && + (oldBehavior == null || (oldBehavior.state() != currentBehavior.state()))) { + getRoleChangeNotifier().get().tell( + new RoleChanged(getId(), oldBehaviorState , currentBehavior.state().name()), + getSelf()); } } @@ -397,8 +402,8 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { * @param identifier * @param data */ - protected void persistData(ActorRef clientActor, String identifier, - Payload data) { + protected void persistData(final ActorRef clientActor, final String identifier, + final Payload data) { ReplicatedLogEntry replicatedLogEntry = new ReplicatedLogImplEntry( context.getReplicatedLog().lastIndex() + 1, @@ -408,9 +413,42 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { LOG.debug("Persist data {}", replicatedLogEntry); } + final RaftActorContext raftContext = getRaftActorContext(); + replicatedLog - .appendAndPersist(clientActor, identifier, replicatedLogEntry); - } + .appendAndPersist(replicatedLogEntry, new Procedure() { + @Override + public void apply(ReplicatedLogEntry replicatedLogEntry) throws Exception { + if(!hasFollowers()){ + // Increment the Commit Index and the Last Applied values + raftContext.setCommitIndex(replicatedLogEntry.getIndex()); + raftContext.setLastApplied(replicatedLogEntry.getIndex()); + + // Apply the state immediately + applyState(clientActor, identifier, data); + + // Send a ApplyLogEntries message so that we write the fact that we applied + // the state to durable storage + self().tell(new ApplyLogEntries((int) replicatedLogEntry.getIndex()), self()); + + // Check if the "real" snapshot capture has been initiated. If no then do the fake snapshot + if(!hasSnapshotCaptureInitiated){ + raftContext.getReplicatedLog().snapshotPreCommit(raftContext.getLastApplied(), + raftContext.getTermInformation().getCurrentTerm()); + raftContext.getReplicatedLog().snapshotCommit(); + } else { + LOG.debug("Skipping fake snapshotting for {} because real snapshotting is in progress", getId()); + } + } else if (clientActor != null) { + // Send message for replication + currentBehavior.handleMessage(getSelf(), + new Replicate(clientActor, identifier, + replicatedLogEntry) + ); + } + + } + }); } protected String getId() { return context.getId(); @@ -650,8 +688,15 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { hasSnapshotCaptureInitiated = false; } + protected boolean hasFollowers(){ + return getRaftActorContext().getPeerAddresses().keySet().size() > 0; + } + private class ReplicatedLogImpl extends AbstractReplicatedLogImpl { + private static final int DATA_SIZE_DIVIDER = 5; + private long dataSizeSinceLastSnapshot = 0; + public ReplicatedLogImpl(Snapshot snapshot) { super(snapshot.getLastAppliedIndex(), snapshot.getLastAppliedTerm(), snapshot.getUnAppliedEntries()); @@ -686,7 +731,7 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { @Override public void appendAndPersist( final ReplicatedLogEntry replicatedLogEntry) { - appendAndPersist(null, null, replicatedLogEntry); + appendAndPersist(replicatedLogEntry, null); } @Override @@ -694,9 +739,9 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { return dataSize; } - public void appendAndPersist(final ActorRef clientActor, - final String identifier, - final ReplicatedLogEntry replicatedLogEntry) { + public void appendAndPersist( + final ReplicatedLogEntry replicatedLogEntry, + final Procedure callback) { if(LOG.isDebugEnabled()) { LOG.debug("Append log entry and persist {} ", replicatedLogEntry); @@ -714,22 +759,48 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { new Procedure() { @Override public void apply(ReplicatedLogEntry evt) throws Exception { - dataSize += replicatedLogEntry.size(); + int logEntrySize = replicatedLogEntry.size(); + + dataSize += logEntrySize; + long dataSizeForCheck = dataSize; + + dataSizeSinceLastSnapshot += logEntrySize; + long journalSize = lastIndex()+1; + + if(!hasFollowers()) { + // When we do not have followers we do not maintain an in-memory log + // due to this the journalSize will never become anything close to the + // snapshot batch count. In fact will mostly be 1. + // Similarly since the journal's dataSize depends on the entries in the + // journal the journal's dataSize will never reach a value close to the + // memory threshold. + // By maintaining the dataSize outside the journal we are tracking essentially + // what we have written to the disk however since we no longer are in + // need of doing a snapshot just for the sake of freeing up memory we adjust + // the real size of data by the DATA_SIZE_DIVIDER so that we do not snapshot as often + // as if we were maintaining a real snapshot + dataSizeForCheck = dataSizeSinceLastSnapshot / DATA_SIZE_DIVIDER; + } long dataThreshold = Runtime.getRuntime().totalMemory() * getRaftActorContext().getConfigParams().getSnapshotDataThresholdPercentage() / 100; // when a snaphsot is being taken, captureSnapshot != null if (hasSnapshotCaptureInitiated == false && - ( journal.size() % context.getConfigParams().getSnapshotBatchCount() == 0 || - dataSize > dataThreshold)) { + ( journalSize % context.getConfigParams().getSnapshotBatchCount() == 0 || + dataSizeForCheck > dataThreshold)) { + + dataSizeSinceLastSnapshot = 0; LOG.info("Initiating Snapshot Capture.."); long lastAppliedIndex = -1; long lastAppliedTerm = -1; ReplicatedLogEntry lastAppliedEntry = get(context.getLastApplied()); - if (lastAppliedEntry != null) { + if (!hasFollowers()) { + lastAppliedIndex = replicatedLogEntry.getIndex(); + lastAppliedTerm = replicatedLogEntry.getTerm(); + } else if (lastAppliedEntry != null) { lastAppliedIndex = lastAppliedEntry.getIndex(); lastAppliedTerm = lastAppliedEntry.getTerm(); } @@ -748,12 +819,8 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { null); hasSnapshotCaptureInitiated = true; } - // Send message for replication - if (clientActor != null) { - currentBehavior.handleMessage(getSelf(), - new Replicate(clientActor, identifier, - replicatedLogEntry) - ); + if(callback != null){ + callback.apply(replicatedLogEntry); } } } diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorTest.java index c833a86e9b..d999bb2ba1 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorTest.java +++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorTest.java @@ -883,24 +883,27 @@ public class RaftActorTest extends AbstractActorTest { TestActorRef mockActorRef = TestActorRef.create(getSystem(), MockRaftActor.props(id, Collections.emptyMap(), Optional.of(config), notifierActor), id); - MockRaftActor mockRaftActor = mockActorRef.underlyingActor(); - mockRaftActor.setCurrentBehavior(new Follower(mockRaftActor.getRaftActorContext())); - // sleeping for a minimum of 2 seconds, if it spans more its fine. Uninterruptibles.sleepUninterruptibly(2, TimeUnit.SECONDS); List matches = MessageCollectorActor.getAllMatching(notifierActor, RoleChanged.class); assertNotNull(matches); - assertEquals(2, matches.size()); + assertEquals(3, matches.size()); - // check if the notifier got a role change from Follower to Candidate + // check if the notifier got a role change from null to Follower RoleChanged raftRoleChanged = (RoleChanged) matches.get(0); assertEquals(id, raftRoleChanged.getMemberId()); + assertNull(raftRoleChanged.getOldRole()); + assertEquals(RaftState.Follower.name(), raftRoleChanged.getNewRole()); + + // check if the notifier got a role change from Follower to Candidate + raftRoleChanged = (RoleChanged) matches.get(1); + assertEquals(id, raftRoleChanged.getMemberId()); assertEquals(RaftState.Follower.name(), raftRoleChanged.getOldRole()); assertEquals(RaftState.Candidate.name(), raftRoleChanged.getNewRole()); // check if the notifier got a role change from Candidate to Leader - raftRoleChanged = (RoleChanged) matches.get(1); + raftRoleChanged = (RoleChanged) matches.get(2); assertEquals(id, raftRoleChanged.getMemberId()); assertEquals(RaftState.Candidate.name(), raftRoleChanged.getOldRole()); assertEquals(RaftState.Leader.name(), raftRoleChanged.getNewRole()); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java index 7d6dde9c8a..7ef6e040a9 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java @@ -133,7 +133,7 @@ public class Shard extends RaftActor { private Cancellable txCommitTimeoutCheckSchedule; - private Optional roleChangeNotifier; + private final Optional roleChangeNotifier; /** * Coordinates persistence recovery on startup. @@ -321,8 +321,14 @@ public class Shard extends RaftActor { // currently uses a same thread executor anyway. cohortEntry.getCohort().preCommit().get(); - Shard.this.persistData(getSender(), transactionID, - new CompositeModificationByteStringPayload(cohortEntry.getModification().toSerializable())); + // If we do not have any followers and we are not using persistence we can + // apply modification to the state immediately + if(!hasFollowers() && !persistence().isRecoveryApplicable()){ + applyModificationToState(getSender(), transactionID, cohortEntry.getModification()); + } else { + Shard.this.persistData(getSender(), transactionID, + new CompositeModificationByteStringPayload(cohortEntry.getModification().toSerializable())); + } } catch (InterruptedException | ExecutionException e) { LOG.error(e, "An exception occurred while preCommitting transaction {}", cohortEntry.getTransactionID()); @@ -422,7 +428,7 @@ public class Shard extends RaftActor { doAbortTransaction(abort.getTransactionID(), getSender()); } - private void doAbortTransaction(final String transactionID, final ActorRef sender) { + void doAbortTransaction(final String transactionID, final ActorRef sender) { final CohortEntry cohortEntry = commitCoordinator.getCohortEntryIfCurrent(transactionID); if(cohortEntry != null) { LOG.debug("Aborting transaction {}", transactionID); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardManagerTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardManagerTest.java index e70d79c61a..8c56efd413 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardManagerTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardManagerTest.java @@ -110,8 +110,6 @@ public class ShardManagerTest extends AbstractActorTest { new JavaTestKit(getSystem()) {{ final ActorRef shardManager = getSystem().actorOf(newShardMgrProps()); - shardManager.tell(new UpdateSchemaContext(TestModel.createTestContext()), getRef()); - shardManager.tell(new FindPrimary(Shard.DEFAULT_NAME, false).toSerializable(), getRef()); expectMsgClass(duration("5 seconds"), ActorNotInitialized.class); @@ -174,8 +172,6 @@ public class ShardManagerTest extends AbstractActorTest { new JavaTestKit(getSystem()) {{ final ActorRef shardManager = getSystem().actorOf(newShardMgrProps()); - shardManager.tell(new UpdateSchemaContext(TestModel.createTestContext()), getRef()); - shardManager.tell(new FindLocalShard(Shard.DEFAULT_NAME, false), getRef()); expectMsgClass(duration("5 seconds"), ActorNotInitialized.class); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java index 2792342ab2..ed842b2021 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java @@ -43,6 +43,7 @@ import org.junit.Test; import org.mockito.InOrder; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import org.opendaylight.controller.cluster.datastore.DatastoreContext.Builder; import org.opendaylight.controller.cluster.datastore.identifiers.ShardIdentifier; import org.opendaylight.controller.cluster.datastore.messages.AbortTransaction; import org.opendaylight.controller.cluster.datastore.messages.AbortTransactionReply; @@ -111,12 +112,13 @@ public class ShardTest extends AbstractActorTest { private final ShardIdentifier shardID = ShardIdentifier.builder().memberName("member-1") .shardName("inventory").type("config" + NEXT_SHARD_NUM.getAndIncrement()).build(); - private DatastoreContext dataStoreContext = DatastoreContext.newBuilder(). + private final Builder dataStoreContextBuilder = DatastoreContext.newBuilder(). shardJournalRecoveryLogBatchSize(3).shardSnapshotBatchCount(5000). - shardHeartbeatIntervalInMillis(100).build(); + shardHeartbeatIntervalInMillis(100); @Before public void setUp() { + Builder newBuilder = DatastoreContext.newBuilder(); InMemorySnapshotStore.clear(); InMemoryJournal.clear(); } @@ -127,9 +129,13 @@ public class ShardTest extends AbstractActorTest { InMemoryJournal.clear(); } + private DatastoreContext newDatastoreContext() { + return dataStoreContextBuilder.build(); + } + private Props newShardProps() { return Shard.props(shardID, Collections.emptyMap(), - dataStoreContext, SCHEMA_CONTEXT); + newDatastoreContext(), SCHEMA_CONTEXT); } @Test @@ -186,7 +192,7 @@ public class ShardTest extends AbstractActorTest { @Override public Shard create() throws Exception { return new Shard(shardID, Collections.emptyMap(), - dataStoreContext, SCHEMA_CONTEXT) { + newDatastoreContext(), SCHEMA_CONTEXT) { @Override public void onReceiveCommand(final Object message) throws Exception { if(message instanceof ElectionTimeout && firstElectionTimeout) { @@ -314,7 +320,7 @@ public class ShardTest extends AbstractActorTest { class TestShard extends Shard { TestShard() { super(shardID, Collections.singletonMap(shardID, null), - dataStoreContext, SCHEMA_CONTEXT); + newDatastoreContext(), SCHEMA_CONTEXT); } Map getPeerAddresses() { @@ -470,7 +476,7 @@ public class ShardTest extends AbstractActorTest { @Override public Shard create() throws Exception { return new Shard(shardID, Collections.emptyMap(), - dataStoreContext, SCHEMA_CONTEXT) { + newDatastoreContext(), SCHEMA_CONTEXT) { @Override protected void onRecoveryComplete() { try { @@ -774,15 +780,72 @@ public class ShardTest extends AbstractActorTest { assertTrue("Missing leaf " + TestModel.ID_QNAME.getLocalName(), idLeaf.isPresent()); assertEquals(TestModel.ID_QNAME.getLocalName() + " value", 1, idLeaf.get().getValue()); - for(int i = 0; i < 20 * 5; i++) { - long lastLogIndex = shard.underlyingActor().getShardMBean().getLastLogIndex(); - if(lastLogIndex == 2) { - break; - } - Uninterruptibles.sleepUninterruptibly(50, TimeUnit.MILLISECONDS); + verifyLastLogIndex(shard, 2); + + shard.tell(PoisonPill.getInstance(), ActorRef.noSender()); + }}; + } + + private void verifyLastLogIndex(TestActorRef shard, long expectedValue) { + for(int i = 0; i < 20 * 5; i++) { + long lastLogIndex = shard.underlyingActor().getShardMBean().getLastLogIndex(); + if(lastLogIndex == expectedValue) { + break; } + Uninterruptibles.sleepUninterruptibly(50, TimeUnit.MILLISECONDS); + } + + assertEquals("Last log index", expectedValue, shard.underlyingActor().getShardMBean().getLastLogIndex()); + } + + @Test + public void testCommitWithPersistenceDisabled() throws Throwable { + dataStoreContextBuilder.persistent(false); + new ShardTestKit(getSystem()) {{ + final TestActorRef shard = TestActorRef.create(getSystem(), + newShardProps().withDispatcher(Dispatchers.DefaultDispatcherId()), + "testCommitPhaseFailure"); - assertEquals("Last log index", 2, shard.underlyingActor().getShardMBean().getLastLogIndex()); + waitUntilLeader(shard); + + InMemoryDOMDataStore dataStore = shard.underlyingActor().getDataStore(); + + // Setup a simulated transactions with a mock cohort. + + String transactionID = "tx"; + MutableCompositeModification modification = new MutableCompositeModification(); + NormalizedNode containerNode = ImmutableNodes.containerNode(TestModel.TEST_QNAME); + DOMStoreThreePhaseCommitCohort cohort = setupMockWriteTransaction("cohort", dataStore, + TestModel.TEST_PATH, containerNode, modification); + + FiniteDuration duration = duration("5 seconds"); + + // Simulate the ForwardedReadyTransaction messages that would be sent + // by the ShardTransaction. + + shard.tell(new ForwardedReadyTransaction(transactionID, CURRENT_VERSION, + cohort, modification, true), getRef()); + expectMsgClass(duration, ReadyTransactionReply.SERIALIZABLE_CLASS); + + // Send the CanCommitTransaction message. + + shard.tell(new CanCommitTransaction(transactionID).toSerializable(), getRef()); + CanCommitTransactionReply canCommitReply = CanCommitTransactionReply.fromSerializable( + expectMsgClass(duration, CanCommitTransactionReply.SERIALIZABLE_CLASS)); + assertEquals("Can commit", true, canCommitReply.getCanCommit()); + + // Send the CanCommitTransaction message. + + shard.tell(new CommitTransaction(transactionID).toSerializable(), getRef()); + expectMsgClass(duration, CommitTransactionReply.SERIALIZABLE_CLASS); + + InOrder inOrder = inOrder(cohort); + inOrder.verify(cohort).canCommit(); + inOrder.verify(cohort).preCommit(); + inOrder.verify(cohort).commit(); + + NormalizedNode actualNode = readStore(shard, TestModel.TEST_PATH); + assertEquals(TestModel.TEST_QNAME.getLocalName(), containerNode, actualNode); shard.tell(PoisonPill.getInstance(), ActorRef.noSender()); }}; @@ -954,26 +1017,24 @@ public class ShardTest extends AbstractActorTest { waitUntilLeader(shard); final FiniteDuration duration = duration("5 seconds"); - final Timeout timeout = new Timeout(duration); - InMemoryDOMDataStore dataStore = shard.underlyingActor().getDataStore(); final String transactionID = "tx1"; - final CountDownLatch abortComplete = new CountDownLatch(1); Function> preCommit = new Function>() { @Override public ListenableFuture apply(final DOMStoreThreePhaseCommitCohort cohort) { ListenableFuture preCommitFuture = cohort.preCommit(); - Future abortFuture = Patterns.ask(shard, - new AbortTransaction(transactionID).toSerializable(), timeout); - abortFuture.onComplete(new OnComplete() { - @Override - public void onComplete(final Throwable e, final Object resp) { - abortComplete.countDown(); - } - }, getSystem().dispatcher()); + // Simulate an AbortTransaction message occurring during replication, after + // persisting and before finishing the commit to the in-memory store. + // We have no followers so due to optimizations in the RaftActor, it does not + // attempt replication and thus we can't send an AbortTransaction message b/c + // it would be processed too late after CommitTransaction completes. So we'll + // simulate an AbortTransaction message occurring during replication by calling + // the shard directly. + // + shard.underlyingActor().doAbortTransaction(transactionID, null); return preCommitFuture; } @@ -993,14 +1054,14 @@ public class ShardTest extends AbstractActorTest { expectMsgClass(duration, CanCommitTransactionReply.SERIALIZABLE_CLASS)); assertEquals("Can commit", true, canCommitReply.getCanCommit()); - Future commitFuture = Patterns.ask(shard, - new CommitTransaction(transactionID).toSerializable(), timeout); - - assertEquals("Abort complete", true, abortComplete.await(5, TimeUnit.SECONDS)); - - Await.result(commitFuture, duration); + shard.tell(new CommitTransaction(transactionID).toSerializable(), getRef()); + expectMsgClass(duration, CommitTransactionReply.SERIALIZABLE_CLASS); NormalizedNode node = readStore(shard, TestModel.TEST_PATH); + + // Since we're simulating an abort occurring during replication and before finish commit, + // the data should still get written to the in-memory store since we've gotten past + // canCommit and preCommit and persisted the data. assertNotNull(TestModel.TEST_QNAME.getLocalName() + " not found", node); shard.tell(PoisonPill.getInstance(), ActorRef.noSender()); @@ -1009,7 +1070,7 @@ public class ShardTest extends AbstractActorTest { @Test public void testTransactionCommitTimeout() throws Throwable { - dataStoreContext = DatastoreContext.newBuilder().shardTransactionCommitTimeoutInSeconds(1).build(); + dataStoreContextBuilder.shardTransactionCommitTimeoutInSeconds(1); new ShardTestKit(getSystem()) {{ final TestActorRef shard = TestActorRef.create(getSystem(), @@ -1081,7 +1142,7 @@ public class ShardTest extends AbstractActorTest { @Test public void testTransactionCommitQueueCapacityExceeded() throws Throwable { - dataStoreContext = DatastoreContext.newBuilder().shardTransactionCommitQueueCapacity(1).build(); + dataStoreContextBuilder.shardTransactionCommitQueueCapacity(1); new ShardTestKit(getSystem()) {{ final TestActorRef shard = TestActorRef.create(getSystem(), @@ -1214,15 +1275,7 @@ public class ShardTest extends AbstractActorTest { // Wait for the 2nd Tx to complete the canCommit phase. - final CountDownLatch latch = new CountDownLatch(1); - canCommitFuture.onComplete(new OnComplete() { - @Override - public void onComplete(final Throwable t, final Object resp) { - latch.countDown(); - } - }, getSystem().dispatcher()); - - assertEquals("2nd CanCommit complete", true, latch.await(5, TimeUnit.SECONDS)); + Await.ready(canCommitFuture, duration); InOrder inOrder = inOrder(cohort1, cohort2); inOrder.verify(cohort1).canCommit(); @@ -1253,7 +1306,7 @@ public class ShardTest extends AbstractActorTest { @Override public Shard create() throws Exception { return new Shard(shardID, Collections.emptyMap(), - dataStoreContext, SCHEMA_CONTEXT) { + newDatastoreContext(), SCHEMA_CONTEXT) { @Override protected void commitSnapshot(final long sequenceNumber) { super.commitSnapshot(sequenceNumber); diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java index e24500a76c..dd51246493 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java @@ -891,10 +891,6 @@ public class RestconfImpl implements RestconfService { final DataNodeContainer parentSchema = (DataNodeContainer) incompleteInstIdWithData.getSchemaNode(); DOMMountPoint mountPoint = incompleteInstIdWithData.getMountPoint(); final Module module = findModule(mountPoint, payload); - if (module == null) { - throw new RestconfDocumentedException("Module was not found for \"" + payloadNS + "\"", - ErrorType.PROTOCOL, ErrorTag.UNKNOWN_ELEMENT); - } String payloadName = this.getName(payload); final DataSchemaNode schemaNode = ControllerContext.findInstanceDataChildByNameAndNamespace( @@ -941,11 +937,6 @@ public class RestconfImpl implements RestconfService { } final Module module = this.findModule(null, payload); - if (module == null) { - throw new RestconfDocumentedException( - "Data has bad format. Root element node has incorrect namespace (XML format) or module name(JSON format)", - ErrorType.PROTOCOL, ErrorTag.UNKNOWN_NAMESPACE); - } String payloadName = this.getName(payload); final DataSchemaNode schemaNode = ControllerContext.findInstanceDataChildByNameAndNamespace(module, @@ -1112,19 +1103,23 @@ public class RestconfImpl implements RestconfService { } private Module findModule(final DOMMountPoint mountPoint, final Node data) { + Module module = null; if (data instanceof NodeWrapper) { - return findModule(mountPoint, (NodeWrapper) data); + module = findModule(mountPoint, (NodeWrapper) data); } else if (data != null) { URI namespace = data.getNodeType().getNamespace(); if (mountPoint != null) { - return this.controllerContext.findModuleByNamespace(mountPoint, namespace); + module = this.controllerContext.findModuleByNamespace(mountPoint, namespace); } else { - return this.controllerContext.findModuleByNamespace(namespace); + module = this.controllerContext.findModuleByNamespace(namespace); } - } else { - throw new IllegalArgumentException("Unhandled parameter types: " - + Arrays. asList(mountPoint, data).toString()); } + if (module != null) { + return module; + } + throw new RestconfDocumentedException( + "Data has bad format. Root element node has incorrect namespace (XML format) or module name(JSON format)", + ErrorType.PROTOCOL, ErrorTag.UNKNOWN_NAMESPACE); } private Module findModule(final DOMMountPoint mountPoint, final NodeWrapper data) { diff --git a/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPusherImpl.java b/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPusherImpl.java index c20007d397..cf9958e7f8 100644 --- a/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPusherImpl.java +++ b/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPusherImpl.java @@ -115,17 +115,20 @@ public class ConfigPusherImpl implements ConfigPusher { */ private synchronized EditAndCommitResponse pushConfigWithConflictingVersionRetries(ConfigSnapshotHolder configSnapshotHolder) throws NetconfDocumentedException { ConflictingVersionException lastException; - Stopwatch stopwatch = new Stopwatch().start(); + Stopwatch stopwatch = new Stopwatch(); do { String idForReporting = configSnapshotHolder.toString(); SortedSet expectedCapabilities = checkNotNull(configSnapshotHolder.getCapabilities(), "Expected capabilities must not be null - %s, check %s", idForReporting, configSnapshotHolder.getClass().getName()); try (NetconfOperationService operationService = getOperationServiceWithRetries(expectedCapabilities, idForReporting)) { + if(!stopwatch.isRunning()) { + stopwatch.start(); + } return pushConfig(configSnapshotHolder, operationService); } catch (ConflictingVersionException e) { lastException = e; - LOG.debug("Conflicting version detected, will retry after timeout"); + LOG.info("Conflicting version detected, will retry after timeout"); sleep(); } } while (stopwatch.elapsed(TimeUnit.MILLISECONDS) < conflictingVersionTimeoutMillis); diff --git a/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/osgi/ConfigPersisterActivator.java b/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/osgi/ConfigPersisterActivator.java index 135d5ff9be..787f8b10b0 100644 --- a/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/osgi/ConfigPersisterActivator.java +++ b/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/osgi/ConfigPersisterActivator.java @@ -41,7 +41,7 @@ public class ConfigPersisterActivator implements BundleActivator { public static final String MAX_WAIT_FOR_CAPABILITIES_MILLIS_PROPERTY = "maxWaitForCapabilitiesMillis"; private static final long MAX_WAIT_FOR_CAPABILITIES_MILLIS_DEFAULT = TimeUnit.MINUTES.toMillis(2); public static final String CONFLICTING_VERSION_TIMEOUT_MILLIS_PROPERTY = "conflictingVersionTimeoutMillis"; - private static final long CONFLICTING_VERSION_TIMEOUT_MILLIS_DEFAULT = TimeUnit.SECONDS.toMillis(30); + private static final long CONFLICTING_VERSION_TIMEOUT_MILLIS_DEFAULT = TimeUnit.MINUTES.toMillis(1); public static final String NETCONF_CONFIG_PERSISTER = "netconf.config.persister"; diff --git a/opendaylight/netconf/netconf-netty-util/pom.xml b/opendaylight/netconf/netconf-netty-util/pom.xml index f1f7375f0a..45088f6086 100644 --- a/opendaylight/netconf/netconf-netty-util/pom.xml +++ b/opendaylight/netconf/netconf-netty-util/pom.xml @@ -85,13 +85,7 @@ maven-bundle-plugin - org.apache.sshd.*, com.google.common.base, com.google.common.collect, io.netty.buffer, - io.netty.channel, io.netty.channel.socket, io.netty.handler.codec, io.netty.handler.ssl, io.netty.util, - io.netty.util.concurrent, javax.xml.transform, javax.xml.transform.dom, javax.xml.transform.sax, - javax.xml.transform.stream, org.opendaylight.controller.netconf.api, - org.opendaylight.controller.netconf.util.messages, org.opendaylight.controller.netconf.util.xml, - org.opendaylight.protocol.framework, org.openexi.proc, org.openexi.proc.common, org.openexi.proc.grammars, - org.openexi.sax, org.openexi.schema, org.slf4j, org.w3c.dom, org.xml.sax + * org.opendaylight.controller.netconf.nettyutil, org.opendaylight.controller.netconf.nettyutil.handler, org.opendaylight.controller.netconf.nettyutil.handler.exi,