Close OpenFlowPluginProvider during shutdown 60/58260/3
authorTomas Slusny <tomas.slusny@pantheon.tech>
Mon, 5 Jun 2017 15:47:51 +0000 (17:47 +0200)
committerTomas Slusny <tomas.slusny@pantheon.tech>
Sun, 11 Jun 2017 13:15:37 +0000 (13:15 +0000)
- Call close method on OpenFlowPluginProvider during shutdown
- Close all created managers, thread pool and timer during shutdown
- Remove unused notification service
- Remove warning when unknown property is loaded on start (because of
  recent FRM changes)

Resolves: bug 8598

Change-Id: Ib5a4f84ea1fa0c957b90ef216346eb85aec81ad0
Signed-off-by: Tomas Slusny <tomas.slusny@pantheon.tech>
openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/OpenFlowPluginProviderFactory.java
openflowplugin-blueprint-config/src/main/resources/org/opendaylight/blueprint/openflowplugin.xml
openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/OpenFlowPluginProviderFactoryImpl.java
openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/OpenFlowPluginProviderImpl.java
openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/OpenFlowPluginProviderImplTest.java

index c1fcaa2ef9b2014f3d8bb22f851845f050e28721..8a0bf197daf7e88f5bd0d6b9e487848f9845be7d 100644 (file)
@@ -10,7 +10,6 @@ package org.opendaylight.openflowplugin.api.openflow;
 import java.util.List;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService;
-import org.opendaylight.controller.md.sal.binding.api.NotificationService;
 import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipService;
 import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider;
@@ -25,7 +24,6 @@ public interface OpenFlowPluginProviderFactory {
     OpenFlowPluginProvider newInstance(OpenflowProviderConfig providerConfig,
                                        DataBroker dataBroker,
                                        RpcProviderRegistry rpcRegistry,
-                                       NotificationService notificationService,
                                        NotificationPublishService notificationPublishService,
                                        EntityOwnershipService entityOwnershipService,
                                        List<SwitchConnectionProvider> switchConnectionProviders,
index 58c2916e111e305a8b6c6104be4e6abcc2cb66a9..088c8cbb5369be93fea3176c0119e407b8eb99ca 100644 (file)
@@ -6,7 +6,6 @@
 
     <reference id="dataBroker" interface="org.opendaylight.controller.md.sal.binding.api.DataBroker" odl:type="pingpong"/>
     <reference id="rpcRegistry" interface="org.opendaylight.controller.sal.binding.api.RpcProviderRegistry"/>
-    <reference id="notificationService" interface="org.opendaylight.controller.md.sal.binding.api.NotificationService"/>
     <reference id="notificationPublishService" interface="org.opendaylight.controller.md.sal.binding.api.NotificationPublishService"/>
     <reference id="entityOwnershipService" interface="org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipService"/>
     <reference id="clusterSingletonServiceProvider" interface="org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider"/>
     <reference id="openflowPluginProviderFactory"
                interface="org.opendaylight.openflowplugin.api.openflow.OpenFlowPluginProviderFactory"/>
 
-    <bean id="openflowPluginProvider" factory-ref="openflowPluginProviderFactory" factory-method="newInstance">
+    <bean id="openflowPluginProvider" factory-ref="openflowPluginProviderFactory" factory-method="newInstance" destroy-method="close">
         <argument ref="openflowProviderConfig"/>
         <argument ref="dataBroker"/>
         <argument ref="rpcRegistry"/>
-        <argument ref="notificationService"/>
         <argument ref="notificationPublishService"/>
         <argument ref="entityOwnershipService"/>
         <argument>
index 934fb58ca515083fc29383656f7f90d74fcc31e2..6044298c19af3e3db99e6d685d99a9287a2c48bd 100644 (file)
@@ -15,7 +15,6 @@ import java.util.Map;
 import java.util.Optional;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService;
-import org.opendaylight.controller.md.sal.binding.api.NotificationService;
 import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipService;
 import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider;
@@ -43,7 +42,6 @@ public class OpenFlowPluginProviderFactoryImpl implements OpenFlowPluginProvider
     public OpenFlowPluginProvider newInstance(final OpenflowProviderConfig providerConfig,
                                               final DataBroker dataBroker,
                                               final RpcProviderRegistry rpcRegistry,
-                                              final NotificationService notificationService,
                                               final NotificationPublishService notificationPublishService,
                                               final EntityOwnershipService entityOwnershipService,
                                               final List<SwitchConnectionProvider> switchConnectionProviders,
@@ -56,7 +54,6 @@ public class OpenFlowPluginProviderFactoryImpl implements OpenFlowPluginProvider
                 switchConnectionProviders,
                 dataBroker,
                 rpcRegistry,
-                notificationService,
                 notificationPublishService,
                 singletonServiceProvider,
                 entityOwnershipService);
index c919099948de5c035f3686c5da1c5a53d6a13e63..58137e51012851421111090c50037ae353802fe9 100644 (file)
@@ -12,6 +12,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import io.netty.util.HashedWheelTimer;
+import io.netty.util.Timer;
 import java.lang.management.ManagementFactory;
 import java.util.Collection;
 import java.util.List;
@@ -24,6 +25,7 @@ import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
 import javax.management.MBeanRegistrationException;
 import javax.management.MBeanServer;
 import javax.management.MalformedObjectNameException;
@@ -31,7 +33,6 @@ import javax.management.NotCompliantMBeanException;
 import javax.management.ObjectName;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService;
-import org.opendaylight.controller.md.sal.binding.api.NotificationService;
 import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipService;
 import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider;
@@ -71,13 +72,18 @@ import org.slf4j.LoggerFactory;
 public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenFlowPluginConfigurationService, OpenFlowPluginExtensionRegistratorProvider {
 
     private static final Logger LOG = LoggerFactory.getLogger(OpenFlowPluginProviderImpl.class);
-    private static final MessageIntelligenceAgency messageIntelligenceAgency = new MessageIntelligenceAgencyImpl();
+
     private static final int TICKS_PER_WHEEL = 500; // 0.5 sec.
     private static final long TICK_DURATION = 10;
     private static final String POOL_NAME = "ofppool";
 
+    private static final MessageIntelligenceAgency MESSAGE_INTELLIGENCE_AGENCY = new MessageIntelligenceAgencyImpl();
+    private static final String MESSAGE_INTELLIGENCE_AGENCY_MX_BEAN_NAME = String
+            .format("%s:type=%s",
+                    MessageIntelligenceAgencyMXBean.class.getPackage().getName(),
+                    MessageIntelligenceAgencyMXBean.class.getSimpleName());
+
     private final HashedWheelTimer hashedWheelTimer = new HashedWheelTimer(TICK_DURATION, TimeUnit.MILLISECONDS, TICKS_PER_WHEEL);
-    private final NotificationService notificationProviderService;
     private final NotificationPublishService notificationPublishService;
     private final ExtensionConverterManager extensionConverterManager;
     private final DataBroker dataBroker;
@@ -85,6 +91,8 @@ public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenF
     private final DeviceInitializerProvider deviceInitializerProvider;
     private final ConvertorManager convertorManager;
     private final ContextChainHolder contextChainHolder;
+    private final RpcProviderRegistry rpcProviderRegistry;
+    private final ClusterSingletonServiceProvider singletonServicesProvider;
     private int rpcRequestsQuota;
     private long globalNotificationQuota;
     private long barrierInterval;
@@ -92,7 +100,6 @@ public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenF
     private long echoReplyTimeout;
     private DeviceManager deviceManager;
     private RpcManager rpcManager;
-    private RpcProviderRegistry rpcProviderRegistry;
     private StatisticsManager statisticsManager;
     private ConnectionManager connectionManager;
     private boolean switchFeaturesMandatory;
@@ -104,27 +111,24 @@ public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenF
     private long maximumTimerDelay;
     private boolean useSingleLayerSerialization;
     private ThreadPoolExecutor threadPool;
-    private ClusterSingletonServiceProvider singletonServicesProvider;
     private int threadPoolMinThreads;
     private int threadPoolMaxThreads;
     private long threadPoolTimeout;
     private boolean initialized = false;
 
     public static MessageIntelligenceAgency getMessageIntelligenceAgency() {
-        return messageIntelligenceAgency;
+        return MESSAGE_INTELLIGENCE_AGENCY;
     }
 
-    public OpenFlowPluginProviderImpl(final List<SwitchConnectionProvider> switchConnectionProviders,
-                                      final DataBroker dataBroker,
-                                      final RpcProviderRegistry rpcProviderRegistry,
-                                      final NotificationService notificationProviderService,
-                                      final NotificationPublishService notificationPublishService,
-                                      final ClusterSingletonServiceProvider singletonServiceProvider,
-                                      final EntityOwnershipService entityOwnershipService) {
+    OpenFlowPluginProviderImpl(final List<SwitchConnectionProvider> switchConnectionProviders,
+                               final DataBroker dataBroker,
+                               final RpcProviderRegistry rpcProviderRegistry,
+                               final NotificationPublishService notificationPublishService,
+                               final ClusterSingletonServiceProvider singletonServiceProvider,
+                               final EntityOwnershipService entityOwnershipService) {
         this.switchConnectionProviders = switchConnectionProviders;
         this.dataBroker = dataBroker;
         this.rpcProviderRegistry = rpcProviderRegistry;
-        this.notificationProviderService = notificationProviderService;
         this.notificationPublishService = notificationPublishService;
         this.singletonServicesProvider = singletonServiceProvider;
         convertorManager = ConvertorManagerFactory.createDefaultManager();
@@ -137,7 +141,7 @@ public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenF
 
     private void startSwitchConnections() {
         Futures.addCallback(Futures.allAsList(switchConnectionProviders.stream().map(switchConnectionProvider -> {
-            // Inject OpenflowPlugin custom serializers and deserializers into OpenflowJava
+            // Inject OpenFlowPlugin custom serializers and deserializers into OpenFlowJava
             if (useSingleLayerSerialization) {
                 SerializerInjector.injectSerializers(switchConnectionProvider);
                 DeserializerInjector.injectDeserializers(switchConnectionProvider);
@@ -155,19 +159,36 @@ public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenF
             }
 
             @Override
-            public void onFailure(@Nonnull final Throwable t) {
-                LOG.warn("Some switchConnectionProviders failed to start.", t);
+            public void onFailure(@Nonnull final Throwable throwable) {
+                LOG.warn("Some switchConnectionProviders failed to start.", throwable);
+            }
+        });
+    }
+
+    private void shutdownSwitchConnections() {
+        Futures.addCallback(Futures.allAsList(switchConnectionProviders.stream().map(switchConnectionProvider -> {
+            // Revert deserializers to their original state
+            if (useSingleLayerSerialization) {
+                DeserializerInjector.revertDeserializers(switchConnectionProvider);
+            }
+
+            // Shutdown switch connection provider
+            return switchConnectionProvider.shutdown();
+        }).collect(Collectors.toSet())), new FutureCallback<List<Boolean>>() {
+            @Override
+            public void onSuccess(final List<Boolean> result) {
+                LOG.info("All switchConnectionProviders were successfully shut down ({}).", result.size());
+            }
+
+            @Override
+            public void onFailure(@Nonnull final Throwable throwable) {
+                LOG.warn("Some switchConnectionProviders failed to shutdown.", throwable);
             }
         });
     }
 
     @Override
     public void initialize() {
-        Preconditions.checkNotNull(dataBroker, "missing data broker");
-        Preconditions.checkNotNull(rpcProviderRegistry, "missing RPC provider registry");
-        Preconditions.checkNotNull(notificationProviderService, "missing notification provider service");
-        Preconditions.checkNotNull(singletonServicesProvider, "missing singleton services provider");
-
         // TODO: copied from OpenFlowPluginProvider (Helium) misusesing the old way of distributing extension converters
         // TODO: rewrite later!
         OFSessionUtil.getSessionManager().setExtensionConverterProvider(extensionConverterManager);
@@ -184,7 +205,7 @@ public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenF
         connectionManager = new ConnectionManagerImpl(threadPool);
         connectionManager.setEchoReplyTimeout(echoReplyTimeout);
 
-        registerMXBean(messageIntelligenceAgency);
+        registerMXBean(MESSAGE_INTELLIGENCE_AGENCY, MESSAGE_INTELLIGENCE_AGENCY_MX_BEAN_NAME);
 
         contextChainHolder.addSingletonServicesProvider(singletonServicesProvider);
 
@@ -241,8 +262,6 @@ public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenF
 
             if (Objects.nonNull(propertyType)) {
                 updateProperty(propertyType, value);
-            } else if (!key.equals("service.pid") && !key.equals("felix.fileinstall.filename")) {
-                LOG.warn("Unsupported configuration property '{}={}'", key, value);
             }
         });
     }
@@ -474,23 +493,57 @@ public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenF
     @Override
     public void close() throws Exception {
         initialized = false;
-        //TODO: consider wrapping each manager into try-catch
-        deviceManager.close();
-        rpcManager.close();
-        statisticsManager.close();
+        gracefulShutdown(contextChainHolder);
+        gracefulShutdown(deviceManager);
+        gracefulShutdown(rpcManager);
+        gracefulShutdown(statisticsManager);
+        gracefulShutdown(threadPool);
+        gracefulShutdown(hashedWheelTimer);
+        shutdownSwitchConnections();
+        unregisterMXBean(MESSAGE_INTELLIGENCE_AGENCY_MX_BEAN_NAME);
+    }
 
-        // Manually shutdown all remaining running threads in pool
-        threadPool.shutdown();
+    private static void gracefulShutdown(final AutoCloseable closeable) {
+        if (Objects.isNull(closeable)) {
+            return;
+        }
+
+        try {
+            closeable.close();
+        } catch (Exception e) {
+            LOG.warn("Failed to shutdown {} gracefully.", closeable);
+        }
+    }
+
+    private static void gracefulShutdown(final Timer timer) {
+        if (Objects.isNull(timer)) {
+            return;
+        }
+
+        try {
+            timer.stop();
+        } catch (Exception e) {
+            LOG.warn("Failed to shutdown {} gracefully.", timer);
+        }
     }
 
-    private static void registerMXBean(final MessageIntelligenceAgency messageIntelligenceAgency) {
+    private static void gracefulShutdown(final ThreadPoolExecutor threadPoolExecutor) {
+        if (Objects.isNull(threadPoolExecutor)) {
+            return;
+        }
+
+        try {
+            threadPoolExecutor.shutdown();
+        } catch (Exception e) {
+            LOG.warn("Failed to shutdown {} gracefully.", threadPoolExecutor);
+        }
+    }
+
+    private static void registerMXBean(final Object bean, final String beanName) {
         final MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
+
         try {
-            final String pathToMxBean = String.format("%s:type=%s",
-                    MessageIntelligenceAgencyMXBean.class.getPackage().getName(),
-                    MessageIntelligenceAgencyMXBean.class.getSimpleName());
-            final ObjectName name = new ObjectName(pathToMxBean);
-            mbs.registerMBean(messageIntelligenceAgency, name);
+            mbs.registerMBean(bean, new ObjectName(beanName));
         } catch (MalformedObjectNameException
                 | NotCompliantMBeanException
                 | MBeanRegistrationException
@@ -498,4 +551,16 @@ public class OpenFlowPluginProviderImpl implements OpenFlowPluginProvider, OpenF
             LOG.warn("Error registering MBean {}", e);
         }
     }
+
+    private static void unregisterMXBean(final String beanName) {
+        final MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
+
+        try {
+            mbs.unregisterMBean(new ObjectName(beanName));
+        } catch (InstanceNotFoundException
+                | MBeanRegistrationException
+                | MalformedObjectNameException e) {
+            LOG.warn("Error unregistering MBean {}", e);
+        }
+    }
 }
index 1f2d4e9b82b2d46d5c35fe955ea46aec0ffd65bd..0162beb03736aa7796dc793dbf9a0f3c6a35d37e 100644 (file)
@@ -23,7 +23,6 @@ import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService;
-import org.opendaylight.controller.md.sal.binding.api.NotificationService;
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipListenerRegistration;
 import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipService;
@@ -43,9 +42,6 @@ public class OpenFlowPluginProviderImplTest {
     @Mock
     RpcProviderRegistry rpcProviderRegistry;
 
-    @Mock
-    NotificationService notificationService;
-
     @Mock
     NotificationPublishService notificationPublishService;
 
@@ -87,7 +83,6 @@ public class OpenFlowPluginProviderImplTest {
                 Lists.newArrayList(switchConnectionProvider),
                 dataBroker,
                 rpcProviderRegistry,
-                notificationService,
                 notificationPublishService,
                 clusterSingletonServiceProvider,
                 entityOwnershipService);