Remove Peer.getRawIdentifier()

[bgpcep.git] / bgp / rib-impl / src / main / java / org / opendaylight / protocol / bgp / rib / impl / AbstractPeer.java

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractPeer.java
index 3e3dd3bbac20e27b110009953f84b66c734a926c..7893470d4a1c327cfbbfae668ac4ada3b39a5f0a 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractPeer.java
@@ -9,10 +9,10 @@ package org.opendaylight.protocol.bgp.rib.impl;
 
 import static org.opendaylight.protocol.bgp.rib.spi.RIBNodeIdentifiers.PEER_NID;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.util.concurrent.FluentFuture;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.MoreExecutors;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -21,8 +21,6 @@ import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.eclipse.jdt.annotation.Nullable;
-import org.opendaylight.mdsal.binding.api.TransactionChain;
-import org.opendaylight.mdsal.binding.api.TransactionChainListener;
 import org.opendaylight.mdsal.common.api.CommitInfo;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeWriteOperations;
@@ -66,23 +64,35 @@ import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImportParameters, TransactionChainListener,
-        DOMTransactionChainListener, Peer, PeerTransactionChain {
+abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImportParameters, Peer,
+        PeerTransactionChain, DOMTransactionChainListener {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractPeer.class);
-    protected final RIB rib;
-    final String name;
-    final PeerRole peerRole;
+
+    final RTCClientRouteCache rtCache = new RTCClientRouteCache();
+    final RIB rib;
+
     private final ClusterIdentifier clusterId;
+    private final PeerRole peerRole;
     private final AsNumber localAs;
+    private final String name;
+
+    // FIXME: revisit locking here to improve concurrency:
+    //        -- identifiers, peerId are a shared resource
+    //        -- domChain seems to really be 'ribInChain', accessed from netty thread
+    //        -- ribOutChain is accessed from LocRibWriter
+    //        hence we want to use the two chains concurrently. The problem is their lifecycle in response to errors,
+    //        which needs figuring out.
     @GuardedBy("this")
     private DOMTransactionChain domChain;
     @GuardedBy("this")
-    TransactionChain bindingChain;
-    byte[] rawIdentifier;
-    @GuardedBy("this")
     PeerId peerId;
+
+    // These seem to be separate
+    @GuardedBy("this")
+    @VisibleForTesting
+    DOMTransactionChain ribOutChain;
+    @GuardedBy("this")
     private FluentFuture<? extends CommitInfo> submitted;
-    RTCClientRouteCache rtCache = new RTCClientRouteCache();
 
     AbstractPeer(
             final RIB rib,
@@ -152,11 +162,6 @@ abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImp
         return this.peerRole;
     }
 
-    @Override
-    public final synchronized byte[] getRawIdentifier() {
-        return Arrays.copyOf(this.rawIdentifier, this.rawIdentifier.length);
-    }
-
     @Override
     public final PeerRole getFromPeerRole() {
         return getRole();
@@ -177,11 +182,6 @@ abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImp
         LOG.debug("Transaction chain {} successful.", chain);
     }
 
-    @Override
-    public final void onTransactionChainSuccessful(final TransactionChain chain) {
-        LOG.debug("Transaction chain {} successful.", chain);
-    }
-
     @Override
     public final BGPErrorHandlingState getBGPErrorHandlingState() {
         return this;
@@ -228,7 +228,7 @@ abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImp
     public final synchronized <C extends Routes & DataObject & ChoiceIn<Tables>, S extends ChildOf<? super C>>
             void initializeRibOut(final RouteEntryDependenciesContainer entryDep,
                     final List<ActualBestPathRoutes<C, S>> routesToStore) {
-        if (this.bindingChain == null) {
+        if (this.ribOutChain == null) {
             LOG.debug("Session closed, skip changes to peer AdjRibsOut {}", getPeerId());
             return;
         }
@@ -237,7 +237,7 @@ abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImp
         final YangInstanceIdentifier tableRibout = getRibOutIId(ribSupport.tablesKey());
         final boolean addPathSupported = supportsAddPathSupported(ribSupport.getTablesKey());
 
-        final DOMDataTreeWriteTransaction tx = this.domChain.newWriteOnlyTransaction();
+        final DOMDataTreeWriteTransaction tx = this.ribOutChain.newWriteOnlyTransaction();
         for (final ActualBestPathRoutes<C, S> initRoute : routesToStore) {
             if (!supportsLLGR() && initRoute.isDepreferenced()) {
                 // Stale Long-lived Graceful Restart routes should not be propagated
@@ -281,11 +281,11 @@ abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImp
     public final synchronized <C extends Routes & DataObject & ChoiceIn<Tables>, S extends ChildOf<? super C>>
             void refreshRibOut(final RouteEntryDependenciesContainer entryDep,
                 final List<StaleBestPathRoute> staleRoutes, final List<AdvertizedRoute<C, S>> newRoutes) {
-        if (this.bindingChain == null) {
+        if (this.ribOutChain == null) {
             LOG.debug("Session closed, skip changes to peer AdjRibsOut {}", getPeerId());
             return;
         }
-        final DOMDataTreeWriteTransaction tx = this.domChain.newWriteOnlyTransaction();
+        final DOMDataTreeWriteTransaction tx = this.ribOutChain.newWriteOnlyTransaction();
         final RIBSupport<C, S> ribSupport = entryDep.getRIBSupport();
         deleteRouteRibOut(ribSupport, staleRoutes, tx);
         installRouteRibOut(entryDep, newRoutes, tx);
@@ -309,7 +309,7 @@ abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImp
     public final synchronized <C extends Routes & DataObject & ChoiceIn<Tables>, S extends ChildOf<? super C>>
             void reEvaluateAdvertizement(final RouteEntryDependenciesContainer entryDep,
                 final List<ActualBestPathRoutes<C, S>> routesToStore) {
-        if (this.bindingChain == null) {
+        if (this.ribOutChain == null) {
             LOG.debug("Session closed, skip changes to peer AdjRibsOut {}", getPeerId());
             return;
         }
@@ -318,7 +318,7 @@ abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImp
         final NodeIdentifierWithPredicates tk = ribSupport.tablesKey();
         final boolean addPathSupported = supportsAddPathSupported(ribSupport.getTablesKey());
 
-        final DOMDataTreeWriteTransaction tx = this.domChain.newWriteOnlyTransaction();
+        final DOMDataTreeWriteTransaction tx = this.ribOutChain.newWriteOnlyTransaction();
         for (final ActualBestPathRoutes<C, S> actualBestRoute : routesToStore) {
             final PeerId fromPeerId = actualBestRoute.getFromPeerId();
             if (!filterRoutes(fromPeerId, ribSupport.getTablesKey())) {
@@ -475,7 +475,7 @@ abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImp
         tx.delete(LogicalDatastoreType.OPERATIONAL, ribOutTarget);
     }
 
-    final synchronized void releaseBindingChain(final boolean isWaitForSubmitted) {
+    final synchronized void releaseRibOutChain(final boolean isWaitForSubmitted) {
         if (isWaitForSubmitted) {
             if (this.submitted != null) {
                 try {
@@ -485,14 +485,11 @@ abstract class AbstractPeer extends BGPPeerStateImpl implements BGPRouteEntryImp
                 }
             }
         }
-        closeBindingChain();
-    }
 
-    private synchronized void closeBindingChain() {
-        if (this.bindingChain != null) {
+        if (this.ribOutChain != null) {
             LOG.info("Closing peer chain {}", getPeerId());
-            this.bindingChain.close();
-            this.bindingChain = null;
+            this.ribOutChain.close();
+            this.ribOutChain = null;
         }
     }