Bug 3599 - Li - transaction chain manager should not propagate connection context...

Change-Id: Ia03c152cec6c7ea3afbb29e5778b53cc1d80fd83
Signed-off-by: Martin Bobak <mbobak@cisco.com>

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 d889bd066b3372ea0e42cc28a0d49f611385abb9..176311cce9c8bd46c464b7804b66a4971b4b610b 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
@@ -102,7 +102,7 @@ import org.slf4j.LoggerFactory;
 /**
  *
  */
-public class DeviceManagerImpl implements DeviceManager, AutoCloseable, ReadyForNewTransactionChainHandler {
+public class DeviceManagerImpl implements DeviceManager, AutoCloseable {
 
     private static final Logger LOG = LoggerFactory.getLogger(DeviceManagerImpl.class);
 
@@ -176,7 +176,10 @@ public class DeviceManagerImpl implements DeviceManager, AutoCloseable, ReadyFor
     public void deviceConnected(@CheckForNull final ConnectionContext connectionContext) {
         Preconditions.checkArgument(connectionContext != null);
 
-        TransactionChainManager transactionChainManager = deviceTransactionChainManagerProvider.provideTransactionChainManagerOrWaitForNotification(connectionContext, dataBroker, this);
+        ReadyForNewTransactionChainHandler readyForNewTransactionChainHandler = new ReadyForNewTransactionChainHandlerImpl(this, connectionContext);
+        TransactionChainManager transactionChainManager = deviceTransactionChainManagerProvider.provideTransactionChainManagerOrWaitForNotification(connectionContext,
+                dataBroker,
+                readyForNewTransactionChainHandler);
         initializeDeviceContextSafely(connectionContext, transactionChainManager);
     }
 
@@ -600,12 +603,6 @@ public class DeviceManagerImpl implements DeviceManager, AutoCloseable, ReadyFor
         spyPool.scheduleAtFixedRate(messageIntelligenceAgency, spyRate, spyRate, TimeUnit.SECONDS);
     }
 
-    @Override
-    public void onReadyForNewTransactionChain(final ConnectionContext connectionContext) {
-        TransactionChainManager transactionChainManager = deviceTransactionChainManagerProvider.provideTransactionChainManagerOrWaitForNotification(connectionContext, dataBroker, this);
-        initializeDeviceContextSafely(connectionContext, transactionChainManager);
-    }
-
     private void initializeDeviceContextSafely(final ConnectionContext connectionContext, final TransactionChainManager transactionChainManager) {
         if (null != transactionChainManager) {
             initializeDeviceContext(connectionContext, transactionChainManager);

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceTransactionChainManagerProvider.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceTransactionChainManagerProvider.java
index 703fdf583d76e3cab61935afe0d09b087cf071eb..b453c4c9de744aab9150f56e488828ba37e49782 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceTransactionChainManagerProvider.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceTransactionChainManagerProvider.java
@@ -12,6 +12,7 @@ import java.util.HashMap;
 import java.util.Map;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext;
+import org.opendaylight.openflowplugin.impl.util.DeviceStateUtil;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
 import org.opendaylight.yangtools.concepts.Registration;
 import org.slf4j.Logger;
@@ -40,30 +41,35 @@ public class DeviceTransactionChainManagerProvider {
                         txChManagers.remove(nodeId);
                     }
                 };
-                transactionChainManager = new TransactionChainManager(dataBroker, connectionContext, registration);
-                txChManagers.put(nodeId, transactionChainManager);
+                transactionChainManager = new TransactionChainManager(dataBroker,
+                        DeviceStateUtil.createNodeInstanceIdentifier(connectionContext.getNodeId()),
+                        registration);
+                synchronized (txChManagers) {
+                    TransactionChainManager possibleValue = txChManagers.get(nodeId);
+                    //some other thread managed to register its transaction chain manager before this block ended - drop this one
+                    if (null == possibleValue) {
+                        txChManagers.put(nodeId, transactionChainManager);
+                    } else {
+                        dropNewConnection(connectionContext, nodeId, transactionChainManager);
+                    }
+                }
                 return transactionChainManager;
             } else {
-                try {
-                    if (!transactionChainManager.attemptToRegisterHandler(readyForNewTransactionChainHandler)) {
-                        if (TransactionChainManager.TransactionChainManagerStatus.WORKING.equals(transactionChainManager.getTransactionChainManagerStatus())) {
-                            LOG.info("There already exists one handler for connection described as {}. Connection is working will not try again.", nodeId);
-                            connectionContext.closeConnection(false);
-                        } else {
-                            LOG.info("There already exists one handler for connection described as {}. Transaction chain manager is in state {}. Will try again.",
-                                    nodeId,
-                                    transactionChainManager.getTransactionChainManagerStatus());
-                            readyForNewTransactionChainHandler.onReadyForNewTransactionChain(connectionContext);
-                        }
-                    }
-                } catch (Exception e) {
-                    LOG.info("Transaction closed handler registration for node {} failed because we most probably hit previous transaction chain  manager's close process. Will try again.", nodeId);
-                    readyForNewTransactionChainHandler.onReadyForNewTransactionChain(connectionContext);
+                if (!transactionChainManager.attemptToRegisterHandler(readyForNewTransactionChainHandler)) {
+                    dropNewConnection(connectionContext, nodeId, transactionChainManager);
                 }
             }
         }
+
         return null;
     }
 
+    private void dropNewConnection(final ConnectionContext connectionContext, final NodeId nodeId, final TransactionChainManager transactionChainManager) {
+        LOG.info("There already exists one handler for connection described as {}. Transaction chain manager is in state {}. Will not try again.",
+                nodeId,
+                transactionChainManager.getTransactionChainManagerStatus());
+        connectionContext.closeConnection(false);
+    }
+
 
 }

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/ReadyForNewTransactionChainHandler.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/ReadyForNewTransactionChainHandler.java
index 58b22b5daca4b006d864940a30c2ed600c89877a..255e4cf5e8d144777cba11e8eb51577e3d8cd5de 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/ReadyForNewTransactionChainHandler.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/ReadyForNewTransactionChainHandler.java
@@ -8,12 +8,10 @@
 
 package org.opendaylight.openflowplugin.impl.device;
 
-import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext;
-
 /**
  * Created by Martin Bobak <mbobak@cisco.com> on 2.6.2015.
  */
 public interface ReadyForNewTransactionChainHandler {
 
-    void onReadyForNewTransactionChain(ConnectionContext connectionContext);
+    void onReadyForNewTransactionChain();
 }

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/ReadyForNewTransactionChainHandlerImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/ReadyForNewTransactionChainHandlerImpl.java
new file mode 100644 (file)
index 0000000..76ead2f
--- /dev/null
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/ReadyForNewTransactionChainHandlerImpl.java
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2015 Cisco Systems, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.opendaylight.openflowplugin.impl.device;
+
+import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext;
+import org.opendaylight.openflowplugin.api.openflow.device.DeviceManager;
+
+/**
+ * Created by Martin Bobak <mbobak@cisco.com> on 5.6.2015.
+ */
+public class ReadyForNewTransactionChainHandlerImpl implements ReadyForNewTransactionChainHandler {
+
+    private final DeviceManager deviceManager;
+    private final ConnectionContext connectionContext;
+
+    public ReadyForNewTransactionChainHandlerImpl(final DeviceManager deviceManager, final ConnectionContext connectionContext) {
+        this.deviceManager = deviceManager;
+        this.connectionContext = connectionContext;
+    }
+
+    @Override
+    public void onReadyForNewTransactionChain() {
+        deviceManager.deviceConnected(connectionContext);
+    }
+}

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManager.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManager.java
index 4aad2d945ad2befd68cb4f0f09aa1feafcad6aa0..8bcad4e1f22e42040ab50456f0d01e78726ad905 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManager.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManager.java
@@ -22,8 +22,6 @@ import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionChain;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionChainListener;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
-import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext;
-import org.opendaylight.openflowplugin.impl.util.DeviceStateUtil;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey;
 import org.opendaylight.yangtools.concepts.Registration;
@@ -36,14 +34,14 @@ import org.slf4j.LoggerFactory;
 /**
  * openflowplugin-impl
  * org.opendaylight.openflowplugin.impl.device
- * <p/>
+ * <p>
  * Package protected class for controlling {@link WriteTransaction} life cycle. It is
  * a {@link TransactionChainListener} and provide package protected methods for writeToTransaction
  * method (wrapped {@link WriteTransaction#put(LogicalDatastoreType, InstanceIdentifier, DataObject)})
  * and submitTransaction method (wrapped {@link WriteTransaction#submit()})
  *
  * @author <a href="mailto:vdemcak@cisco.com">Vaclav Demcak</a>
- *         <p/>
+ *         </p>
  *         Created: Apr 2, 2015
  */
 class TransactionChainManager implements TransactionChainListener, AutoCloseable {
@@ -64,15 +62,13 @@ class TransactionChainManager implements TransactionChainListener, AutoCloseable
     private TransactionChainManagerStatus transactionChainManagerStatus;
     private ReadyForNewTransactionChainHandler readyForNewTransactionChainHandler;
     private final KeyedInstanceIdentifier<Node, NodeKey> nodeII;
-    private final ConnectionContext connectionContext;
     private Registration managerRegistration;
 
     TransactionChainManager(@Nonnull final DataBroker dataBroker,
-                            @Nonnull final ConnectionContext connectionContext,
+                            @Nonnull final KeyedInstanceIdentifier<Node, NodeKey> nodeII,
                             @Nonnull final Registration managerRegistration) {
         this.dataBroker = Preconditions.checkNotNull(dataBroker);
-        this.nodeII = Preconditions.checkNotNull(DeviceStateUtil.createNodeInstanceIdentifier(connectionContext.getNodeId()));
-        this.connectionContext = Preconditions.checkNotNull(connectionContext);
+        this.nodeII = Preconditions.checkNotNull(nodeII);
         this.managerRegistration = Preconditions.checkNotNull(managerRegistration);
         createTxChain(dataBroker);
         LOG.debug("created txChainManager");
@@ -88,11 +84,12 @@ class TransactionChainManager implements TransactionChainListener, AutoCloseable
         submitWriteTransaction();
     }
 
-    public boolean attemptToRegisterHandler(final ReadyForNewTransactionChainHandler readyForNewTransactionChainHandler) {
-        if (null == this.readyForNewTransactionChainHandler) {
-            synchronized (this) {
-                Preconditions.checkState(null != this.managerRegistration);
-                this.readyForNewTransactionChainHandler = readyForNewTransactionChainHandler;
+    public synchronized boolean attemptToRegisterHandler(final ReadyForNewTransactionChainHandler readyForNewTransactionChainHandler) {
+        if (TransactionChainManagerStatus.SHUTTING_DOWN.equals(this.transactionChainManagerStatus)
+                && null == this.readyForNewTransactionChainHandler) {
+            this.readyForNewTransactionChainHandler = readyForNewTransactionChainHandler;
+            if (managerRegistration == null) {
+                this.readyForNewTransactionChainHandler.onReadyForNewTransactionChain();
             }
             return true;
         } else {
@@ -199,7 +196,7 @@ class TransactionChainManager implements TransactionChainListener, AutoCloseable
     private void notifyReadyForNewTransactionChainAndCloseFactory() {
         synchronized (this) {
             if (null != readyForNewTransactionChainHandler) {
-                readyForNewTransactionChainHandler.onReadyForNewTransactionChain(connectionContext);
+                readyForNewTransactionChainHandler.onReadyForNewTransactionChain();
             }
             try {
                 LOG.debug("Closing registration in manager.");

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 538176c0f21b4f035dab42ef90ae29a2964c2e40..704f1847b882de1d7dd6b508bc5a5274202f5027 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
@@ -32,6 +32,8 @@ import org.opendaylight.openflowplugin.api.openflow.device.RequestContext;
 import org.opendaylight.openflowplugin.api.openflow.device.TranslatorLibrary;
 import org.opendaylight.openflowplugin.api.openflow.device.Xid;
 import org.opendaylight.openflowplugin.api.openflow.statistics.ofpspecific.MessageIntelligenceAgency;
+import org.opendaylight.openflowplugin.impl.util.DeviceStateUtil;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.GetAsyncReply;
@@ -75,8 +77,8 @@ public class DeviceContextImplTest {
     OutboundQueueProvider outboundQueueProvider;
     @Mock
     ConnectionAdapter connectionAdapter;
-    @Mock
-    KeyedInstanceIdentifier<Node, NodeKey> nodeKeyIdent;
+    NodeId nodeId = new NodeId("h2g2:42");
+    KeyedInstanceIdentifier<Node, NodeKey> nodeKeyIdent = DeviceStateUtil.createNodeInstanceIdentifier(nodeId);
     @Mock
     TranslatorLibrary translatorLibrary;
     @Mock
@@ -91,7 +93,7 @@ public class DeviceContextImplTest {
         Mockito.when(dataBroker.newReadOnlyTransaction()).thenReturn(rTx);
         Mockito.when(dataBroker.createTransactionChain(Mockito.any(TransactionChainManager.class))).thenReturn(txChainFactory);
         Mockito.when(deviceState.getNodeInstanceIdentifier()).thenReturn(nodeKeyIdent);
-        txChainManager = new TransactionChainManager(dataBroker, connectionContext, registration);
+        txChainManager = new TransactionChainManager(dataBroker, nodeKeyIdent, registration);
         final SettableFuture<RpcResult<GetAsyncReply>> settableFuture = SettableFuture.create();
         final SettableFuture<RpcResult<MultipartReply>> settableFutureMultiReply = SettableFuture.create();
         Mockito.when(requestContext.getFuture()).thenReturn(settableFuture);

diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManagerTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManagerTest.java
index 81180fa562da8261418eb7d086f311885e8effc0..5bbb2407a6979310375b77053e5ea5bf6aaa5654 100644 (file)
--- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManagerTest.java
+++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManagerTest.java
@@ -32,6 +32,7 @@ import org.opendaylight.controller.md.sal.common.api.data.TransactionChain;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionChainListener;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
 import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext;
+import org.opendaylight.openflowplugin.impl.util.DeviceStateUtil;
 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.nodes.Node;
@@ -77,12 +78,12 @@ public class TransactionChainManagerTest {
         Mockito.when(dataBroker.newReadOnlyTransaction()).thenReturn(readOnlyTx);
         Mockito.when(dataBroker.createTransactionChain(Matchers.any(TransactionChainListener.class)))
                 .thenReturn(txChain);
-        txChainManager = new TransactionChainManager(dataBroker, connectionContext, registration);
+        nodeId = new NodeId("h2g2:42");
+        nodeKeyIdent = DeviceStateUtil.createNodeInstanceIdentifier(nodeId);
+        txChainManager = new TransactionChainManager(dataBroker, nodeKeyIdent, registration);
         Mockito.when(txChain.newWriteOnlyTransaction()).thenReturn(writeTx);
 
-        nodeId = new NodeId("h2g2:42");
         path = InstanceIdentifier.create(Nodes.class).child(Node.class, new NodeKey(nodeId));
-
         Mockito.when(writeTx.submit()).thenReturn(Futures.<Void, TransactionCommitFailedException>immediateCheckedFuture(null));
     }