Revert "Fix issues during connection flap with scaled pcc sessions."

This reverts commit a162197f9be41a2296e7571ba7bbfac73dce50f7. The patch
needed to be split up, which unfortuantely did not happen on the same
change-id. We will follow up on this with properly split up patches.

JIRA: BGPCEP-920
Change-Id: I06d5326de134c9b70c703a761498365643a4f7df
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
index 706b34f958dc91cd68374eca7b98264830333428..a85730af250a2b05b5eabbd4779c02d7bd420e76 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
@@ -343,7 +343,6 @@ public abstract class AbstractTopologySessionListener<S, L> implements TopologyS
     private void clearNodeState() {
         if (this.nodeState != null) {
             this.serverSessionManager.unbind(this.nodeState.getNodeId());
-            LOG.info("Clear Node state: {}", this.nodeState.getNodeId().getKey().getNodeId().getValue());
             this.nodeState = null;
         }
     }

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
index 58b1eb6d7366d3241e72bf27ea731fd13713de61..eaa343a295560f4dece1d21127cdea09212ad5ad 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
@@ -127,7 +127,6 @@ final class ServerSessionManager implements PCEPSessionListenerFactory, Topology
             return;
         }
         this.nodes.remove(createNodeId(session.getRemoteAddress()));
-        this.state.remove(createNodeId(session.getRemoteAddress()));
         if (nodeState != null) {
             LOG.debug("Node {} unbound", nodeState.getNodeId());
             nodeState.released(persistNode);

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/TopologyNodeState.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/TopologyNodeState.java
index 3dd2ccc1b0f6a4f0f8fb14a248be0cbc507ba958..63abe010df709615013064193ed4e86f3b05483e 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/TopologyNodeState.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/TopologyNodeState.java
@@ -16,7 +16,6 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.atomic.AtomicBoolean;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.binding.api.DataBroker;
@@ -45,7 +44,6 @@ import org.slf4j.LoggerFactory;
 // This class is thread-safe
 final class TopologyNodeState implements AutoCloseable, TransactionChainListener {
     private static final Logger LOG = LoggerFactory.getLogger(TopologyNodeState.class);
-    private final AtomicBoolean closed = new AtomicBoolean(false);
     private final Map<String, Metadata> metadata = new HashMap<>();
     private final KeyedInstanceIdentifier<Node, NodeKey> nodeId;
     private final TransactionChain chain;
@@ -103,7 +101,6 @@ final class TopologyNodeState implements AutoCloseable, TransactionChainListener
             }, MoreExecutors.directExecutor());
         }
 
-        close();
         this.lastReleased = System.nanoTime();
     }
 
@@ -153,9 +150,6 @@ final class TopologyNodeState implements AutoCloseable, TransactionChainListener
         // FIXME: flip internal state, so that the next attempt to update fails, triggering node reconnect
         LOG.error("Unexpected transaction failure in node {} transaction {}",
                 this.nodeId, transaction.getIdentifier(), cause);
-        if (closed.compareAndSet(false, true)) {
-            pchain.close();
-        }
     }
 
     @Override
@@ -165,9 +159,7 @@ final class TopologyNodeState implements AutoCloseable, TransactionChainListener
 
     @Override
     public synchronized void close() {
-        if (closed.compareAndSet(false, true)) {
-            this.chain.close();
-        }
+        this.chain.close();
     }
 
     private synchronized void putTopologyNode() {
@@ -184,7 +176,7 @@ final class TopologyNodeState implements AutoCloseable, TransactionChainListener
 
             @Override
             public void onFailure(final Throwable throwable) {
-                LOG.error("Put topology Node failed {}, value {}", TopologyNodeState.this.nodeId, node, throwable);
+                LOG.trace("Put topology Node failed {}, value {}", TopologyNodeState.this.nodeId, node, throwable);
             }
         }, MoreExecutors.directExecutor());
     }

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/session/stats/SessionStateImpl.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/session/stats/SessionStateImpl.java
index 6ecbf6eaff2aee388039272e33eda14185cae44c..3e2cc294da045e8caa9f15dfd86e20fcefee2647 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/session/stats/SessionStateImpl.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/session/stats/SessionStateImpl.java
@@ -135,9 +135,6 @@ public final class SessionStateImpl implements PcepSessionState {
 
     @Override
     public Messages getMessages() {
-        if (pcepSessionState == null) {
-            return null;
-        }
         return new MessagesBuilder(this.pcepSessionState.getMessages())
                 .setReplyTime(setReplyTime())
                 .addAugmentation(createStatefulMessages())

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 e0d7a6d54997978856f21bad6bb08f2268214be0..5546c116d339baabedcac4602ad949f32e0de85b 100644 (file)
--- 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
@@ -78,53 +78,35 @@ public final class TopologyStatsProviderImpl implements TransactionChainListener
     @SuppressWarnings("checkstyle:IllegalCatch")
     @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD",
             justification = "https://github.com/spotbugs/spotbugs/issues/811")
-    private void updatePcepStats() {
-        synchronized (this.transactionChain) {
-            WriteTransaction tx = null;
-
-            try {
-                tx = TopologyStatsProviderImpl.this.transactionChain.newWriteOnlyTransaction();
-                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 =
-                            entry.getKey().augmentation(PcepTopologyNodeStatsAug.class);
-                    tx.put(LogicalDatastoreType.OPERATIONAL, statId, nodeStatsAug);
-                }
-                tx.commit().addCallback(new FutureCallback<CommitInfo>() {
-                    @Override
-                    public void onSuccess(final CommitInfo result) {
-                        LOG.debug("Successfully committed Topology stats update");
-                    }
-
-                    @Override
-                    public void onFailure(final Throwable ex) {
-                        LOG.error("Failed to commit Topology stats update", ex);
-                    }
-                }, MoreExecutors.directExecutor());
-            } catch (final Exception e) {
-                if (tx != null) {
-                    LOG.warn("Failed to prepare Tx {} for PCEP stats update", tx.getIdentifier(), e);
-                    tx.cancel();
-                    this.transactionChain.close();
-                    recreateTxChain();
-                }
-            }
-        }
-    }
+    private synchronized void updatePcepStats() {
+        final WriteTransaction tx = TopologyStatsProviderImpl.this.transactionChain.newWriteOnlyTransaction();
 
-    @SuppressWarnings("checkstyle:IllegalCatch")
-    private synchronized void recreateTxChain() {
         try {
-            if (!closed.get()) {
-                transactionChain = dataBroker.createMergingTransactionChain(this);
+            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 =
+                        entry.getKey().augmentation(PcepTopologyNodeStatsAug.class);
+                tx.put(LogicalDatastoreType.OPERATIONAL, statId, nodeStatsAug);
             }
+            tx.commit().addCallback(new FutureCallback<CommitInfo>() {
+                @Override
+                public void onSuccess(final CommitInfo result) {
+                    LOG.debug("Successfully committed Topology stats update");
+                }
+
+                @Override
+                public void onFailure(final Throwable ex) {
+                    LOG.error("Failed to commit Topology stats update", ex);
+                }
+            }, MoreExecutors.directExecutor());
         } catch (final Exception e) {
-            LOG.error("Failed to recreate transaction chain {}", transactionChain, e);
+            LOG.warn("Failed to prepare Tx for PCEP stats update", e);
+            tx.cancel();
         }
     }
 
@@ -133,75 +115,58 @@ public final class TopologyStatsProviderImpl implements TransactionChainListener
         if (closed.compareAndSet(false, true)) {
             LOG.info("Closing TopologyStatsProvider service.");
             this.scheduleTask.cancel(true);
-            this.transactionChain.close();
-            final WriteTransaction wTx = this.dataBroker.newWriteOnlyTransaction();
+            final WriteTransaction wTx = this.transactionChain.newWriteOnlyTransaction();
             for (final KeyedInstanceIdentifier<Node, NodeKey> statId : this.statsMap.keySet()) {
                 wTx.delete(LogicalDatastoreType.OPERATIONAL, statId);
             }
             wTx.commit().get();
             this.statsMap.clear();
+            this.transactionChain.close();
             this.scheduler.shutdown();
         }
     }
 
     @Override
-    public void onTransactionChainFailed(final TransactionChain chain,
+    public synchronized void onTransactionChainFailed(final TransactionChain chain,
             final Transaction transaction, final Throwable cause) {
         LOG.error("Transaction chain {} failed for tx {}",
                 chain, transaction != null ? transaction.getIdentifier() : null, cause);
-        chain.close();
-        if (chain == this.transactionChain) {
-            recreateTxChain();
+
+        if (!closed.get()) {
+            transactionChain.close();
+            transactionChain = dataBroker.createMergingTransactionChain(this);
         }
     }
 
     @Override
-    public void onTransactionChainSuccessful(final TransactionChain chain) {
+    public synchronized void onTransactionChainSuccessful(final TransactionChain chain) {
         LOG.debug("Transaction chain {} successful.", chain);
     }
 
     @Override
-    public void bind(final KeyedInstanceIdentifier<Node, NodeKey> nodeId,
+    public synchronized void bind(final KeyedInstanceIdentifier<Node, NodeKey> nodeId,
             final PcepSessionState sessionState) {
-        LOG.info("Bind: {}", nodeId.getKey().getNodeId());
-        synchronized (this.transactionChain) {
-            this.statsMap.put(nodeId, sessionState);
-        }
+        this.statsMap.put(nodeId, sessionState);
     }
 
-    @SuppressWarnings("checkstyle:IllegalCatch")
     @Override
-    public void unbind(final KeyedInstanceIdentifier<Node, NodeKey> nodeId) {
-        synchronized (this.transactionChain) {
-            LOG.info("Unbind: {}", nodeId.getKey().getNodeId());
-            this.statsMap.remove(nodeId);
-            this.statsPendingDelete.add(nodeId);
-            WriteTransaction tx = null;
-            try {
-                tx = this.transactionChain.newWriteOnlyTransaction();
-                tx.delete(LogicalDatastoreType.OPERATIONAL, nodeId);
-                tx.commit().addCallback(new FutureCallback<CommitInfo>() {
-                    @Override
-                    public void onSuccess(final CommitInfo result) {
-                        LOG.info("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());
-            } catch (final Exception e) {
-                if (tx != null) {
-                    LOG.warn("Failed to prepare Tx {} for Pcep Node stats delete for node {}.", tx.getIdentifier(),
-                            nodeId.getKey().getNodeId(), e);
-                    tx.cancel();
-                    this.transactionChain.close();
-                    recreateTxChain();
-                }
+    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);
+        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());
     }
 }