Bug 4957 Fixing DeviceCtx lifeCycle

 - validate unique DeviceCtx registration in DeviceManager
 - onDeviceContextLevelUp has to stop PacketInFiltering for slave too
 - simplification of teardown
 - invocation of closeHanders in reverse order

Change-Id: I6f4f172e9cb7497c91a03712a4c5d571e99b7f41
Signed-off-by: Jozef Bacigal <jbacigal@cisco.com>

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java
index 683bb12c661937dbd80aaaf8158903d6c1c4d7c2..27d48e592bf63716689c94a9f235d041ad11a01a 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java
@@ -10,8 +10,10 @@ package org.opendaylight.openflowplugin.impl.device;
 import javax.annotation.Nonnull;
 import java.math.BigInteger;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -180,6 +182,11 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi
         itemLifeCycleSourceRegistry.registerLifeCycleSource(flowLifeCycleKeeper);
     }
 
+    /**
+     * @deprecated will be deleted
+     * @param txChainManager
+     */
+    @Deprecated
     void setTransactionChainManager(final TransactionChainManager txChainManager) {
         this.transactionChainManager = Preconditions.checkNotNull(txChainManager);
     }
@@ -434,7 +441,7 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi
         primaryConnectionContext.closeConnection(false);
     }
 
-    private void tearDown() {
+    private synchronized void tearDown() {
         deviceState.setValid(false);
 
         for (final ConnectionContext connectionContext : auxiliaryConnectionContexts.values()) {
@@ -445,17 +452,17 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi
         deviceFlowRegistry.close();
         deviceMeterRegistry.close();
 
-        itemLifeCycleSourceRegistry.clear();
-
-
-        for (final DeviceContextClosedHandler deviceContextClosedHandler : closeHandlers) {
+        final LinkedList<DeviceContextClosedHandler> reversedCloseHandlers = new LinkedList<>(closeHandlers);
+        Collections.reverse(reversedCloseHandlers);
+        for (final DeviceContextClosedHandler deviceContextClosedHandler : reversedCloseHandlers) {
             deviceContextClosedHandler.onDeviceContextClosed(this);
         }
-
-        LOG.info("Closing transaction chain manager without cleaning inventory operational");
-        transactionChainManager.close();
+        closeHandlers.clear();
     }
 
+    /**
+     * @deprecated will be deleted
+     */
     @Override
     public void onDeviceDisconnectedFromCluster() {
         LOG.info("Removing device from operational and closing transaction Manager for device:{}", getDeviceState().getNodeId());

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceManagerImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceManagerImpl.java
index 0c42b68d5f12d0e233287b0887d992dd0ef659a5..49036c7ff304e1f22e830bc4393ba941868dd752 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceManagerImpl.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceManagerImpl.java
@@ -11,11 +11,13 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import java.util.Collections;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 
 import com.google.common.base.Preconditions;
+import com.google.common.base.Verify;
 import io.netty.util.HashedWheelTimer;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService;
@@ -40,7 +42,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodesBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.role.service.rev150727.OfpRole;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -64,7 +65,7 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi
     private NotificationService notificationService;
     private NotificationPublishService notificationPublishService;
 
-    private final ConcurrentHashMap<NodeId, DeviceContext> deviceContexts = new ConcurrentHashMap<>();
+    private final ConcurrentMap<NodeId, DeviceContext> deviceContexts = new ConcurrentHashMap<>();
     private final MessageIntelligenceAgency messageIntelligenceAgency;
 
     private final long barrierNanos = TimeUnit.MILLISECONDS.toNanos(500);
@@ -106,31 +107,19 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi
         // final phase - we have to add new Device to MD-SAL DataStore
         Preconditions.checkNotNull(deviceContext);
         try {
-
-            if (deviceContext.getDeviceState().getRole() != OfpRole.BECOMESLAVE) {
-                ((DeviceContextImpl) deviceContext).initialSubmitTransaction();
-                deviceContext.onPublished();
-
-            } else {
-                //if role = slave
-                try {
-                    ((DeviceContextImpl) deviceContext).cancelTransaction();
-                } catch (final Exception e) {
-                    //TODO: how can we avoid it. pingpong does not have cancel
-                    LOG.debug("Expected Exception: Cancel Txn exception thrown for slaves", e);
-                }
-
-            }
+            ((DeviceContextImpl) deviceContext).initialSubmitTransaction();
+            deviceContext.onPublished();
 
         } catch (final Exception e) {
             LOG.warn("Node {} can not be add to OPERATIONAL DataStore yet because {} ", deviceContext.getDeviceState().getNodeId(), e.getMessage());
             LOG.trace("Problem with add node {} to OPERATIONAL DataStore", deviceContext.getDeviceState().getNodeId(), e);
             try {
                 deviceContext.close();
-            } catch (final Exception e1) {
-                LOG.warn("Device context close FAIL - " + deviceContext.getDeviceState().getNodeId());
+            } catch (Exception e1) {
+                LOG.warn("Exception on device context close. ", e);
             }
         }
+
     }
 
     @Override
@@ -161,8 +150,9 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi
                 hashedWheelTimer, messageIntelligenceAgency, outboundQueueProvider, translatorLibrary);
 
         deviceContext.addDeviceContextClosedHandler(this);
-        deviceContexts.put(connectionContext.getNodeId(), deviceContext);
+        Verify.verify(deviceContexts.putIfAbsent(connectionContext.getNodeId(), deviceContext) == null);
 
+        // FIXME : txChainManager has to be propagate by Cluster MasterShip Election (remove this couple of lines)
         // We would like to crete/register TxChainManager after
         final DeviceTransactionChainManagerProvider.TransactionChainManagerRegistration txChainManagerReg = deviceTransactionChainManagerProvider
                 .provideTransactionChainManager(connectionContext);
@@ -174,12 +164,12 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi
             deviceContext.close();
             return;
         }
+        // --- remove end ----
 
         ((ExtensionConverterProviderKeeper) deviceContext).setExtensionConverterProvider(extensionConverterProvider);
         deviceContext.setNotificationService(notificationService);
         deviceContext.setNotificationPublishService(notificationPublishService);
 
-
         updatePacketInRateLimiters();
 
         final OpenflowProtocolListenerFullImpl messageListener = new OpenflowProtocolListenerFullImpl(

diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java
index 71d16b6c88b6d472cbf44ec747b141bb0b53ac5c..43e83ebc91b71b6752b05380d804df5878853ff8 100644 (file)
--- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java
+++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java
@@ -383,7 +383,6 @@ public class DeviceContextImplTest {
         deviceContext.close();
         verify(connectionContext).closeConnection(eq(false));
         verify(deviceState).setValid(eq(false));
-        verify(txChainManager).close();
         verify(mockedAuxiliaryConnectionContext).closeConnection(eq(false));
     }
 
@@ -493,6 +492,5 @@ public class DeviceContextImplTest {
         Assert.assertEquals(0, deviceContext.getDeviceGroupRegistry().getAllGroupIds().size());
         Assert.assertEquals(0, deviceContext.getDeviceMeterRegistry().getAllMeterIds().size());
 
-        Mockito.verify(txChainManager).close();
     }
 }