From: Tomas Olvecky Date: Wed, 19 Feb 2014 10:58:33 +0000 (+0100) Subject: Stop config-manager as first bundle when shutting down container. X-Git-Tag: autorelease-tag-v20140601202136_82eb3f9~405 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=3e0009d12281aeda90231c01c3945031aca5c528 Stop config-manager as first bundle when shutting down container. If config-manager is stopped after a configured module's bundle has been stopped, config-manager will be unable to cleanly close other instances, so it needs to be stopped as first bundle. GlobalEventExecutorModule (netty-global-event-executor) is singleton, enforce it in only allowing one instance called 'singleton'. Make shutdown grace period smaller in order to close netty threads sooner that shutdown issues System.exit(1). Change-Id: I042c606a5bedc49492784dcb0a5ad4dcbdcb0939 Signed-off-by: Tomas Olvecky --- diff --git a/opendaylight/config/netty-event-executor-config/src/main/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModule.java b/opendaylight/config/netty-event-executor-config/src/main/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModule.java index 3c52d8c17a..2c4c211784 100644 --- a/opendaylight/config/netty-event-executor-config/src/main/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModule.java +++ b/opendaylight/config/netty-event-executor-config/src/main/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModule.java @@ -17,19 +17,14 @@ */ package org.opendaylight.controller.config.yang.netty.eventexecutor; -import io.netty.util.concurrent.AbstractEventExecutor; +import com.google.common.reflect.AbstractInvocationHandler; +import com.google.common.reflect.Reflection; import io.netty.util.concurrent.EventExecutor; -import io.netty.util.concurrent.EventExecutorGroup; -import io.netty.util.concurrent.Future; import io.netty.util.concurrent.GlobalEventExecutor; -import io.netty.util.concurrent.ScheduledFuture; -import java.util.concurrent.Callable; +import java.lang.reflect.Method; import java.util.concurrent.TimeUnit; -/** -* -*/ public final class GlobalEventExecutorModule extends org.opendaylight.controller.config.yang.netty.eventexecutor.AbstractGlobalEventExecutorModule { @@ -51,91 +46,35 @@ public final class GlobalEventExecutorModule extends @Override public java.lang.AutoCloseable createInstance() { - return new GlobalEventExecutorCloseable(GlobalEventExecutor.INSTANCE); + final CloseableGlobalEventExecutorMixin closeableGlobalEventExecutorMixin = + new CloseableGlobalEventExecutorMixin(GlobalEventExecutor.INSTANCE); + return Reflection.newProxy(AutoCloseableEventExecutor.class, new AbstractInvocationHandler() { + @Override + protected Object handleInvocation(Object proxy, Method method, Object[] args) throws Throwable { + if (method.getName().equals("close")) { + closeableGlobalEventExecutorMixin.close(); + return null; + } else { + return method.invoke(GlobalEventExecutor.INSTANCE, args); + } + } + }); } - static final private class GlobalEventExecutorCloseable extends AbstractEventExecutor implements AutoCloseable { - - private EventExecutor executor; - - public GlobalEventExecutorCloseable(EventExecutor executor) { - this.executor = executor; - } - - @Override - public EventExecutorGroup parent() { - return this.executor.parent(); - } - - @Override - public boolean inEventLoop(Thread thread) { - return this.executor.inEventLoop(thread); - } - - @Override - public boolean isShuttingDown() { - return this.executor.isShuttingDown(); - } - - @Override - public Future shutdownGracefully(long quietPeriod, long timeout, TimeUnit unit) { - return this.executor.shutdownGracefully(quietPeriod, timeout, unit); - } - - @Override - public Future terminationFuture() { - return this.executor.terminationFuture(); - } - - @Override - public boolean isShutdown() { - return this.executor.isShutdown(); - } + public static interface AutoCloseableEventExecutor extends EventExecutor, AutoCloseable { - @Override - public boolean isTerminated() { - return this.executor.isTerminated(); - } - - @Override - public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException { - return this.executor.awaitTermination(timeout, unit); - } - - @Override - public void execute(Runnable command) { - this.executor.execute(command); - } - - @Override - public void close() throws Exception { - shutdownGracefully(); - } - - @SuppressWarnings("deprecation") - @Override - public void shutdown() { - this.executor.shutdown(); - } - - @Override - public ScheduledFuture scheduleWithFixedDelay(Runnable command, long initialDelay, long delay, TimeUnit unit) { - return this.executor.scheduleWithFixedDelay(command, initialDelay, delay, unit); - } + } - @Override - public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) { - return this.executor.schedule(command, delay, unit); - } + public static class CloseableGlobalEventExecutorMixin implements AutoCloseable { + private final GlobalEventExecutor eventExecutor; - @Override - public ScheduledFuture schedule(Callable callable, long delay, TimeUnit unit) { - return this.executor.schedule(callable, delay, unit); + public CloseableGlobalEventExecutorMixin(GlobalEventExecutor eventExecutor) { + this.eventExecutor = eventExecutor; } @Override - public ScheduledFuture scheduleAtFixedRate(Runnable command, long initialDelay, long period, TimeUnit unit) { - return this.executor.scheduleAtFixedRate(command, initialDelay, period, unit); + public void close() { + eventExecutor.shutdownGracefully(0, 1, TimeUnit.SECONDS); } } } diff --git a/opendaylight/config/netty-event-executor-config/src/main/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModuleFactory.java b/opendaylight/config/netty-event-executor-config/src/main/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModuleFactory.java index c03ade8b5f..1585bbf6de 100644 --- a/opendaylight/config/netty-event-executor-config/src/main/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModuleFactory.java +++ b/opendaylight/config/netty-event-executor-config/src/main/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModuleFactory.java @@ -7,21 +7,34 @@ */ /** -* Generated file - -* Generated from: yang module name: netty-event-executor yang module local name: netty-global-event-executor -* Generated by: org.opendaylight.controller.config.yangjmxgenerator.plugin.JMXGenerator -* Generated at: Tue Nov 12 10:44:21 CET 2013 -* -* Do not modify this file unless it is present under src/main directory -*/ + * Generated file + + * Generated from: yang module name: netty-event-executor yang module local name: netty-global-event-executor + * Generated by: org.opendaylight.controller.config.yangjmxgenerator.plugin.JMXGenerator + * Generated at: Tue Nov 12 10:44:21 CET 2013 + * + * Do not modify this file unless it is present under src/main directory + */ package org.opendaylight.controller.config.yang.netty.eventexecutor; -/** -* -*/ -public class GlobalEventExecutorModuleFactory extends org.opendaylight.controller.config.yang.netty.eventexecutor.AbstractGlobalEventExecutorModuleFactory -{ +import org.opendaylight.controller.config.api.DependencyResolver; +import org.osgi.framework.BundleContext; + +import static com.google.common.base.Preconditions.checkArgument; + +public class GlobalEventExecutorModuleFactory extends org.opendaylight.controller.config.yang.netty.eventexecutor.AbstractGlobalEventExecutorModuleFactory { + public static final String SINGLETON_NAME = "singleton"; + + @Override + public GlobalEventExecutorModule instantiateModule(String instanceName, DependencyResolver dependencyResolver, GlobalEventExecutorModule oldModule, AutoCloseable oldInstance, BundleContext bundleContext) { + checkArgument(SINGLETON_NAME.equals(instanceName),"Illegal instance name '" + instanceName + "', only allowed name is " + SINGLETON_NAME); + return super.instantiateModule(instanceName, dependencyResolver, oldModule, oldInstance, bundleContext); + } + @Override + public GlobalEventExecutorModule instantiateModule(String instanceName, DependencyResolver dependencyResolver, BundleContext bundleContext) { + checkArgument(SINGLETON_NAME.equals(instanceName),"Illegal instance name '" + instanceName + "', only allowed name is " + SINGLETON_NAME); + return super.instantiateModule(instanceName, dependencyResolver, bundleContext); + } } diff --git a/opendaylight/config/netty-event-executor-config/src/test/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModuleTest.java b/opendaylight/config/netty-event-executor-config/src/test/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModuleTest.java index 71c4b192e5..f29895c6d0 100644 --- a/opendaylight/config/netty-event-executor-config/src/test/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModuleTest.java +++ b/opendaylight/config/netty-event-executor-config/src/test/java/org/opendaylight/controller/config/yang/netty/eventexecutor/GlobalEventExecutorModuleTest.java @@ -8,9 +8,6 @@ package org.opendaylight.controller.config.yang.netty.eventexecutor; -import javax.management.InstanceAlreadyExistsException; -import javax.management.ObjectName; - import org.junit.Before; import org.junit.Test; import org.opendaylight.controller.config.api.ConflictingVersionException; @@ -20,10 +17,16 @@ import org.opendaylight.controller.config.manager.impl.AbstractConfigTest; import org.opendaylight.controller.config.manager.impl.factoriesresolver.HardcodedModuleFactoriesResolver; import org.opendaylight.controller.config.util.ConfigTransactionJMXClient; +import javax.management.InstanceAlreadyExistsException; +import javax.management.ObjectName; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + public class GlobalEventExecutorModuleTest extends AbstractConfigTest { private GlobalEventExecutorModuleFactory factory; - private final String instanceName = "netty1"; + private final String instanceName = GlobalEventExecutorModuleFactory.SINGLETON_NAME; @Before public void setUp() { @@ -37,12 +40,23 @@ public class GlobalEventExecutorModuleTest extends AbstractConfigTest { ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction(); createInstance(transaction, instanceName); - createInstance(transaction, instanceName + 2); + transaction.validateConfig(); CommitStatus status = transaction.commit(); - assertBeanCount(2, factory.getImplementationName()); - assertStatus(status, 2, 0, 0); + assertBeanCount(1, factory.getImplementationName()); + assertStatus(status, 1, 0, 0); + } + + @Test + public void testConflictingName() throws Exception { + ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction(); + try { + createInstance(transaction, instanceName + "x"); + fail(); + }catch(IllegalArgumentException e){ + assertTrue(e.getMessage() + " failure", e.getMessage().contains("only allowed name is singleton")); + } } @Test diff --git a/opendaylight/config/netty-threadgroup-config/src/main/java/org/opendaylight/controller/config/yang/netty/threadgroup/NettyThreadgroupModule.java b/opendaylight/config/netty-threadgroup-config/src/main/java/org/opendaylight/controller/config/yang/netty/threadgroup/NettyThreadgroupModule.java index 3d5a3bf4ad..06c9ae885d 100644 --- a/opendaylight/config/netty-threadgroup-config/src/main/java/org/opendaylight/controller/config/yang/netty/threadgroup/NettyThreadgroupModule.java +++ b/opendaylight/config/netty-threadgroup-config/src/main/java/org/opendaylight/controller/config/yang/netty/threadgroup/NettyThreadgroupModule.java @@ -21,6 +21,8 @@ import io.netty.channel.nio.NioEventLoopGroup; import org.opendaylight.controller.config.api.JmxAttributeValidationException; +import java.util.concurrent.TimeUnit; + /** * */ @@ -61,7 +63,7 @@ public final class NettyThreadgroupModule extends org.opendaylight.controller.co @Override public void close() throws Exception { - shutdownGracefully(); + shutdownGracefully(0, 1, TimeUnit.SECONDS); } } } diff --git a/opendaylight/config/shutdown-impl/src/main/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownServiceImpl.java b/opendaylight/config/shutdown-impl/src/main/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownServiceImpl.java index 584ea1766e..f9622192fe 100644 --- a/opendaylight/config/shutdown-impl/src/main/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownServiceImpl.java +++ b/opendaylight/config/shutdown-impl/src/main/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownServiceImpl.java @@ -82,6 +82,7 @@ class Impl implements ShutdownService { class StopSystemBundleThread extends Thread { private static final Logger logger = LoggerFactory.getLogger(StopSystemBundleThread.class); + public static final String CONFIG_MANAGER_SYMBOLIC_NAME = "org.opendaylight.controller.config-manager"; private final Bundle systemBundle; StopSystemBundleThread(Bundle systemBundle) { @@ -94,6 +95,14 @@ class StopSystemBundleThread extends Thread { try { // wait so that JMX response is received Thread.sleep(1000); + // first try to stop config-manager + Bundle configManager = findConfigManager(); + if (configManager != null){ + logger.debug("Stopping config-manager"); + configManager.stop(); + Thread.sleep(1000); + } + logger.debug("Stopping system bundle"); systemBundle.stop(); } catch (BundleException e) { logger.warn("Can not stop OSGi server", e); @@ -101,6 +110,16 @@ class StopSystemBundleThread extends Thread { logger.warn("Shutdown process interrupted", e); } } + + private Bundle findConfigManager() { + for(Bundle bundle: systemBundle.getBundleContext().getBundles()){ + if (CONFIG_MANAGER_SYMBOLIC_NAME.equals(bundle.getSymbolicName())) { + return bundle; + } + } + return null; + } + } class CallSystemExitThread extends Thread { diff --git a/opendaylight/config/shutdown-impl/src/test/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownTest.java b/opendaylight/config/shutdown-impl/src/test/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownTest.java index 5887e98f30..d1abe08d52 100644 --- a/opendaylight/config/shutdown-impl/src/test/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownTest.java +++ b/opendaylight/config/shutdown-impl/src/test/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownTest.java @@ -18,6 +18,7 @@ import org.opendaylight.controller.config.manager.impl.factoriesresolver.ModuleF import org.opendaylight.controller.config.util.ConfigTransactionJMXClient; import org.osgi.framework.Bundle; +import javax.management.InstanceNotFoundException; import javax.management.JMX; import javax.management.ObjectName; import java.util.Collections; @@ -26,15 +27,14 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.opendaylight.controller.config.yang.shutdown.impl.ShutdownModuleFactory.NAME; public class ShutdownTest extends AbstractConfigTest { private final ShutdownModuleFactory factory = new ShutdownModuleFactory(); @Mock - private Bundle mockedSysBundle; + private Bundle mockedSysBundle, mockedConfigManager; + @Before public void setUp() throws Exception { @@ -44,6 +44,13 @@ public class ShutdownTest extends AbstractConfigTest { doReturn(mockedSysBundle).when(mockedContext).getBundle(0); mockedContext.getBundle(0); doNothing().when(mockedSysBundle).stop(); + doNothing().when(mockedConfigManager).stop(); + doReturn(mockedContext).when(mockedSysBundle).getBundleContext(); + doReturn(new Bundle[]{mockedSysBundle, mockedConfigManager}).when(mockedContext).getBundles(); + doReturn("system bundle").when(mockedSysBundle).getSymbolicName(); + doReturn(StopSystemBundleThread.CONFIG_MANAGER_SYMBOLIC_NAME).when(mockedConfigManager).getSymbolicName(); + + ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction(); // initialize default instance transaction.commit(); @@ -80,13 +87,22 @@ public class ShutdownTest extends AbstractConfigTest { @Test public void testWithSecret() throws Exception { + String secret = "secret"; + setSecret(secret); + shutdownViaRuntimeJMX(secret); + } + + private void setSecret(String secret) throws InstanceNotFoundException { ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction(); ObjectName on = transaction.lookupConfigBean(NAME, NAME); ShutdownModuleMXBean proxy = transaction.newMXBeanProxy(on, ShutdownModuleMXBean.class); - String secret = "secret"; proxy.setSecret(secret); transaction.commit(); - shutdownViaRuntimeJMX(secret); + } + + @Test + public void testWrongSecret() throws Exception { + setSecret("secret"); try { ShutdownRuntimeMXBean runtime = JMX.newMXBeanProxy(platformMBeanServer, runtimeON, ShutdownRuntimeMXBean.class); runtime.shutdown("foo", 60000L, null); @@ -109,12 +125,9 @@ public class ShutdownTest extends AbstractConfigTest { assertStopped(); } - private void assertStopped() throws Exception { - Thread.sleep(2000); // happens on another thread + Thread.sleep(3000); // happens on another thread + verify(mockedConfigManager).stop(); verify(mockedSysBundle).stop(); - verifyNoMoreInteractions(mockedSysBundle); - reset(mockedSysBundle); - doNothing().when(mockedSysBundle).stop(); } } diff --git a/opendaylight/distribution/opendaylight/src/main/resources/configuration/initial/00-netty.xml b/opendaylight/distribution/opendaylight/src/main/resources/configuration/initial/00-netty.xml index 9ba590820a..2365c700f9 100644 --- a/opendaylight/distribution/opendaylight/src/main/resources/configuration/initial/00-netty.xml +++ b/opendaylight/distribution/opendaylight/src/main/resources/configuration/initial/00-netty.xml @@ -23,7 +23,7 @@ netty:netty-global-event-executor - global-event-executor + singleton @@ -43,7 +43,7 @@ netty:netty-event-executor global-event-executor - /modules/module[type='netty-global-event-executor'][name='global-event-executor'] + /modules/module[type='netty-global-event-executor'][name='singleton'] diff --git a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfImplActivator.java b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfImplActivator.java index 95f7353600..b8dc9550c7 100644 --- a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfImplActivator.java +++ b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfImplActivator.java @@ -26,6 +26,7 @@ import java.lang.management.ManagementFactory; import java.net.InetSocketAddress; import java.util.Dictionary; import java.util.Hashtable; +import java.util.concurrent.TimeUnit; public class NetconfImplActivator implements BundleActivator { @@ -88,7 +89,7 @@ public class NetconfImplActivator implements BundleActivator { logger.info("Shutting down netconf because YangStoreService service was removed"); commitNot.close(); - eventLoopGroup.shutdownGracefully(); + eventLoopGroup.shutdownGracefully(0, 1, TimeUnit.SECONDS); timer.stop(); regMonitoring.unregister();