From: Martin Sunal <msunal@cisco.com>
Date: Tue, 28 Jun 2016 13:17:30 +0000 (+0200)
Subject: Fixed transaction chain in PortHandler
X-Git-Tag: release/boron~78
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=e1a0f58018b7b5163fae0155895c541176b7471a;p=groupbasedpolicy.git

Fixed transaction chain in PortHandler

- only TransactionChain is used for creating of transactions
- synchronized access for write/delete to datastore

Change-Id: Ifd4614516f9229bc7039a975d6b73b2caf87d9e6
Signed-off-by: Martin Sunal <msunal@cisco.com>
---

diff --git a/neutron-vpp-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandler.java b/neutron-vpp-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandler.java
index 74a0f6be9..687fd0b5c 100644
--- a/neutron-vpp-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandler.java
+++ b/neutron-vpp-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandler.java
@@ -11,13 +11,11 @@ package org.opendaylight.groupbasedpolicy.neutron.vpp.mapper.processors;
 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.ReadOnlyTransaction;
-import org.opendaylight.controller.md.sal.binding.api.ReadTransaction;
 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.groupbasedpolicy.neutron.vpp.mapper.SocketInfo;
 import org.opendaylight.groupbasedpolicy.util.DataStoreHelper;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid;
@@ -37,11 +35,11 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.por
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.ports.attributes.ports.Port;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.ports.attributes.ports.PortKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.rev150712.Neutron;
-import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology;
-import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyKey;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NodeId;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.TopologyId;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyKey;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeKey;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
@@ -72,9 +70,10 @@ public class PortHandler implements TransactionChainListener {
     }
 
     void processCreated(Port port) {
-        ReadTransaction rTx = dataBroker.newReadOnlyTransaction();
+        ReadOnlyTransaction rTx = transactionChain.newReadOnlyTransaction();
         Optional<BaseEndpointByPort> optBaseEpByPort = DataStoreHelper.readFromDs(LogicalDatastoreType.OPERATIONAL,
                 createBaseEpByPortIid(port.getUuid()), rTx);
+        rTx.close();
         if (!optBaseEpByPort.isPresent()) {
             return;
         }
@@ -82,9 +81,10 @@ public class PortHandler implements TransactionChainListener {
     }
 
     void processCreated(BaseEndpointByPort bebp) {
-        ReadTransaction rTx = dataBroker.newReadOnlyTransaction();
+        ReadOnlyTransaction rTx = transactionChain.newReadOnlyTransaction();
         Optional<Port> optPort = DataStoreHelper.readFromDs(LogicalDatastoreType.CONFIGURATION,
                 createPortIid(bebp.getPortId()), rTx);
+        rTx.close();
         if (!optPort.isPresent()) {
             return;
         }
@@ -120,7 +120,7 @@ public class PortHandler implements TransactionChainListener {
 
     void processUpdated(Port original, Port delta) {
         if (isValidVhostUser(original)) {
-            ReadOnlyTransaction rTx = dataBroker.newReadOnlyTransaction();
+            ReadOnlyTransaction rTx = transactionChain.newReadOnlyTransaction();
             Optional<BaseEndpointByPort> optBebp = DataStoreHelper.readFromDs(LogicalDatastoreType.OPERATIONAL,
                     createBaseEpByPortIid(original.getUuid()), rTx);
             rTx.close();
@@ -145,20 +145,14 @@ public class PortHandler implements TransactionChainListener {
         }
     }
 
-    private void writeVppEndpoint(InstanceIdentifier<VppEndpoint> vppEpIid, VppEndpoint vppEp) {
+    private synchronized void writeVppEndpoint(InstanceIdentifier<VppEndpoint> vppEpIid, VppEndpoint vppEp) {
         WriteTransaction wTx = transactionChain.newWriteOnlyTransaction();
         if (vppEp != null) {
             wTx.put(LogicalDatastoreType.CONFIGURATION, vppEpIid, vppEp, true);
         } else {
             wTx.delete(LogicalDatastoreType.CONFIGURATION, vppEpIid);
         }
-        try {
-            wTx.submit().checkedGet();
-        } catch (TransactionCommitFailedException e) {
-            LOG.error("Transaction chain commit failed. {}", e);
-            transactionChain.close();
-            transactionChain = dataBroker.createTransactionChain(this);
-        }
+        wTx.submit();
     }
 
     @VisibleForTesting
@@ -214,7 +208,8 @@ public class PortHandler implements TransactionChainListener {
     @Override
     public void onTransactionChainFailed(TransactionChain<?, ?> chain, AsyncTransaction<?, ?> transaction,
             Throwable cause) {
-        LOG.error("Transaction chain failed. {}", cause.getMessage());
+        LOG.error("Transaction chain failed. {} \nTransaction which caused the chain to fail {}", cause.getMessage(),
+                transaction, cause);
         transactionChain.close();
         transactionChain = dataBroker.createTransactionChain(this);
     }
diff --git a/neutron-vpp-mapper/src/test/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandlerTest.java b/neutron-vpp-mapper/src/test/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandlerTest.java
index 7180b26c0..5aae2bc85 100644
--- a/neutron-vpp-mapper/src/test/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandlerTest.java
+++ b/neutron-vpp-mapper/src/test/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandlerTest.java
@@ -10,12 +10,10 @@ package org.opendaylight.groupbasedpolicy.neutron.vpp.mapper.processors;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertNotNull;
-import static org.mockito.Mockito.any;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import org.junit.Before;
@@ -142,8 +140,6 @@ public class PortHandlerTest extends AbstractDataBrokerTest {
         ReadOnlyTransaction rTx = dataBroker.newReadOnlyTransaction();
         Optional<VppEndpoint> optVppEp = DataStoreHelper.readFromDs(LogicalDatastoreType.CONFIGURATION,
                 TestUtils.createVppEpIid(TestUtils.createVppEndpointKey(bebp)), rTx);
-        verify(transactionChain).newReadOnlyTransaction();
-        verify(transactionChain, times(2)).newWriteOnlyTransaction();
         assertTrue(optVppEp.isPresent());
     }
 
@@ -158,10 +154,6 @@ public class PortHandlerTest extends AbstractDataBrokerTest {
         ReadOnlyTransaction rTx = dataBroker.newReadOnlyTransaction();
         Optional<VppEndpoint> optVppEp = DataStoreHelper.readFromDs(LogicalDatastoreType.CONFIGURATION,
                 TestUtils.createVppEpIid(TestUtils.createVppEndpointKey(bebp)), rTx);
-        // looks for existing Vpp endpoint
-        verify(transactionChain).newReadOnlyTransaction();
-        // only removes former valid vpp endpoint
-        verify(transactionChain).newWriteOnlyTransaction();
         assertFalse(optVppEp.isPresent());
     }
 
@@ -175,8 +167,6 @@ public class PortHandlerTest extends AbstractDataBrokerTest {
         Optional<VppEndpoint> optVppEp = DataStoreHelper.readFromDs(LogicalDatastoreType.CONFIGURATION,
                 TestUtils.createVppEpIid(TestUtils.createVppEndpointKey(bebp)), rTx);
         assertFalse(optVppEp.isPresent());
-        verify(transactionChain).newReadOnlyTransaction();
-        verify(transactionChain).newWriteOnlyTransaction();
     }
 
     private void putVppEp(Port port, BaseEndpointByPort bebp, WriteTransaction rwTx) {