From 2cb352533464b608896336b88e69ddd8f5df0031 Mon Sep 17 00:00:00 2001 From: Ajay Lele Date: Tue, 26 May 2020 11:05:10 -0700 Subject: [PATCH] Prevent deadlock when updating PCEP stats when Tx chain fails 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 Change-Id: I4a65b3dd00fb6f1255bf6eeb8b5f1d03f3b3a182 --- .../provider/TopologyStatsProviderImpl.java | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/pcep/topology/topology-stats/src/main/java/org/opendaylight/bgpcep/pcep/topology/stats/provider/TopologyStatsProviderImpl.java b/pcep/topology/topology-stats/src/main/java/org/opendaylight/bgpcep/pcep/topology/stats/provider/TopologyStatsProviderImpl.java index 6f87680839..5546c116d3 100644 --- a/pcep/topology/topology-stats/src/main/java/org/opendaylight/bgpcep/pcep/topology/stats/provider/TopologyStatsProviderImpl.java +++ b/pcep/topology/topology-stats/src/main/java/org/opendaylight/bgpcep/pcep/topology/stats/provider/TopologyStatsProviderImpl.java @@ -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, PcepSessionState> statsMap = new HashMap<>(); + private final Set> 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, 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 statId = @@ -147,12 +152,21 @@ public final class TopologyStatsProviderImpl implements TransactionChainListener @Override public synchronized void unbind(final KeyedInstanceIdentifier 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() { + @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()); } } -- 2.36.6