Optimize deletion of entire adj-rib-in table

In case we are deleting an entire table, there is no point in
issuing delete requests for its children.

This refactor eliminates the delete operations, invokes checks
for route target table only once and updates the counters only
once.

This is a stepping stone towards being able to handle LLGR_STALE
routes in an atomic fashion.

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

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/CountersUtil.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/CountersUtil.java
index 63d8beb2e6cd15cc3d3bb0ac2bda5f31a681f84a..22b4148be2bf213da8acbf49b5f29068c5d14427 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/CountersUtil.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/CountersUtil.java
@@ -5,7 +5,6 @@
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
  * and is available at http://www.eclipse.org/legal/epl-v10.html
  */
-
 package org.opendaylight.protocol.bgp.rib.impl;
 
 import java.util.concurrent.atomic.LongAdder;
@@ -32,12 +31,8 @@ public final class CountersUtil {
      * @param counter   counter
      * @param tablesKey tablesKey Type
      */
-    static void increment(@Nullable final LongAdder counter, @Nonnull TablesKey tablesKey) {
-        if (counter != null) {
-            counter.increment();
-            return;
-        }
-        LOG.warn("Family {} not supported", tablesKey);
+    static void increment(@Nullable final LongAdder counter, @Nonnull final TablesKey tablesKey) {
+        add(counter, tablesKey, 1);
     }
 
     /**
@@ -46,11 +41,21 @@ public final class CountersUtil {
      * @param counter   counter
      * @param tablesKey tablesKey Type
      */
-    static void decrement(@Nullable final LongAdder counter, @Nonnull TablesKey tablesKey) {
+    static void decrement(@Nullable final LongAdder counter, @Nonnull final TablesKey tablesKey) {
+        add(counter, tablesKey, -1);
+    }
+
+    /**
+     * Add specified valut to the counter if supported, otherwise produce a warn.
+     *
+     * @param counter   counter
+     * @param tablesKey tablesKey Type
+     */
+    static void add(@Nullable final LongAdder counter, @Nonnull final TablesKey tablesKey, final long amount) {
         if (counter != null) {
-            counter.decrement();
-            return;
+            counter.add(amount);
+        } else {
+            LOG.warn("Family {} not supported", tablesKey);
         }
-        LOG.warn("Family {} not supported", tablesKey);
     }
 }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/EffectiveRibInWriter.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/EffectiveRibInWriter.java
index b657428940350a16de11268808ee22b85f5c38ad..eb98ce556589acef9413be73d315f03e4177ceb9 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/EffectiveRibInWriter.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/EffectiveRibInWriter.java
@@ -67,6 +67,8 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
+import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode;
+import org.opendaylight.yangtools.yang.data.api.schema.MapNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodes;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate;
@@ -242,7 +244,27 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
 
     private void deleteTable(final DOMDataWriteTransaction tx, final RIBSupportContext ribContext,
             final YangInstanceIdentifier effectiveTablePath, final DataTreeCandidateNode table) {
-        processTableChildren(tx, ribContext.getRibSupport(), effectiveTablePath, table.getChildNodes());
+        // Routes are special in that we need to process the to keep our counters accurate
+        final RIBSupport<?, ?, ?, ?> ribSupport = ribContext.getRibSupport();
+        final Optional<NormalizedNode<?, ?>> maybeRoutesBefore = NormalizedNodes.findNode(
+                NormalizedNodes.findNode(table.getDataBefore(), ROUTES), ribSupport.relativeRoutesPath());
+        if (maybeRoutesBefore.isPresent()) {
+            final NormalizedNode<?, ?> routesBefore = maybeRoutesBefore.get();
+            verify(routesBefore instanceof MapNode, "Expected a MapNode, have %s", routesBefore);
+            final Collection<MapEntryNode> deletedRoutes = ((MapNode) routesBefore).getValue();
+            if (ribSupport.getSafi() == RouteTargetConstrainSubsequentAddressFamily.class) {
+                final YangInstanceIdentifier routesPath = concat(effectiveTablePath.node(ROUTES),
+                    ribSupport.relativeRoutesPath());
+                for (MapEntryNode routeBefore : deletedRoutes) {
+                    deleteRouteTarget(ribSupport, routesPath.node(routeBefore.getIdentifier()), routeBefore);
+                }
+                this.rtMembershipsUpdated = true;
+            }
+
+            final TablesKey tablesKey = ribSupport.getTablesKey();
+            CountersUtil.add(prefixesInstalled.get(tablesKey), tablesKey, -deletedRoutes.size());
+        }
+
         LOG.debug("Delete Effective Table {}", effectiveTablePath);
         tx.delete(LogicalDatastoreType.OPERATIONAL, effectiveTablePath);
     }
@@ -405,22 +427,36 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
         CountersUtil.decrement(this.prefixesInstalled.get(tablesKey), tablesKey);
     }
 
+    private void addRouteTarget(final RouteTargetConstrainRoute rtc) {
+        final RouteTarget rtMembership = RouteTargetMembeshipUtil.getRT(rtc);
+        if (PeerRole.Ebgp != this.peerImportParameters.getFromPeerRole()) {
+            this.rtCache.cacheRoute(rtc);
+        }
+        this.rtMemberships.add(rtMembership);
+    }
+
+    private void deleteRouteTarget(final RIBSupport<?, ?, ?, ?> ribSupport, final YangInstanceIdentifier routeIdPath,
+            final NormalizedNode<?, ?> route) {
+        deleteRouteTarget((RouteTargetConstrainRoute) ribSupport.fromNormalizedNode(routeIdPath, route));
+    }
+
+    private void deleteRouteTarget(final RouteTargetConstrainRoute rtc) {
+        final RouteTarget rtMembership = RouteTargetMembeshipUtil.getRT(rtc);
+        if (PeerRole.Ebgp != this.peerImportParameters.getFromPeerRole()) {
+            this.rtCache.uncacheRoute(rtc);
+        }
+        this.rtMemberships.remove(rtMembership);
+    }
+
     private void handleRouteTarget(final ModificationType modificationType, final RIBSupport<?, ?, ?, ?> ribSupport,
             final YangInstanceIdentifier routeIdPath, final NormalizedNode<?, ?> route) {
         if (ribSupport.getSafi() == RouteTargetConstrainSubsequentAddressFamily.class) {
             final RouteTargetConstrainRoute rtc =
                 (RouteTargetConstrainRoute) ribSupport.fromNormalizedNode(routeIdPath, route);
-            final RouteTarget rtMembership = RouteTargetMembeshipUtil.getRT(rtc);
             if (ModificationType.DELETE == modificationType) {
-                if (PeerRole.Ebgp != this.peerImportParameters.getFromPeerRole()) {
-                    this.rtCache.uncacheRoute(rtc);
-                }
-                this.rtMemberships.remove(rtMembership);
+                deleteRouteTarget(rtc);
             } else {
-                if (PeerRole.Ebgp != this.peerImportParameters.getFromPeerRole()) {
-                    this.rtCache.cacheRoute(rtc);
-                }
-                this.rtMemberships.add(rtMembership);
+                addRouteTarget(rtc);
             }
             this.rtMembershipsUpdated = true;
         }
@@ -447,6 +483,14 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
                 .path.attributes.AttributesBuilder(attrs).setCommunities(newCommunities).build();
     }
 
+    private static YangInstanceIdentifier concat(final YangInstanceIdentifier parent, final List<PathArgument> args) {
+        YangInstanceIdentifier ret = parent;
+        for (PathArgument arg : args) {
+            ret = ret.node(arg);
+        }
+        return ret;
+    }
+
     private YangInstanceIdentifier effectiveTablePath(final NodeIdentifierWithPredicates tableKey) {
         return this.effRibTables.node(TABLES).node(tableKey);
     }

diff --git a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/AbstractRIBSupport.java b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/AbstractRIBSupport.java
index d20109195944b7c8e920e93be693aa891da28b49..7217bf62bf9d5038af0ac55e12c680dd0594ffe7 100644 (file)
--- a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/AbstractRIBSupport.java
+++ b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/AbstractRIBSupport.java
@@ -19,8 +19,10 @@ import com.google.common.annotations.Beta;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.collect.ImmutableList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 import java.util.Optional;
 import javax.annotation.Nonnull;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
@@ -127,6 +129,7 @@ public abstract class AbstractRIBSupport<
     protected final BindingNormalizedNodeSerializer mappingService;
     protected final YangInstanceIdentifier routeDefaultYii;
     private final TablesKey tk;
+    private final ImmutableList<PathArgument> relativeRoutesPath;
 
     /**
      * Default constructor. Requires the QName of the container augmented under the routes choice
@@ -151,15 +154,15 @@ public abstract class AbstractRIBSupport<
             final Class<? extends SubsequentAddressFamily> safiClass,
             final QName destContainerQname) {
         final QNameModule module = BindingReflections.getQNameModule(cazeClass);
-        this.routesContainerIdentifier
-                = new NodeIdentifier(BindingReflections.findQName(containerClass).withModule(module));
-        this.routeAttributesIdentifier = new NodeIdentifier(Attributes.QNAME.withModule(module));
+        this.routesContainerIdentifier = NodeIdentifier.create(
+            BindingReflections.findQName(containerClass).withModule(module));
+        this.routeAttributesIdentifier = NodeIdentifier.create(Attributes.QNAME.withModule(module));
         this.cazeClass = requireNonNull(cazeClass);
         this.mappingService = requireNonNull(mappingService);
         this.containerClass = requireNonNull(containerClass);
         this.listClass = requireNonNull(listClass);
         this.routeQname = BindingReflections.findQName(listClass).withModule(module);
-        this.routesListIdentifier = new NodeIdentifier(this.routeQname);
+        this.routesListIdentifier = NodeIdentifier.create(this.routeQname);
         this.tk = new TablesKey(afiClass, safiClass);
         this.emptyTable = (MapEntryNode) this.mappingService
                 .toNormalizedNode(TABLES_II, new TablesBuilder().withKey(tk)
@@ -167,9 +170,9 @@ public abstract class AbstractRIBSupport<
                                 .rev180329.rib.tables.AttributesBuilder().build()).build()).getValue();
         this.afiClass = afiClass;
         this.safiClass = safiClass;
-        this.destinationNid = new NodeIdentifier(destContainerQname);
+        this.destinationNid = NodeIdentifier.create(destContainerQname);
         this.pathIdQname = QName.create(routeQName(), "path-id").intern();
-        this.pathIdNid = new NodeIdentifier(this.pathIdQname);
+        this.pathIdNid = NodeIdentifier.create(this.pathIdQname);
         this.routeKeyQname = QName.create(routeQName(), ROUTE_KEY).intern();
         this.prefixTypeNid = NodeIdentifier.create(QName.create(destContainerQname, "prefix").intern());
         this.rdNid = NodeIdentifier.create(QName.create(destContainerQname, "route-distinguisher").intern());
@@ -182,9 +185,10 @@ public abstract class AbstractRIBSupport<
                         .node(TABLES)
                         .node(TABLES)
                         .node(ROUTES)
-                        .node(BindingReflections.findQName(containerClass).withModule(module))
-                        .node(this.routeQname)
-                        .node(this.routeQname).build();
+                        .node(this.routesContainerIdentifier)
+                        .node(this.routesListIdentifier)
+                        .node(this.routesListIdentifier).build();
+        this.relativeRoutesPath = ImmutableList.of(routesContainerIdentifier, routesListIdentifier);
     }
 
     @Override
@@ -275,7 +279,7 @@ public abstract class AbstractRIBSupport<
      *
      * @return Container identifier, may not be null.
      */
-    protected final NodeIdentifier routesContainerIdentifier() {
+    public final NodeIdentifier routesContainerIdentifier() {
         return this.routesContainerIdentifier;
     }
 
@@ -389,6 +393,11 @@ public abstract class AbstractRIBSupport<
         return routesYangInstanceIdentifier(routesTablePaths.node(ROUTES));
     }
 
+    @Override
+    public final List<PathArgument> relativeRoutesPath() {
+        return relativeRoutesPath;
+    }
+
     @Override
     public final InstanceIdentifier<R> createRouteIdentifier(
             final KeyedInstanceIdentifier<Tables, TablesKey> tableIId, final I key) {

diff --git a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/RIBSupport.java b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/RIBSupport.java
index fbe9cadc54ff11ba98d7a663835d3862e41517cb..694cfb1ef95f93d87d7d6916d2fefef11cfa02f6 100644 (file)
--- a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/RIBSupport.java
+++ b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/RIBSupport.java
@@ -198,6 +198,14 @@ public interface RIBSupport<
     @Nonnull
     YangInstanceIdentifier routesPath(@Nonnull YangInstanceIdentifier routesPath);
 
+    /**
+     * Return the relative path from the generic routes container to the AFI/SAFI specific route list.
+     *
+     * @return Relative path.
+     */
+    @Nonnull
+    List<PathArgument> relativeRoutesPath();
+
     /**
      * To send routes out, we'd need to transform the DOM representation of route to
      * binding-aware format. This needs to be done per each AFI/SAFI.