Bug-7614: LocRibWriter does not recover well from transaction chain failure

If transaction chain associated with LocRibWriter fails, create a
new transaction chain and re-initialize it with the new chain.

Change-Id: Ibbb60c41594ad6b3602cabac18f5e4bccaa093bb
Signed-off-by: Ajay <ajayl.bro@gmail.com>
Signed-off-by: Claudio D. Gasparini <claudio.gasparini@pantheon.tech>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/LocRibWriter.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/LocRibWriter.java
index 90d8e7551d16fe23de65bc7580824f9943f99db4..ec785e68f7b7498d48e567e153bbb8ed18f15d1b 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/LocRibWriter.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/LocRibWriter.java
@@ -68,13 +68,16 @@ final class LocRibWriter implements AutoCloseable, TotalPrefixesCounter, TotalPa
 
     private final Map<PathArgument, RouteEntry> routeEntries = new HashMap<>();
     private final YangInstanceIdentifier locRibTarget;
-    private final DOMTransactionChain chain;
+    private final NodeIdentifierWithPredicates tableKey;
+    private DOMTransactionChain chain;
     private final ExportPolicyPeerTracker exportPolicyPeerTracker;
     private final NodeIdentifier attributesIdentifier;
     private final Long ourAs;
     private final RIBSupport ribSupport;
     private final TablesKey localTablesKey;
-    private final ListenerRegistration<LocRibWriter> reg;
+    private final YangInstanceIdentifier target;
+    private final DOMDataTreeChangeService service;
+    private ListenerRegistration<LocRibWriter> reg;
     private final PathSelectionMode pathSelectionMode;
     private final LongAdder totalPathsCounter = new LongAdder();
     private final LongAdder totalPrefixesCounter = new LongAdder();
@@ -84,23 +87,32 @@ final class LocRibWriter implements AutoCloseable, TotalPrefixesCounter, TotalPa
         final ExportPolicyPeerTracker exportPolicyPeerTracker, final TablesKey tablesKey,
         final PathSelectionMode pathSelectionMode) {
         this.chain = Preconditions.checkNotNull(chain);
+        this.target = Preconditions.checkNotNull(target);
         final NodeIdentifierWithPredicates tableKey = RibSupportUtils.toYangTablesKey(tablesKey);
         this.localTablesKey = tablesKey;
+        this.tableKey = tableKey;
         this.locRibTarget = YangInstanceIdentifier.create(target.node(LocRib.QNAME).node(Tables.QNAME).node(tableKey).getPathArguments());
         this.ourAs = Preconditions.checkNotNull(ourAs);
+        this.service = Preconditions.checkNotNull(service);
         this.ribSupport = registry.getRIBSupportContext(tablesKey).getRibSupport();
         this.attributesIdentifier = this.ribSupport.routeAttributesIdentifier();
         this.exportPolicyPeerTracker = exportPolicyPeerTracker;
         this.pathSelectionMode = pathSelectionMode;
 
+        init();
+    }
+
+    private synchronized void init() {
         final DOMDataWriteTransaction tx = this.chain.newWriteOnlyTransaction();
         tx.merge(LogicalDatastoreType.OPERATIONAL, this.locRibTarget.node(Routes.QNAME), this.ribSupport.emptyRoutes());
-        tx.merge(LogicalDatastoreType.OPERATIONAL, this.locRibTarget.node(Attributes.QNAME).node(ATTRIBUTES_UPTODATE_TRUE.getNodeType()), ATTRIBUTES_UPTODATE_TRUE);
+        tx.merge(LogicalDatastoreType.OPERATIONAL, this.locRibTarget.node(Attributes.QNAME)
+            .node(ATTRIBUTES_UPTODATE_TRUE.getNodeType()), ATTRIBUTES_UPTODATE_TRUE);
         tx.submit();
 
-        final YangInstanceIdentifier tableId = target.node(Peer.QNAME).node(Peer.QNAME).node(EffectiveRibIn.QNAME).node(Tables.QNAME).node(tableKey);
+        final YangInstanceIdentifier tableId = this.target.node(Peer.QNAME).node(Peer.QNAME).node(EffectiveRibIn.QNAME)
+            .node(Tables.QNAME).node(this.tableKey);
         final DOMDataTreeIdentifier wildcard = new DOMDataTreeIdentifier(LogicalDatastoreType.OPERATIONAL, tableId);
-        this.reg = service.registerDataTreeChangeListener(wildcard, this);
+        this.reg = this.service.registerDataTreeChangeListener(wildcard, this);
     }
 
     public static LocRibWriter create(@Nonnull final RIBSupportContextRegistry registry, @Nonnull final TablesKey tablesKey,
@@ -111,10 +123,22 @@ final class LocRibWriter implements AutoCloseable, TotalPrefixesCounter, TotalPa
             pathSelectionStrategy);
     }
 
+    /**
+     * Re-initialize this LocRibWriter with new transaction chain.
+     *
+     * @param newChain new transaction chain
+     */
+    synchronized void restart(@Nonnull final DOMTransactionChain newChain) {
+        Preconditions.checkNotNull(newChain);
+        close();
+        this.chain = newChain;
+        init();
+    }
+
     @Override
     public void close() {
         this.reg.close();
-        // FIXME: wait for the chain to close? unfortunately RIBImpl is the listener, so that may require some work
+        this.reg = null;
         this.chain.close();
     }
 

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java
index 3551c95ffc95c9315c24ea66be1ab2b28f760b13..dd48667963ac8e976d4705ae7a276193923a1c1b 100755 (executable)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java
@@ -14,7 +14,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.ListenableFuture;
-import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -106,7 +106,7 @@ public final class RIBImpl extends BGPRIBStateImpl implements ClusterSingletonSe
     private final BgpDeployer.WriteConfiguration configurationWriter;
     private ClusterSingletonServiceRegistration registration;
     private final DOMDataBrokerExtension service;
-    private final List<LocRibWriter> locRibs = new ArrayList<>();
+    private final Map<TransactionChain, LocRibWriter> txChainToLocRibWriter = new HashMap<>();
     private final Map<TablesKey, PathSelectionMode> bestPathSelectionStrategies;
     private final ImportPolicyPeerTracker importPolicyPeerTracker;
     private final RIBImplRuntimeMXBeanImpl renderStats;
@@ -183,17 +183,22 @@ public final class RIBImpl extends BGPRIBStateImpl implements ClusterSingletonSe
         } catch (final TransactionCommitFailedException e1) {
             LOG.error("Failed to initiate LocRIB for key {}", key, e1);
         }
+        createLocRibWriter(key);
+    }
 
+    private synchronized void createLocRibWriter(final TablesKey key) {
+        LOG.debug("Creating LocRIB writer for key {}", key);
+        final DOMTransactionChain txChain = createPeerChain(this);
         PathSelectionMode pathSelectionStrategy = this.bestPathSelectionStrategies.get(key);
         if (pathSelectionStrategy == null) {
             pathSelectionStrategy = BasePathSelectionModeFactory.createBestPathSelectionStrategy();
         }
 
-        final LocRibWriter locRibWriter = LocRibWriter.create(this.ribContextRegistry, key, createPeerChain(this),
+        final LocRibWriter locRibWriter = LocRibWriter.create(this.ribContextRegistry, key, txChain,
             getYangRibId(), this.localAs, getService(), this.exportPolicyPeerTrackerMap.get(key), pathSelectionStrategy);
         registerTotalPathCounter(key, locRibWriter);
         registerTotalPrefixesCounter(key, locRibWriter);
-        this.locRibs.add(locRibWriter);
+        this.txChainToLocRibWriter.put(txChain, locRibWriter);
     }
 
     @Override
@@ -235,8 +240,14 @@ public final class RIBImpl extends BGPRIBStateImpl implements ClusterSingletonSe
     }
 
     @Override
-    public void onTransactionChainFailed(final TransactionChain<?, ?> chain, final AsyncTransaction<?, ?> transaction, final Throwable cause) {
+    public synchronized void onTransactionChainFailed(final TransactionChain<?, ?> chain, final AsyncTransaction<?, ?> transaction, final Throwable cause) {
         LOG.error("Broken chain in RIB {} transaction {}", getInstanceIdentifier(), transaction != null ? transaction.getIdentifier() : null, cause);
+        if (this.txChainToLocRibWriter.containsKey(chain)) {
+            final LocRibWriter locRibWriter = this.txChainToLocRibWriter.remove(chain);
+            final DOMTransactionChain newChain = createPeerChain(this);
+            locRibWriter.restart(newChain);
+            this.txChainToLocRibWriter.put(newChain, locRibWriter);
+        }
     }
 
     @Override
@@ -338,10 +349,10 @@ public final class RIBImpl extends BGPRIBStateImpl implements ClusterSingletonSe
     }
 
     @Override
-    public ListenableFuture<Void> closeServiceInstance() {
-        LOG.info("Close RIB Singleton Service {}", getIdentifier());
-        this.locRibs.forEach(LocRibWriter::close);
-        this.locRibs.clear();
+    public synchronized ListenableFuture<Void> closeServiceInstance() {
+        LOG.info("RIB {} closing instance", this.ribId.getValue());
+        this.txChainToLocRibWriter.values().forEach(LocRibWriter::close);
+        this.txChainToLocRibWriter.clear();
 
         final DOMDataWriteTransaction t = this.domChain.newWriteOnlyTransaction();
         t.delete(LogicalDatastoreType.OPERATIONAL, getYangRibId());