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: If0c51be8fe80106a6722abb93dbe4b47548fce1b
Signed-off-by: adetalhouet <adetalhouet@inocybe.com>
(cherry picked from commit 76cce56c2bf85efde7f360f2af779a75b57141ee)

diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDatastoreAdapter.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDatastoreAdapter.java
index 9b499c03b3c7d54df3413dbd96bb5243d40ed34a..54dda9ddb8acf76d7885a7a47576db09761cb2ae 100644 (file)
--- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDatastoreAdapter.java
+++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/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.netconf.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/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProvider.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProvider.java
index 32a5abd3541deb35fdf8c3d6f0543d09d5168b28..a01b74e94149bdd78c673bcae856900374154b28 100644 (file)
--- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProvider.java
+++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProvider.java
@@ -8,9 +8,15 @@
 package org.opendaylight.netconf.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 @@ public class NetconfDeviceSalProvider implements AutoCloseable, Provider, Bindin
 
     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 @@ public class NetconfDeviceSalProvider implements AutoCloseable, Provider, Bindin
     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 @@ public class NetconfDeviceSalProvider implements AutoCloseable, Provider, Bindin
         datastoreAdapter = null;
         topologyDatastoreAdapter.close();
         topologyDatastoreAdapter = null;
+        txChain.close();
     }
 
     public static final class MountInstance implements AutoCloseable {

diff --git a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceTopologyAdapter.java b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceTopologyAdapter.java
index b9f009177737b4e7e20e7b4d409a5047c811f9da..bb307439b0aa6984071d1b5f967e7ed417a3f3d9 100644 (file)
--- a/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceTopologyAdapter.java
+++ b/opendaylight/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/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.netconf.sal.connect.netconf.listener.NetconfDeviceCapabilities;
 import org.opendaylight.netconf.sal.connect.util.RemoteDeviceId;
@@ -74,27 +73,15 @@ final class NetconfDeviceTopologyAdapter implements AutoCloseable {
     };
 
     private final RemoteDeviceId id;
-    private final BindingTransactionChain txChain;
+    private BindingTransactionChain txChain;
 
     private final InstanceIdentifier<NetworkTopology> networkTopologyPath;
     private final KeyedInstanceIdentifier<Topology, TopologyKey> 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) {
-                LOG.error("{}: TransactionChain({}) {} FAILED!", id, chain,
-                        transaction.getIdentifier(), cause);
-                throw new IllegalStateException(id + "  TransactionChain(" + chain + ") not committed correctly", cause);
-            }
-
-            @Override
-            public void onTransactionChainSuccessful(TransactionChain<?, ?> chain) {
-                LOG.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())));
@@ -201,7 +188,12 @@ final class NetconfDeviceTopologyAdapter implements AutoCloseable {
                 "{}: Close device state transaction {} removing all data ended.",
                 id, writeTx.getIdentifier());
 
-        commitTransaction(writeTx, "close");
+        try {
+            writeTx.submit().get();
+        } catch (InterruptedException | ExecutionException e) {
+            LOG.error("{}: Transaction(close) {} FAILED!", id, writeTx.getIdentifier(), e);
+            throw new IllegalStateException(id + "  Transaction(close) not committed correctly", e);
+        }
     }
 
     private void createNetworkTopologyIfNotPresent(final WriteTransaction writeTx) {
@@ -253,6 +245,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/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceTopologyAdapterTest.java b/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceTopologyAdapterTest.java
index e5ab553934cc18a6e9a13d3b377a300905fe1823..c3e4d0d4ac70dccbe3642a42f4b3ceef63226509 100644 (file)
--- a/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceTopologyAdapterTest.java
+++ b/opendaylight/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/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();