From 571b41953805ad4c14abbbd031fc86f4dfccf747 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 --- .../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