Prevent deadlock when updating PCEP stats when Tx chain fails 68/89968/4
authorAjay Lele <ajayslele@gmail.com>
Tue, 26 May 2020 18:05:10 +0000 (11:05 -0700)
committerRobert Varga <nite@hq.sk>
Fri, 17 Jul 2020 10:05:09 +0000 (10:05 +0000)
Multiple threads are blocked on TopologyStatsProviderImpl
instance lock which is held from TopologyStatsProviderImpl#unbind()
which in turn is waiting for delete transaction commit
future to complete. The transaction chain has failed but
the callback TopologyStatsProviderImpl#onTransactionChainFailed()
is blocked on lock held by earlier thread, thus creating a deadlock.
Patch avoids this by registering callback on delete transaction
commit future instead of the blocking call.

JIRA: BGPCEP-901
Signed-off-by: Ajay Lele <ajayslele@gmail.com>
Change-Id: I4a65b3dd00fb6f1255bf6eeb8b5f1d03f3b3a182

pcep/topology/topology-stats/src/main/java/org/opendaylight/bgpcep/pcep/topology/stats/provider/TopologyStatsProviderImpl.java

index 6f87680839e61959ed492224b0c8f5e8be047526..5546c116d339baabedcac4602ad949f32e0de85b 100644 (file)
@@ -15,8 +15,9 @@ import com.google.common.util.concurrent.MoreExecutors;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Set;
 import java.util.TimerTask;
-import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
@@ -48,6 +49,7 @@ public final class TopologyStatsProviderImpl implements TransactionChainListener
     private static final Logger LOG = LoggerFactory.getLogger(TopologyStatsProviderImpl.class);
     @GuardedBy("this")
     private final Map<KeyedInstanceIdentifier<Node, NodeKey>, PcepSessionState> statsMap = new HashMap<>();
+    private final Set<KeyedInstanceIdentifier<Node, NodeKey>> statsPendingDelete = ConcurrentHashMap.newKeySet();
     private final DataBroker dataBroker;
     private final int timeout;
     private TransactionChain transactionChain;
@@ -82,6 +84,9 @@ public final class TopologyStatsProviderImpl implements TransactionChainListener
         try {
             for (final Map.Entry<KeyedInstanceIdentifier<Node, NodeKey>, PcepSessionState> entry
                     : this.statsMap.entrySet()) {
+                if (this.statsPendingDelete.contains(entry.getKey())) {
+                    continue;
+                }
                 final PcepTopologyNodeStatsAug nodeStatsAug = new PcepTopologyNodeStatsAugBuilder()
                         .setPcepSessionState(new PcepSessionStateBuilder(entry.getValue()).build()).build();
                 final InstanceIdentifier<PcepTopologyNodeStatsAug> statId =
@@ -147,12 +152,21 @@ public final class TopologyStatsProviderImpl implements TransactionChainListener
     @Override
     public synchronized void unbind(final KeyedInstanceIdentifier<Node, NodeKey> nodeId) {
         this.statsMap.remove(nodeId);
+        this.statsPendingDelete.add(nodeId);
         final WriteTransaction wTx = this.transactionChain.newWriteOnlyTransaction();
         wTx.delete(LogicalDatastoreType.OPERATIONAL, nodeId);
-        try {
-            wTx.commit().get();
-        } catch (final InterruptedException | ExecutionException e) {
-            LOG.warn("Failed to remove Pcep Node stats {}.", nodeId.getKey().getNodeId(), e);
-        }
+        wTx.commit().addCallback(new FutureCallback<CommitInfo>() {
+            @Override
+            public void onSuccess(final CommitInfo result) {
+                LOG.debug("Successfully removed Pcep Node stats {}.", nodeId.getKey().getNodeId());
+                TopologyStatsProviderImpl.this.statsPendingDelete.remove(nodeId);
+            }
+
+            @Override
+            public void onFailure(final Throwable ex) {
+                LOG.warn("Failed to remove Pcep Node stats {}.", nodeId.getKey().getNodeId(), ex);
+                TopologyStatsProviderImpl.this.statsPendingDelete.remove(nodeId);
+            }
+        }, MoreExecutors.directExecutor());
     }
 }