From 88786c967320e41af534cb1bf340aadec2c3ea2b Mon Sep 17 00:00:00 2001
From: Martin Bobak
Date: Fri, 5 Jun 2015 15:19:52 +0200
Subject: [PATCH] Bug 3599 - Li - transaction chain manager should not
propagate connection context in notification
Change-Id: Ia03c152cec6c7ea3afbb29e5778b53cc1d80fd83
Signed-off-by: Martin Bobak
(cherry picked from commit 571b41953805ad4c14abbbd031fc86f4dfccf747)
---
.../impl/device/DeviceManagerImpl.java | 13 +++---
...DeviceTransactionChainManagerProvider.java | 40 +++++++++++--------
.../ReadyForNewTransactionChainHandler.java | 4 +-
...eadyForNewTransactionChainHandlerImpl.java | 31 ++++++++++++++
.../impl/device/TransactionChainManager.java | 25 +++++-------
.../impl/device/DeviceContextImplTest.java | 8 ++--
.../device/TransactionChainManagerTest.java | 7 ++--
7 files changed, 80 insertions(+), 48 deletions(-)
create mode 100644 openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/ReadyForNewTransactionChainHandlerImpl.java
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 d889bd066b..176311cce9 100644
--- 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 703fdf583d..b453c4c9de 100644
--- 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 58b22b5dac..255e4cf5e8 100644
--- 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
index 0000000000..76ead2f63c
--- /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 4aad2d945a..8bcad4e1f2 100644
--- 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
- *
+ *
* 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 Vaclav Demcak
- *
+ *
* 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 nodeII;
- private final ConnectionContext connectionContext;
private Registration managerRegistration;
TransactionChainManager(@Nonnull final DataBroker dataBroker,
- @Nonnull final ConnectionContext connectionContext,
+ @Nonnull final KeyedInstanceIdentifier 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 538176c0f2..704f1847b8 100644
--- 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 nodeKeyIdent;
+ NodeId nodeId = new NodeId("h2g2:42");
+ KeyedInstanceIdentifier 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> settableFuture = SettableFuture.create();
final SettableFuture> 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 81180fa562..5bbb2407a6 100644
--- 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.immediateCheckedFuture(null));
}
--
2.36.6