Bug 8608 - quick fix for async transaction creation

Transactions created from BindingTransactionChain have
not been synced properly. A lock is added to force other
threads to wait until created transactions are submitted.
More comprehensive fix will be submitted later due to
shortage of time.

Change-Id: Ie27cd95b6136699edb75118f4c2f81ee305996c0
Signed-off-by: Tomas Cechvala <tcechval@cisco.com>

diff --git a/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/util/EndpointUtils.java b/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/util/EndpointUtils.java
index dd60cab670c17d7f0af535d4e74afdc3a423d34d..3b2e7a17adacb15c869fbb4c93392c7cfe8e6143 100644 (file)
--- a/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/util/EndpointUtils.java
+++ b/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/util/EndpointUtils.java
@@ -63,7 +63,7 @@ public class EndpointUtils {
 
     public static @Nonnull List<ParentEndpoint> getParentEndpoints(
             @Nullable ParentEndpointChoice parentEndpointChoice) {
-        if (parentEndpointChoice instanceof ParentEndpointCase) {
+        if (parentEndpointChoice != null && parentEndpointChoice instanceof ParentEndpointCase) {
             ParentEndpointCase parentEpCase = (ParentEndpointCase) parentEndpointChoice;
             List<ParentEndpoint> parentEndpoints = parentEpCase.getParentEndpoint();
             if (parentEndpoints != null) {

diff --git a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/iface/VppEndpointLocationProvider.java b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/iface/VppEndpointLocationProvider.java
index 1bce842c1795333fae756ab07191cf8903823b93..3a32adab55b78f3dc963cd688aeceb181df45494 100644 (file)
--- a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/iface/VppEndpointLocationProvider.java
+++ b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/iface/VppEndpointLocationProvider.java
@@ -16,6 +16,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.stream.Collectors;
 
 import javax.annotation.Nonnull;
@@ -28,6 +29,7 @@ import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
 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.LogicalDatastoreType;
+import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
 import org.opendaylight.groupbasedpolicy.renderer.util.AddressEndpointUtils;
 import org.opendaylight.groupbasedpolicy.renderer.vpp.manager.VppNodeManager;
 import org.opendaylight.groupbasedpolicy.renderer.vpp.util.CloseOnFailTransactionChain;
@@ -72,6 +74,7 @@ import org.slf4j.LoggerFactory;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
+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;
@@ -86,6 +89,8 @@ public class VppEndpointLocationProvider extends DataTreeChangeHandler<AddressEn
     private final Map<VppEndpointKey, VppEndpoint> vppEndpoints = new HashMap<>();
     private final Map<VppEndpointKey, DataObjectModification<AddressEndpoint>> cachedVppEndpoints = new HashMap<>();
 
+    public static final ReentrantLock REENTRANT_LOCK = new ReentrantLock();
+
     public VppEndpointLocationProvider(DataBroker dataProvider) {
         super(dataProvider);
         LocationProvider locationProvider = new LocationProviderBuilder().setProvider(VPP_ENDPOINT_LOCATION_PROVIDER)
@@ -259,11 +264,14 @@ public class VppEndpointLocationProvider extends DataTreeChangeHandler<AddressEn
     }
 
     public ListenableFuture<Void> writeLocation(ProviderAddressEndpointLocation location) {
+        REENTRANT_LOCK.lock();
         WriteTransaction wTx = txChain.newWriteOnlyTransaction();
         wTx.put(LogicalDatastoreType.CONFIGURATION,
                 IidFactory.providerAddressEndpointLocationIid(VPP_ENDPOINT_LOCATION_PROVIDER, location.getKey()),
                 location, true);
-        return Futures.transform(wTx.submit(), new Function<Void, Void>() {
+        CheckedFuture<Void, TransactionCommitFailedException> submit = wTx.submit();
+        REENTRANT_LOCK.unlock();
+        return Futures.transform(submit, new Function<Void, Void>() {
 
             @Override
             public Void apply(Void input) {
@@ -274,11 +282,14 @@ public class VppEndpointLocationProvider extends DataTreeChangeHandler<AddressEn
     }
 
     public ListenableFuture<Void> deleteLocation(ProviderAddressEndpointLocationKey key) {
+        REENTRANT_LOCK.lock();
         ReadWriteTransaction rwTx = txChain.newReadWriteTransaction();
         LOG.debug("Deleting location for {}", key);
         DataStoreHelper.removeIfExists(LogicalDatastoreType.CONFIGURATION,
                 IidFactory.providerAddressEndpointLocationIid(VPP_ENDPOINT_LOCATION_PROVIDER, key), rwTx);
-        return Futures.transform(rwTx.submit(), new Function<Void, Void>() {
+        CheckedFuture<Void, TransactionCommitFailedException> submit = rwTx.submit();
+        REENTRANT_LOCK.unlock();
+        return Futures.transform(submit, new Function<Void, Void>() {
 
             @Override
             public Void apply(Void input) {
@@ -317,11 +328,14 @@ public class VppEndpointLocationProvider extends DataTreeChangeHandler<AddressEn
             return Futures.immediateFuture(null);
         }
         ProviderAddressEndpointLocation providerLocation = builder.build();
+        REENTRANT_LOCK.lock();
         WriteTransaction wTx = txChain.newWriteOnlyTransaction();
         wTx.put(LogicalDatastoreType.CONFIGURATION, IidFactory.providerAddressEndpointLocationIid(
                 VPP_ENDPOINT_LOCATION_PROVIDER, providerLocation.getKey()), providerLocation);
         LOG.debug("Updating location for {}", builder.build().getKey());
-        return Futures.transform(wTx.submit(), new Function<Void, Void>() {
+        CheckedFuture<Void, TransactionCommitFailedException> submit = wTx.submit();
+        REENTRANT_LOCK.unlock();
+        return Futures.transform(submit, new Function<Void, Void>() {
 
             @Override
             public Void apply(Void input) {
@@ -390,6 +404,7 @@ public class VppEndpointLocationProvider extends DataTreeChangeHandler<AddressEn
         }
 
         ListenableFuture<Void> syncMultiparents() {
+            REENTRANT_LOCK.lock();
             ReadWriteTransaction rwTx = transactionChain.newReadWriteTransaction();
             if (before != null) {
                 for (ParentEndpoint pe : EndpointUtils.getParentEndpoints(before.getParentEndpointChoice())) {
@@ -427,7 +442,9 @@ public class VppEndpointLocationProvider extends DataTreeChangeHandler<AddressEn
 
                 }
             }
-            return rwTx.submit();
+            CheckedFuture<Void, TransactionCommitFailedException> submit = rwTx.submit();
+            REENTRANT_LOCK.unlock();
+            return submit;
         }
 
         private List<ChildEndpoint> abc(ReadWriteTransaction rTx, ParentEndpoint pe) {

diff --git a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/acl/DestinationMapper.java b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/acl/DestinationMapper.java
index 73c2bbdfd79af567d654d1b94294eeae18ce8c60..951c1d850b2b1872b3214d1ebdba1eb170104201 100644 (file)
--- a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/acl/DestinationMapper.java
+++ b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/acl/DestinationMapper.java
@@ -41,12 +41,13 @@ class DestinationMapper extends AddressMapper {
         if (addrEp.getContextType().isAssignableFrom(L3Context.class)) {
             address = addrEp.getAddress();
         } else {
-            ParentEndpoint parentEp = EndpointUtils.getParentEndpoints(addrEp.getParentEndpointChoice()).get(0);
-            if (parentEp == null || !parentEp.getContextType().isAssignableFrom(L3Context.class)) {
+            List<ParentEndpoint> parentEndpoints = EndpointUtils.getParentEndpoints(addrEp.getParentEndpointChoice());
+            if (parentEndpoints == null || parentEndpoints.isEmpty()
+                    || !parentEndpoints.get(0).getContextType().isAssignableFrom(L3Context.class)) {
                 LOG.warn("Cannot resolve IP address for endpoint {}", addrEp);
                 return;
             }
-            address = parentEp.getAddress();
+            address = parentEndpoints.get(0).getAddress();
         }
         LOG.trace("Setting dst IP address {} in rule {}", address, aclRuleBuilder);
         try {

diff --git a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/acl/SourceMapper.java b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/acl/SourceMapper.java
index a163f09a3bfcadde00bb15d11c21e82de06f13f3..b17e4a5fea891afcb0d2f19988e64cc336a7f0e4 100644 (file)
--- a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/acl/SourceMapper.java
+++ b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/acl/SourceMapper.java
@@ -41,12 +41,13 @@ class SourceMapper extends AddressMapper {
         if (addrEp.getContextType().isAssignableFrom(L3Context.class)) {
             address = addrEp.getAddress();
         } else {
-            ParentEndpoint parentEp = EndpointUtils.getParentEndpoints(addrEp.getParentEndpointChoice()).get(0);
-            if (parentEp == null || !parentEp.getContextType().isAssignableFrom(L3Context.class)) {
+            List<ParentEndpoint> parentEndpoints = EndpointUtils.getParentEndpoints(addrEp.getParentEndpointChoice());
+            if (parentEndpoints == null || parentEndpoints.isEmpty()
+                    || !parentEndpoints.get(0).getContextType().isAssignableFrom(L3Context.class)) {
                 LOG.warn("Cannot resolve IP address for endpoint {}", addrEp);
                 return;
             }
-            address = parentEp.getAddress();
+            address = parentEndpoints.get(0).getAddress();
         }
         LOG.trace("Setting src IP address {} in rule {}", address, aclRuleBuilder);
         try {