Migrate to NamedLocks

KeyedLocks have been deprecated, have an very bad API and do not
quite work as advertized.

Migrate to NamedLocks, which have a safe and easy-to-use API while
providing improved performance. While doing that, change
DirectTunnelUtils to not leak the entire structure, but only provide
a lockTunnel() method.

Change-Id: I8c099e491189c49015ee2939a3ea20f2c0f8c8c5
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/cache/DPNTEPsInfoCache.java b/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/cache/DPNTEPsInfoCache.java
index 858017c49aa6e70b99a23602e9930c2eb57b7533..e921a817d1238f39a8de3d3844a590995992d5c4 100644 (file)
--- a/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/cache/DPNTEPsInfoCache.java
+++ b/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/cache/DPNTEPsInfoCache.java
@@ -28,6 +28,7 @@ import org.opendaylight.genius.itm.utils.TunnelStateInfoBuilder;
 import org.opendaylight.genius.mdsalutil.cache.InstanceIdDataObjectCache;
 import org.opendaylight.infrautils.caches.CacheProvider;
 import org.opendaylight.infrautils.jobcoordinator.JobCoordinator;
+import org.opendaylight.infrautils.utils.concurrent.NamedSimpleReentrantLock.Acquired;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.op.rev160406.DpnEndpoints;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.op.rev160406.dpn.endpoints.DPNTEPsInfo;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
@@ -66,9 +67,11 @@ public class DPNTEPsInfoCache extends InstanceIdDataObjectCache<DPNTEPsInfo> {
     protected void added(InstanceIdentifier<DPNTEPsInfo> path, DPNTEPsInfo dpnTepsInfo) {
         LOG.info("DPNTepsInfo Add Received for {}", dpnTepsInfo.getDPNID());
         String dpnId = dpnTepsInfo.getDPNID().toString();
-        directTunnelUtils.getTunnelLocks().lock(dpnId);
-        Collection<TunnelStateInfo> tunnelStateInfoList = unprocessedNodeConnectorEndPointCache.remove(dpnId);
-        directTunnelUtils.getTunnelLocks().unlock(dpnId);
+
+        Collection<TunnelStateInfo> tunnelStateInfoList;
+        try (Acquired lock = directTunnelUtils.lockTunnel(dpnId)) {
+            tunnelStateInfoList = unprocessedNodeConnectorEndPointCache.remove(dpnId);
+        }
 
         if (tunnelStateInfoList != null) {
             for (TunnelStateInfo tsInfo : tunnelStateInfoList) {
@@ -82,8 +85,7 @@ public class DPNTEPsInfoCache extends InstanceIdDataObjectCache<DPNTEPsInfo> {
                     dstDpnTepsInfo = tsInfo.getDstDpnTepsInfo();
                     if (dstDpnTepsInfo == null) {
                         // Check if the destination End Point has come
-                        try {
-                            directTunnelUtils.getTunnelLocks().lock(tunnelEndPointInfo.getDstEndPointInfo());
+                        try (Acquired lock = directTunnelUtils.lockTunnel(tunnelEndPointInfo.getDstEndPointInfo())) {
                             Optional<DPNTEPsInfo> dstInfoOpt = getDPNTepFromDPNId(
                                     new BigInteger(tunnelEndPointInfo.getDstEndPointInfo()));
                             if (dstInfoOpt.isPresent()) {
@@ -99,8 +101,6 @@ public class DPNTEPsInfoCache extends InstanceIdDataObjectCache<DPNTEPsInfo> {
                                 unprocessedNodeConnectorEndPointCache.add(tunnelEndPointInfo
                                         .getDstEndPointInfo(), tunnelStateInfoNew);
                             }
-                        } finally {
-                            directTunnelUtils.getTunnelLocks().unlock(tunnelEndPointInfo.getDstEndPointInfo());
                         }
                     }
                 } else if (dpnId.equals(tunnelEndPointInfo.getDstEndPointInfo())) {
@@ -108,8 +108,7 @@ public class DPNTEPsInfoCache extends InstanceIdDataObjectCache<DPNTEPsInfo> {
                     srcDpnTepsInfo = tsInfo.getSrcDpnTepsInfo();
                     // Check if the destination End Point has come
                     if (srcDpnTepsInfo == null) {
-                        try {
-                            directTunnelUtils.getTunnelLocks().lock(tunnelEndPointInfo.getSrcEndPointInfo());
+                        try (Acquired lock = directTunnelUtils.lockTunnel(tunnelEndPointInfo.getSrcEndPointInfo())) {
                             Optional<DPNTEPsInfo> srcInfoOpt = getDPNTepFromDPNId(
                                     new BigInteger(tunnelEndPointInfo.getSrcEndPointInfo()));
                             if (srcInfoOpt.isPresent()) {
@@ -126,8 +125,6 @@ public class DPNTEPsInfoCache extends InstanceIdDataObjectCache<DPNTEPsInfo> {
                                 unprocessedNodeConnectorEndPointCache.add(tunnelEndPointInfo.getSrcEndPointInfo(),
                                         tunnelStateInfoNew);
                             }
-                        } finally {
-                            directTunnelUtils.getTunnelLocks().unlock(tunnelEndPointInfo.getSrcEndPointInfo());
                         }
                     }
                 }

diff --git a/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/cache/DpnTepStateCache.java b/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/cache/DpnTepStateCache.java
index ccf071c8dcea8618acdc8865d5c0ac5b7ad7319a..de10bcf2406398b0facc0d8b9fd36759dc32ad78 100644 (file)
--- a/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/cache/DpnTepStateCache.java
+++ b/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/cache/DpnTepStateCache.java
@@ -38,6 +38,7 @@ import org.opendaylight.genius.itm.utils.TunnelStateInfoBuilder;
 import org.opendaylight.genius.mdsalutil.cache.DataObjectCache;
 import org.opendaylight.infrautils.caches.CacheProvider;
 import org.opendaylight.infrautils.jobcoordinator.JobCoordinator;
+import org.opendaylight.infrautils.utils.concurrent.NamedSimpleReentrantLock.Acquired;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.Interface;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rev160406.TunnelMonitoringTypeBfd;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.op.rev160406.DpnTepsState;
@@ -106,9 +107,10 @@ public class DpnTepStateCache extends DataObjectCache<BigInteger, DpnsTeps> {
 
             TunnelStateInfo tunnelStateInfoNew = null;
 
-            directTunnelUtils.getTunnelLocks().lock(remoteDpns.getTunnelName());
-            TunnelStateInfo tunnelStateInfo = unprocessedNCCache.remove(remoteDpns.getTunnelName());
-            directTunnelUtils.getTunnelLocks().unlock(remoteDpns.getTunnelName());
+            TunnelStateInfo tunnelStateInfo;
+            try (Acquired lock = directTunnelUtils.lockTunnel(remoteDpns.getTunnelName())) {
+                tunnelStateInfo = unprocessedNCCache.remove(remoteDpns.getTunnelName());
+            }
 
             if (tunnelStateInfo != null) {
                 LOG.debug("Processing the Unprocessed NodeConnector for Tunnel {}", remoteDpns.getTunnelName());
@@ -126,20 +128,20 @@ public class DpnTepStateCache extends DataObjectCache<BigInteger, DpnsTeps> {
                 tunnelStateInfoNew = builder.build();
                 if (tunnelStateInfoNew.getSrcDpnTepsInfo() == null) {
                     String srcDpnId = tunnelStateInfoNew.getTunnelEndPointInfo().getSrcEndPointInfo();
-                    directTunnelUtils.getTunnelLocks().lock(srcDpnId);
-                    LOG.debug("Source DPNTepsInfo is null for tunnel {}. Hence Parking with key {}",
-                        remoteDpns.getTunnelName(), srcDpnId);
-                    unprocessedNodeConnectorEndPointCache.add(srcDpnId, tunnelStateInfoNew);
-                    directTunnelUtils.getTunnelLocks().unlock(srcDpnId);
+                    try (Acquired lock = directTunnelUtils.lockTunnel(srcDpnId)) {
+                        LOG.debug("Source DPNTepsInfo is null for tunnel {}. Hence Parking with key {}",
+                            remoteDpns.getTunnelName(), srcDpnId);
+                        unprocessedNodeConnectorEndPointCache.add(srcDpnId, tunnelStateInfoNew);
+                    }
                 }
 
                 if (tunnelStateInfoNew.getDstDpnTepsInfo() == null) {
                     String dstDpnId = tunnelStateInfo.getTunnelEndPointInfo().getDstEndPointInfo();
-                    directTunnelUtils.getTunnelLocks().lock(dstDpnId);
-                    LOG.debug("Destination DPNTepsInfo is null for tunnel {}. Hence Parking with key {}",
-                        remoteDpns.getTunnelName(), dstDpnId);
-                    unprocessedNodeConnectorEndPointCache.add(dstDpnId, tunnelStateInfoNew);
-                    directTunnelUtils.getTunnelLocks().unlock(dstDpnId);
+                    try (Acquired lock = directTunnelUtils.lockTunnel(dstDpnId)) {
+                        LOG.debug("Destination DPNTepsInfo is null for tunnel {}. Hence Parking with key {}",
+                            remoteDpns.getTunnelName(), dstDpnId);
+                        unprocessedNodeConnectorEndPointCache.add(dstDpnId, tunnelStateInfoNew);
+                    }
                 }
             }
 

diff --git a/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/itmdirecttunnels/listeners/TunnelInventoryStateListener.java b/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/itmdirecttunnels/listeners/TunnelInventoryStateListener.java
index 3b6b6b99b0fe576abf24e34352f5f9f08666cc60..899df0b71304fd1139cc49003bcec644e6ea7901 100644 (file)
--- a/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/itmdirecttunnels/listeners/TunnelInventoryStateListener.java
+++ b/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/itmdirecttunnels/listeners/TunnelInventoryStateListener.java
@@ -42,6 +42,7 @@ import org.opendaylight.genius.itm.utils.TunnelEndPointInfo;
 import org.opendaylight.genius.itm.utils.TunnelStateInfo;
 import org.opendaylight.genius.itm.utils.TunnelStateInfoBuilder;
 import org.opendaylight.infrautils.jobcoordinator.JobCoordinator;
+import org.opendaylight.infrautils.utils.concurrent.NamedSimpleReentrantLock.Acquired;
 import org.opendaylight.serviceutils.tools.mdsal.listener.AbstractClusteredSyncDataTreeChangeListener;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.state.Interface;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.MacAddress;
@@ -170,8 +171,7 @@ public class TunnelInventoryStateListener extends
             new NodeConnectorInfoBuilder().setNodeConnectorId(key).setNodeConnector(fcNodeConnectorNew).build();
         TunnelStateInfo tunnelStateInfo = null;
         TunnelEndPointInfo tunnelEndPtInfo = null;
-        try {
-            directTunnelUtils.getTunnelLocks().lock(portName);
+        try (Acquired lock = directTunnelUtils.lockTunnel(portName)) {
             if (!dpnTepStateCache.isConfigAvailable(portName)) {
                 // Park the notification
                 LOG.debug("Unable to process the NodeConnector ADD event for {} as Config not available."
@@ -183,8 +183,6 @@ public class TunnelInventoryStateListener extends
                 LOG.debug("{} Interface is not a internal tunnel I/f, so no-op", portName);
                 return;
             }
-        } finally {
-            directTunnelUtils.getTunnelLocks().unlock(portName);
         }
 
         if (DirectTunnelUtils.TUNNEL_PORT_PREDICATE.test(portName) && dpnTepStateCache.isInternal(portName)) {
@@ -197,18 +195,18 @@ public class TunnelInventoryStateListener extends
             tunnelStateInfo = builder.setTunnelEndPointInfo(tunnelEndPtInfo)
                 .setDpnTepInterfaceInfo(dpnTepStateCache.getTunnelFromCache(portName)).build();
             if (tunnelStateInfo.getSrcDpnTepsInfo() == null) {
-                directTunnelUtils.getTunnelLocks().lock(tunnelEndPtInfo.getSrcEndPointInfo());
-                LOG.debug("Source DPNTepsInfo is null for tunnel {}. Hence Parking with key {}",
+                try (Acquired lock = directTunnelUtils.lockTunnel(tunnelEndPtInfo.getSrcEndPointInfo())) {
+                    LOG.debug("Source DPNTepsInfo is null for tunnel {}. Hence Parking with key {}",
                         portName, tunnelEndPtInfo.getSrcEndPointInfo());
-                unprocessedNodeConnectorEndPointCache.add(tunnelEndPtInfo.getSrcEndPointInfo(), tunnelStateInfo);
-                directTunnelUtils.getTunnelLocks().unlock(tunnelEndPtInfo.getSrcEndPointInfo());
+                    unprocessedNodeConnectorEndPointCache.add(tunnelEndPtInfo.getSrcEndPointInfo(), tunnelStateInfo);
+                }
             }
             if (tunnelStateInfo.getDstDpnTepsInfo() == null) {
-                directTunnelUtils.getTunnelLocks().lock(tunnelEndPtInfo.getDstEndPointInfo());
-                LOG.debug("Destination DPNTepsInfo is null for tunnel {}. Hence Parking with key {}",
+                try (Acquired lock = directTunnelUtils.lockTunnel(tunnelEndPtInfo.getDstEndPointInfo())) {
+                    LOG.debug("Destination DPNTepsInfo is null for tunnel {}. Hence Parking with key {}",
                         portName, tunnelEndPtInfo.getDstEndPointInfo());
-                unprocessedNodeConnectorEndPointCache.add(tunnelEndPtInfo.getDstEndPointInfo(), tunnelStateInfo);
-                directTunnelUtils.getTunnelLocks().unlock(tunnelEndPtInfo.getDstEndPointInfo());
+                    unprocessedNodeConnectorEndPointCache.add(tunnelEndPtInfo.getDstEndPointInfo(), tunnelStateInfo);
+                }
             }
         }
 
@@ -312,7 +310,7 @@ public class TunnelInventoryStateListener extends
     }
 
     private boolean modifyOpState(DpnTepInterfaceInfo dpnTepInterfaceInfo, boolean opStateModified) {
-        return opStateModified && (dpnTepInterfaceInfo != null);
+        return opStateModified && dpnTepInterfaceInfo != null;
     }
 
     private boolean modifyTunnelOpState(DpnTepInterfaceInfo dpnTepInterfaceInfo, boolean opStateModified) {

diff --git a/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/itmdirecttunnels/renderer/ovs/utilities/DirectTunnelUtils.java b/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/itmdirecttunnels/renderer/ovs/utilities/DirectTunnelUtils.java
index 78a26102ab983f1c68dc32d556ee833c49e15d8f..f4b984eea0695e90dafffc906cc08aef8c68089a 100644 (file)
--- a/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/itmdirecttunnels/renderer/ovs/utilities/DirectTunnelUtils.java
+++ b/itm/itm-impl/src/main/java/org/opendaylight/genius/itm/itmdirecttunnels/renderer/ovs/utilities/DirectTunnelUtils.java
@@ -10,6 +10,7 @@ package org.opendaylight.genius.itm.itmdirecttunnels.renderer.ovs.utilities;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMap.Builder;
 import com.google.common.util.concurrent.ListenableFuture;
+import edu.umd.cs.findbugs.annotations.CheckReturnValue;
 import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -47,7 +48,8 @@ import org.opendaylight.genius.mdsalutil.matches.MatchInPort;
 import org.opendaylight.genius.mdsalutil.nxmatches.NxMatchRegister;
 import org.opendaylight.genius.mdsalutil.nxmatches.NxMatchTunnelSourceIp;
 import org.opendaylight.genius.utils.clustering.EntityOwnershipUtils;
-import org.opendaylight.infrautils.utils.concurrent.KeyedLocks;
+import org.opendaylight.infrautils.utils.concurrent.NamedLocks;
+import org.opendaylight.infrautils.utils.concurrent.NamedSimpleReentrantLock.Acquired;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Address;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Uri;
@@ -163,7 +165,7 @@ public final class DirectTunnelUtils {
     private static final TopologyId OVSDB_TOPOLOGY_ID = new TopologyId(new Uri("ovsdb:1"));
 
     private static final long INVALID_ID = 0;
-    private final KeyedLocks<String> tunnelLocks = new KeyedLocks<>();
+    private final NamedLocks<String> tunnelLocks = new NamedLocks<>();
 
     // To keep the mapping between Tunnel Types and Tunnel Interfaces
 
@@ -193,8 +195,9 @@ public final class DirectTunnelUtils {
         this.itmConfig = itmConfig;
     }
 
-    public KeyedLocks<String> getTunnelLocks() {
-        return tunnelLocks;
+    @CheckReturnValue
+    public @NonNull Acquired lockTunnel(String tunnelName) {
+        return tunnelLocks.acquire(tunnelName);
     }
 
     public BigInteger getDpnId(DatapathId datapathId) {
@@ -586,4 +589,4 @@ public final class DirectTunnelUtils {
                 .substring(0, 12).replace("-", "");
         return String.format("%s%s", "of", uuidStr);
     }
-}
\ No newline at end of file
+}