From 3457d49a4df4c55c88ed6a46f22c4400ee6b1d22 Mon Sep 17 00:00:00 2001 From: Tomas Cechvala Date: Tue, 6 Jun 2017 12:23:43 +0200 Subject: [PATCH] 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 --- .../groupbasedpolicy/util/EndpointUtils.java | 2 +- .../iface/VppEndpointLocationProvider.java | 25 ++++++++++++++++--- .../vpp/policy/acl/DestinationMapper.java | 7 +++--- .../renderer/vpp/policy/acl/SourceMapper.java | 7 +++--- 4 files changed, 30 insertions(+), 11 deletions(-) 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 dd60cab67..3b2e7a17a 100644 --- 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 getParentEndpoints( @Nullable ParentEndpointChoice parentEndpointChoice) { - if (parentEndpointChoice instanceof ParentEndpointCase) { + if (parentEndpointChoice != null && parentEndpointChoice instanceof ParentEndpointCase) { ParentEndpointCase parentEpCase = (ParentEndpointCase) parentEndpointChoice; List 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 1bce842c1..3a32adab5 100644 --- 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 vppEndpoints = new HashMap<>(); private final Map> 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 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() { + CheckedFuture submit = wTx.submit(); + REENTRANT_LOCK.unlock(); + return Futures.transform(submit, new Function() { @Override public Void apply(Void input) { @@ -274,11 +282,14 @@ public class VppEndpointLocationProvider extends DataTreeChangeHandler 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() { + CheckedFuture submit = rwTx.submit(); + REENTRANT_LOCK.unlock(); + return Futures.transform(submit, new Function() { @Override public Void apply(Void input) { @@ -317,11 +328,14 @@ public class VppEndpointLocationProvider extends DataTreeChangeHandler() { + CheckedFuture submit = wTx.submit(); + REENTRANT_LOCK.unlock(); + return Futures.transform(submit, new Function() { @Override public Void apply(Void input) { @@ -390,6 +404,7 @@ public class VppEndpointLocationProvider extends DataTreeChangeHandler 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 submit = rwTx.submit(); + REENTRANT_LOCK.unlock(); + return submit; } private List 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 73c2bbdfd..951c1d850 100644 --- 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 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 a163f09a3..b17e4a5fe 100644 --- 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 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 { -- 2.36.6