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 <adetalhouet@inocybe.com>

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 48b41da3fa83d617211f1b7850a8380708377a4d..8dd0c3e92efb971152a5b123bb0e71e80b0a2970 100644 (file)
--- 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 bf870bf36a07ccd421f86e9eeba1571ad73f0e5e..9c78ee3c5529fa239e1c3776c6b53e046c41155c 100644 (file)
--- 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 7822ab725f6b0eddd596887af580536c462130bb..df7230228dbb4450a665aaa375da0d90ef3ce651 100644 (file)
--- 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<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) {
-                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 158c4c43f098d0843cf24ec97b3f9fabc6a69831..29662aabe4ddc74e4fee14548e8a730051eef3c5 100644 (file)
--- 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();