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 74a0f6be9b417fb226ef8eb20825fd12e34ff5de..687fd0b5c1d4997cde97235855bd50fb1e2479f1 100644 (file)
--- 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;\r
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;\r
 import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;\r
-import org.opendaylight.controller.md.sal.binding.api.ReadTransaction;\r
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;\r
 import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction;\r
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;\r
 import org.opendaylight.controller.md.sal.common.api.data.TransactionChain;\r
 import org.opendaylight.controller.md.sal.common.api.data.TransactionChainListener;\r
-import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;\r
 import org.opendaylight.groupbasedpolicy.neutron.vpp.mapper.SocketInfo;\r
 import org.opendaylight.groupbasedpolicy.util.DataStoreHelper;\r
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid;\r
@@ -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;\r
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.ports.attributes.ports.PortKey;\r
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.rev150712.Neutron;\r
-import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology;\r
-import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyKey;\r
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology;\r
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NodeId;\r
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.TopologyId;\r
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology;\r
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyKey;\r
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;\r
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeKey;\r
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;\r
@@ -72,9 +70,10 @@ public class PortHandler implements TransactionChainListener {
     }\r
 \r
     void processCreated(Port port) {\r
-        ReadTransaction rTx = dataBroker.newReadOnlyTransaction();\r
+        ReadOnlyTransaction rTx = transactionChain.newReadOnlyTransaction();\r
         Optional<BaseEndpointByPort> optBaseEpByPort = DataStoreHelper.readFromDs(LogicalDatastoreType.OPERATIONAL,\r
                 createBaseEpByPortIid(port.getUuid()), rTx);\r
+        rTx.close();\r
         if (!optBaseEpByPort.isPresent()) {\r
             return;\r
         }\r
@@ -82,9 +81,10 @@ public class PortHandler implements TransactionChainListener {
     }\r
 \r
     void processCreated(BaseEndpointByPort bebp) {\r
-        ReadTransaction rTx = dataBroker.newReadOnlyTransaction();\r
+        ReadOnlyTransaction rTx = transactionChain.newReadOnlyTransaction();\r
         Optional<Port> optPort = DataStoreHelper.readFromDs(LogicalDatastoreType.CONFIGURATION,\r
                 createPortIid(bebp.getPortId()), rTx);\r
+        rTx.close();\r
         if (!optPort.isPresent()) {\r
             return;\r
         }\r
@@ -120,7 +120,7 @@ public class PortHandler implements TransactionChainListener {
 \r
     void processUpdated(Port original, Port delta) {\r
         if (isValidVhostUser(original)) {\r
-            ReadOnlyTransaction rTx = dataBroker.newReadOnlyTransaction();\r
+            ReadOnlyTransaction rTx = transactionChain.newReadOnlyTransaction();\r
             Optional<BaseEndpointByPort> optBebp = DataStoreHelper.readFromDs(LogicalDatastoreType.OPERATIONAL,\r
                     createBaseEpByPortIid(original.getUuid()), rTx);\r
             rTx.close();\r
@@ -145,20 +145,14 @@ public class PortHandler implements TransactionChainListener {
         }\r
     }\r
 \r
-    private void writeVppEndpoint(InstanceIdentifier<VppEndpoint> vppEpIid, VppEndpoint vppEp) {\r
+    private synchronized void writeVppEndpoint(InstanceIdentifier<VppEndpoint> vppEpIid, VppEndpoint vppEp) {\r
         WriteTransaction wTx = transactionChain.newWriteOnlyTransaction();\r
         if (vppEp != null) {\r
             wTx.put(LogicalDatastoreType.CONFIGURATION, vppEpIid, vppEp, true);\r
         } else {\r
             wTx.delete(LogicalDatastoreType.CONFIGURATION, vppEpIid);\r
         }\r
-        try {\r
-            wTx.submit().checkedGet();\r
-        } catch (TransactionCommitFailedException e) {\r
-            LOG.error("Transaction chain commit failed. {}", e);\r
-            transactionChain.close();\r
-            transactionChain = dataBroker.createTransactionChain(this);\r
-        }\r
+        wTx.submit();\r
     }\r
 \r
     @VisibleForTesting\r
@@ -214,7 +208,8 @@ public class PortHandler implements TransactionChainListener {
     @Override\r
     public void onTransactionChainFailed(TransactionChain<?, ?> chain, AsyncTransaction<?, ?> transaction,\r
             Throwable cause) {\r
-        LOG.error("Transaction chain failed. {}", cause.getMessage());\r
+        LOG.error("Transaction chain failed. {} \nTransaction which caused the chain to fail {}", cause.getMessage(),\r
+                transaction, cause);\r
         transactionChain.close();\r
         transactionChain = dataBroker.createTransactionChain(this);\r
     }\r

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 7180b26c004d1ac0653b5f6a009a3f6e72765327..5aae2bc853b91a4d11e8344eb9eba093b53bbca4 100644 (file)
--- 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) {