BUG-2383 : clean up transaction chains in Peers

Change-Id: Iaec5b3b8b1c910c32c08fa63aa9fba4e4374ae0c
Signed-off-by: Dana Kutenicsova <dkutenic@cisco.com>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AdjRibInWriter.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AdjRibInWriter.java
index 8b8a90926cefa761fd39c36edb9768bdbe515f23..0e1134e732e47f1611741b74f384d91babe7954f 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AdjRibInWriter.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AdjRibInWriter.java
@@ -79,9 +79,6 @@ final class AdjRibInWriter {
     private final PeerId peerId;
     private final String role;
 
-    /*
-     * FIXME: transaction chain has to be instantiated in caller, so it can terminate us when it fails.
-     */
     private AdjRibInWriter(final YangInstanceIdentifier ribPath, final DOMTransactionChain chain, final PeerId peerId, final String role, final YangInstanceIdentifier tablesRoot, final Map<TablesKey, TableContext> tables) {
         this.ribPath = Preconditions.checkNotNull(ribPath);
         this.chain = Preconditions.checkNotNull(chain);

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ApplicationPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ApplicationPeer.java
index bb6b4905b5c0b25bfb3084fc5762268b6bbbf181..c480eda500dee8add4cfb65cb74f066c7792de4d 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ApplicationPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ApplicationPeer.java
@@ -42,6 +42,7 @@ public class ApplicationPeer implements AutoCloseable, org.opendaylight.protocol
     private final String name;
     private final YangInstanceIdentifier adjRibsInId;
     private final DOMTransactionChain chain;
+    private final DOMTransactionChain writerChain;
 
     private AdjRibInWriter writer;
 
@@ -52,7 +53,8 @@ public class ApplicationPeer implements AutoCloseable, org.opendaylight.protocol
         final NodeIdentifierWithPredicates peerId = IdentifierUtils.domPeerId(RouterIds.createPeerId(ipAddress));
         this.adjRibsInId = this.targetRib.getYangRibId().node(Peer.QNAME).node(peerId).node(AdjRibIn.QNAME).node(Tables.QNAME);
         this.chain = this.targetRib.createPeerChain(this);
-        this.writer = AdjRibInWriter.create(this.targetRib.getYangRibId(), PeerRole.Ibgp, this.targetRib.createPeerChain(this));
+        this.writerChain = this.targetRib.createPeerChain(this);
+        this.writer = AdjRibInWriter.create(this.targetRib.getYangRibId(), PeerRole.Ibgp, this.writerChain);
         // FIXME: set to true, once it's fixed how to skip advertising routes back to AppPeer
         this.writer = this.writer.transform(RouterIds.createPeerId(ipAddress), this.targetRib.getRibSupportContext(), this.targetRib.getLocalTablesKeys(), false);
     }
@@ -87,6 +89,7 @@ public class ApplicationPeer implements AutoCloseable, org.opendaylight.protocol
     public void close() {
         this.writer.cleanTables(this.targetRib.getLocalTablesKeys());
         this.chain.close();
+        this.writerChain.close();
     }
 
     @Override

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java
index f02f5503d401c28fac22b687ae616c86e4ca42d9..b26f0f0d430a846a476ea909407b21dbb08e4c3e 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java
@@ -28,7 +28,6 @@ import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionChain;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionChainListener;
 import org.opendaylight.controller.md.sal.dom.api.DOMTransactionChain;
-import org.opendaylight.protocol.bgp.rib.impl.spi.AdjRIBsOutRegistration;
 import org.opendaylight.protocol.bgp.rib.impl.spi.BGPSessionStatistics;
 import org.opendaylight.protocol.bgp.rib.impl.spi.RIB;
 import org.opendaylight.protocol.bgp.rib.impl.spi.ReusableBGPPeer;
@@ -69,32 +68,29 @@ public class BGPPeer implements ReusableBGPPeer, Peer, AutoCloseable, BGPPeerRun
 
     @GuardedBy("this")
     private final Set<TablesKey> tables = new HashSet<>();
-    private final RIB rib;
-    private final String name;
-
     @GuardedBy("this")
     private BGPSession session;
     @GuardedBy("this")
     private byte[] rawIdentifier;
     @GuardedBy("this")
-    private AdjRIBsOutRegistration reg;
+    private DOMTransactionChain chain;
+    @GuardedBy("this")
+    private AdjRibInWriter ribWriter;
 
+    private final RIB rib;
+    private final String name;
     private BGPPeerRuntimeRegistrator registrator;
     private BGPPeerRuntimeRegistration runtimeReg;
     private long sessionEstablishedCounter = 0L;
 
-    @GuardedBy("this")
-    private AdjRibInWriter ribWriter;
-
     public BGPPeer(final String name, final RIB rib) {
         this.rib = Preconditions.checkNotNull(rib);
         this.name = name;
+        this.chain = rib.createPeerChain(this);
 
-        final DOMTransactionChain chain = rib.createPeerChain(this);
         // FIXME: make this configurable
         final PeerRole role = PeerRole.Ibgp;
-
-        this.ribWriter = AdjRibInWriter.create(rib.getYangRibId(), role, chain);
+        this.ribWriter = AdjRibInWriter.create(rib.getYangRibId(), role, this.chain);
     }
 
     @Override
@@ -110,7 +106,7 @@ public class BGPPeer implements ReusableBGPPeer, Peer, AutoCloseable, BGPPeerRun
             return;
         }
         final Update message = (Update) msg;
-        //this.rib.updateTables(this, message);
+
         // update AdjRibs
         final Attributes attrs = message.getAttributes();
         MpReachNlri mpReach = null;
@@ -176,28 +172,19 @@ public class BGPPeer implements ReusableBGPPeer, Peer, AutoCloseable, BGPPeerRun
     @Override
     public synchronized void onSessionUp(final BGPSession session) {
         LOG.info("Session with peer {} went up with tables: {}", this.name, session.getAdvertisedTableTypes());
-
         this.session = session;
         this.rawIdentifier = InetAddresses.forString(session.getBgpId().getValue()).getAddress();
 
         for (final BgpTableType t : session.getAdvertisedTableTypes()) {
             final TablesKey key = new TablesKey(t.getAfi(), t.getSafi());
-
             this.tables.add(key);
-            this.rib.initTable(this, key);
 
             // not particularly nice
             if (session instanceof BGPSessionImpl) {
                 AdjRibOutListener.create(key, this.rib.getYangRibId(), ((RIBImpl)this.rib).getService(), this.rib.getRibSupportContext(), ((BGPSessionImpl) session).getLimiter());
             }
         }
-
         this.ribWriter = this.ribWriter.transform(RouterIds.createPeerId(session.getBgpId()), this.rib.getRibSupportContext(), this.tables, false);
-
-        // Not particularly nice, but what can
-        if (session instanceof BGPSessionImpl) {
-            this.reg = this.rib.registerRIBsOut(this, new SessionRIBsOut((BGPSessionImpl) session));
-        }
         this.sessionEstablishedCounter++;
         if (this.registrator != null) {
             this.runtimeReg = this.registrator.register(this);
@@ -207,15 +194,7 @@ public class BGPPeer implements ReusableBGPPeer, Peer, AutoCloseable, BGPPeerRun
     private synchronized void cleanup() {
         // FIXME: BUG-196: support graceful restart
         this.ribWriter.cleanTables(this.tables);
-        for (final TablesKey key : this.tables) {
-            this.rib.clearTable(this, key);
-        }
-
-        if (this.reg != null) {
-            this.reg.close();
-            this.reg = null;
-        }
-
+        this.chain.close();
         this.tables.clear();
     }
 
@@ -247,10 +226,6 @@ public class BGPPeer implements ReusableBGPPeer, Peer, AutoCloseable, BGPPeerRun
         return this.name;
     }
 
-    protected RIB getRib() {
-        return this.rib;
-    }
-
     @Override
     public void releaseConnection() {
         dropConnection();
@@ -320,12 +295,13 @@ public class BGPPeer implements ReusableBGPPeer, Peer, AutoCloseable, BGPPeerRun
 
     @Override
     public void onTransactionChainFailed(final TransactionChain<?, ?> chain, final AsyncTransaction<?, ?> transaction, final Throwable cause) {
-        // TODO Auto-generated method stub
-
+        LOG.error("Transaction chain failed.", cause);
+        this.dropConnection();
+        this.chain = this.rib.createPeerChain(this);
     }
 
     @Override
     public void onTransactionChainSuccessful(final TransactionChain<?, ?> chain) {
-        // TODO Auto-generated method stub
+        LOG.debug("Transaction chain {} successfull.", chain);
     }
 }

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/ApplicationPeerTest.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/ApplicationPeerTest.java
index 297e2d4d1b28dcb3d789889b9a41cdcea8899d6e..85e696cb9138151d4f13217eb18d770bcefc34b4 100644 (file)
--- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/ApplicationPeerTest.java
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/ApplicationPeerTest.java
@@ -355,7 +355,6 @@ public class ApplicationPeerTest {
         final List<BgpParameters> params = Lists.newArrayList(new BgpParametersBuilder().setOptionalCapabilities(Lists.newArrayList(new OptionalCapabilitiesBuilder().setCParameters(new MultiprotocolCaseBuilder()
             .setMultiprotocolCapability(new MultiprotocolCapabilityBuilder().setAfi(Ipv4AddressFamily.class).setSafi(UnicastSubsequentAddressFamily.class).build()).build()).build())).build());
         this.session = new BGPSessionImpl(this.classic, this.channel, new OpenBuilder().setBgpIdentifier(new Ipv4Address("1.1.1.1")).setHoldTimer(50).setMyAsNumber(72).setBgpParameters(params).build(), 30, null);
-        assertEquals(this.r, this.classic.getRib());
         assertEquals("testPeer", this.classic.getName());
         Mockito.doAnswer(new Answer<Object>() {