From 2bde01e8aa670be6573d433d894b2ee523470ba8 Mon Sep 17 00:00:00 2001 From: adetalhouet Date: Mon, 11 Jan 2016 13:02:20 -0500 Subject: [PATCH] Bug 4114 - netconf connector replace causes transaction chain failure In order to avoid race condition when txChain perform delete then write transactions in a row to the same path, perfom the delete as a blocking transaction so once done, write can be perform without any race condition Also reset the txChain after a failure. Change-Id: Ic48d2993318d22f7af2e6aba49d718677a05ea3d Signed-off-by: adetalhouet --- .../sal/NetconfDeviceDatastoreAdapter.java | 36 +++++++--------- .../netconf/sal/NetconfDeviceSalProvider.java | 42 +++++++++++++++++-- .../sal/NetconfDeviceTopologyAdapter.java | 36 +++++++--------- .../sal/NetconfDeviceTopologyAdapterTest.java | 4 +- 4 files changed, 73 insertions(+), 45 deletions(-) diff --git a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceDatastoreAdapter.java b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceDatastoreAdapter.java index 48b41da3fa..8dd0c3e92e 100644 --- a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceDatastoreAdapter.java +++ b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceDatastoreAdapter.java @@ -15,15 +15,14 @@ import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; + import java.util.Set; +import java.util.concurrent.ExecutionException; + import org.opendaylight.controller.md.sal.binding.api.BindingTransactionChain; -import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; -import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction; 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.controller.sal.connect.util.RemoteDeviceId; import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes; @@ -50,22 +49,11 @@ final class NetconfDeviceDatastoreAdapter implements AutoCloseable { private static final Logger logger = LoggerFactory.getLogger(NetconfDeviceDatastoreAdapter.class); private final RemoteDeviceId id; - private final BindingTransactionChain txChain; + private BindingTransactionChain txChain; - NetconfDeviceDatastoreAdapter(final RemoteDeviceId deviceId, final DataBroker dataService) { + NetconfDeviceDatastoreAdapter(final RemoteDeviceId deviceId, final BindingTransactionChain txChain) { this.id = Preconditions.checkNotNull(deviceId); - this.txChain = Preconditions.checkNotNull(dataService).createTransactionChain(new TransactionChainListener() { - @Override - public void onTransactionChainFailed(TransactionChain chain, AsyncTransaction transaction, Throwable cause) { - logger.error("{}: TransactionChain({}) {} FAILED!", id, chain, transaction.getIdentifier(), cause); - throw new IllegalStateException(id + " TransactionChain(" + chain + ") not committed correctly", cause); - } - - @Override - public void onTransactionChainSuccessful(TransactionChain chain) { - logger.trace("{}: TransactionChain({}) {} SUCCESSFUL", id, chain); - } - }); + this.txChain = Preconditions.checkNotNull(txChain); initDeviceData(); } @@ -89,7 +77,12 @@ final class NetconfDeviceDatastoreAdapter implements AutoCloseable { transaction.delete(LogicalDatastoreType.OPERATIONAL, id.getBindingPath()); logger.trace("{}: Close device state transaction {} removing all data ended.", id, transaction.getIdentifier()); - commitTransaction(transaction, "close"); + try { + transaction.submit().get(); + } catch (InterruptedException | ExecutionException e) { + logger.error("{}: Transaction(close) {} FAILED!", id, transaction.getIdentifier(), e); + throw new IllegalStateException(id + " Transaction(close) not committed correctly", e); + } } private void initDeviceData() { @@ -141,7 +134,6 @@ final class NetconfDeviceDatastoreAdapter implements AutoCloseable { @Override public void close() throws Exception { removeDeviceConfigAndState(); - txChain.close(); } public static org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node buildDataForDeviceState( @@ -180,4 +172,8 @@ final class NetconfDeviceDatastoreAdapter implements AutoCloseable { nodeBuilder.setId(id.getBindingKey().getId()); return nodeBuilder; } + + public void setTxChain(BindingTransactionChain txChain) { + this.txChain = Preconditions.checkNotNull(txChain); + } } diff --git a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceSalProvider.java b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceSalProvider.java index bf870bf36a..9c78ee3c55 100644 --- a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceSalProvider.java +++ b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceSalProvider.java @@ -8,9 +8,15 @@ package org.opendaylight.controller.sal.connect.netconf.sal; import com.google.common.base.Preconditions; + import java.util.Collection; import java.util.Collections; + +import org.opendaylight.controller.md.sal.binding.api.BindingTransactionChain; import org.opendaylight.controller.md.sal.binding.api.DataBroker; +import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction; +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.dom.api.DOMDataBroker; import org.opendaylight.controller.md.sal.dom.api.DOMMountPoint; import org.opendaylight.controller.md.sal.dom.api.DOMMountPointService; @@ -37,6 +43,24 @@ final class NetconfDeviceSalProvider implements AutoCloseable, Provider, Binding private volatile NetconfDeviceTopologyAdapter topologyDatastoreAdapter; + private DataBroker dataBroker; + private BindingTransactionChain txChain; + + private final TransactionChainListener transactionChainListener = new TransactionChainListener() { + @Override + public void onTransactionChainFailed(TransactionChain chain, AsyncTransaction transaction, Throwable cause) { + logger.error("{}: TransactionChain({}) {} FAILED!", id, chain, transaction.getIdentifier(), cause); + chain.close(); + resetTransactionChainForAdapaters(); + throw new IllegalStateException(id + " TransactionChain(" + chain + ") not committed correctly", cause); + } + + @Override + public void onTransactionChainSuccessful(TransactionChain chain) { + logger.trace("{}: TransactionChain({}) {} SUCCESSFUL", id, chain); + } + }; + public NetconfDeviceSalProvider(final RemoteDeviceId deviceId) { this.id = deviceId; } @@ -78,10 +102,21 @@ final class NetconfDeviceSalProvider implements AutoCloseable, Provider, Binding public void onSessionInitiated(final BindingAwareBroker.ProviderContext session) { logger.debug("{}: Session with sal established {}", id, session); - final DataBroker dataBroker = session.getSALService(DataBroker.class); - datastoreAdapter = new NetconfDeviceDatastoreAdapter(id, dataBroker); + this.dataBroker = session.getSALService(DataBroker.class); + txChain = Preconditions.checkNotNull(dataBroker).createTransactionChain(transactionChainListener); + + datastoreAdapter = new NetconfDeviceDatastoreAdapter(id, txChain); + topologyDatastoreAdapter = new NetconfDeviceTopologyAdapter(id, txChain); + } + + private void resetTransactionChainForAdapaters() { + txChain = Preconditions.checkNotNull(dataBroker).createTransactionChain(transactionChainListener); + + datastoreAdapter.setTxChain(txChain); + topologyDatastoreAdapter.setTxChain(txChain); + + logger.trace("{}: Resetting TransactionChain {}", id, txChain); - topologyDatastoreAdapter = new NetconfDeviceTopologyAdapter(id, dataBroker); } public void close() throws Exception { @@ -90,6 +125,7 @@ final class NetconfDeviceSalProvider implements AutoCloseable, Provider, Binding datastoreAdapter = null; topologyDatastoreAdapter.close(); topologyDatastoreAdapter = null; + txChain.close(); } static final class MountInstance implements AutoCloseable { diff --git a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceTopologyAdapter.java b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceTopologyAdapter.java index 7822ab725f..df7230228d 100644 --- a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceTopologyAdapter.java +++ b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceTopologyAdapter.java @@ -14,16 +14,15 @@ import com.google.common.collect.FluentIterable; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; + import java.util.ArrayList; import java.util.List; import java.util.Map.Entry; +import java.util.concurrent.ExecutionException; + import org.opendaylight.controller.md.sal.binding.api.BindingTransactionChain; -import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; -import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction; 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.controller.sal.connect.netconf.listener.NetconfDeviceCapabilities; import org.opendaylight.controller.sal.connect.util.RemoteDeviceId; @@ -74,26 +73,15 @@ final class NetconfDeviceTopologyAdapter implements AutoCloseable { }; private final RemoteDeviceId id; - private final BindingTransactionChain txChain; + private BindingTransactionChain txChain; private final InstanceIdentifier networkTopologyPath; private final KeyedInstanceIdentifier topologyListPath; private static final String UNKNOWN_REASON = "Unknown reason"; - NetconfDeviceTopologyAdapter(final RemoteDeviceId id, final DataBroker dataService) { + NetconfDeviceTopologyAdapter(final RemoteDeviceId id, final BindingTransactionChain txChain) { this.id = id; - this.txChain = Preconditions.checkNotNull(dataService).createTransactionChain(new TransactionChainListener() { - @Override - public void onTransactionChainFailed(TransactionChain chain, AsyncTransaction transaction, Throwable cause) { - logger.error("{}: TransactionChain({}) {} FAILED!", id, chain, transaction.getIdentifier(), cause); - throw new IllegalStateException(id + " TransactionChain(" + chain + ") not committed correctly", cause); - } - - @Override - public void onTransactionChainSuccessful(TransactionChain chain) { - logger.trace("{}: TransactionChain({}) {} SUCCESSFUL", id, chain); - } - }); + this.txChain = Preconditions.checkNotNull(txChain); this.networkTopologyPath = InstanceIdentifier.builder(NetworkTopology.class).build(); this.topologyListPath = networkTopologyPath.child(Topology.class, new TopologyKey(new TopologyId(TopologyNetconf.QNAME.getLocalName()))); @@ -182,7 +170,12 @@ final class NetconfDeviceTopologyAdapter implements AutoCloseable { writeTx.delete(LogicalDatastoreType.OPERATIONAL, id.getTopologyBindingPath()); logger.trace("{}: Close device state transaction {} removing all data ended.", id, writeTx.getIdentifier()); - commitTransaction(writeTx, "close"); + try { + writeTx.submit().get(); + } catch (InterruptedException | ExecutionException e) { + logger.error("{}: Transaction(close) {} FAILED!", id, writeTx.getIdentifier(), e); + throw new IllegalStateException(id + " Transaction(close) not committed correctly", e); + } } private void createNetworkTopologyIfNotPresent(final WriteTransaction writeTx) { @@ -231,6 +224,9 @@ final class NetconfDeviceTopologyAdapter implements AutoCloseable { @Override public void close() throws Exception { removeDeviceConfiguration(); - txChain.close(); + } + + public void setTxChain(BindingTransactionChain txChain) { + this.txChain = Preconditions.checkNotNull(txChain); } } diff --git a/opendaylight/md-sal/sal-netconf-connector/src/test/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceTopologyAdapterTest.java b/opendaylight/md-sal/sal-netconf-connector/src/test/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceTopologyAdapterTest.java index 158c4c43f0..29662aabe4 100644 --- a/opendaylight/md-sal/sal-netconf-connector/src/test/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceTopologyAdapterTest.java +++ b/opendaylight/md-sal/sal-netconf-connector/src/test/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceTopologyAdapterTest.java @@ -60,7 +60,7 @@ public class NetconfDeviceTopologyAdapterTest { public void testFailedDevice() throws Exception { doReturn(Futures.immediateCheckedFuture(null)).when(writeTx).submit(); - NetconfDeviceTopologyAdapter adapter = new NetconfDeviceTopologyAdapter(id, broker); + NetconfDeviceTopologyAdapter adapter = new NetconfDeviceTopologyAdapter(id, txChain); adapter.setDeviceAsFailed(null); verify(txChain, times(2)).newWriteOnlyTransaction(); @@ -71,7 +71,7 @@ public class NetconfDeviceTopologyAdapterTest { public void testDeviceUpdate() throws Exception { doReturn(Futures.immediateCheckedFuture(null)).when(writeTx).submit(); - NetconfDeviceTopologyAdapter adapter = new NetconfDeviceTopologyAdapter(id, broker); + NetconfDeviceTopologyAdapter adapter = new NetconfDeviceTopologyAdapter(id, txChain); adapter.updateDeviceData(true, new NetconfDeviceCapabilities()); verify(txChain, times(2)).newWriteOnlyTransaction(); -- 2.36.6