Optimize whole-table overwrite 38/79938/15
authorRobert Varga <robert.varga@pantheon.tech>
Sun, 27 Jan 2019 20:55:01 +0000 (21:55 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 31 Jan 2019 00:23:27 +0000 (01:23 +0100)
When a table is overwritten, we can treat it as a delete followed
by a fill of after-routes. This simplifies the logic quite a bit
and speeds it up. It also allows parts of it to operate on
before/after data rather than a DataTreeCandidateNode, hence it
is a reusable piece we'll need for LLGR_STALE nodes.

JIRA: BGPCEP-495
Change-Id: Ib84296652d84c4616d617583f14e5395058eac1c
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/EffectiveRibInWriter.java

index eb98ce556589acef9413be73d315f03e4177ceb9..f52155d0997a4baca2237e515d2710530189551f 100644 (file)
@@ -10,6 +10,7 @@ package org.opendaylight.protocol.bgp.rib.impl;
 import static com.google.common.base.Verify.verify;
 import static java.util.Objects.requireNonNull;
 import static org.opendaylight.protocol.bgp.rib.spi.RIBNodeIdentifiers.ADJRIBIN;
+import static org.opendaylight.protocol.bgp.rib.spi.RIBNodeIdentifiers.ATTRIBUTES;
 import static org.opendaylight.protocol.bgp.rib.spi.RIBNodeIdentifiers.EFFRIBIN;
 import static org.opendaylight.protocol.bgp.rib.spi.RIBNodeIdentifiers.ROUTES;
 import static org.opendaylight.protocol.bgp.rib.spi.RIBNodeIdentifiers.TABLES;
@@ -67,6 +68,7 @@ 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.DataContainerChild;
 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;
@@ -205,6 +207,60 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
         }
     }
 
+    @Override
+    public synchronized void close() {
+        if (this.reg != null) {
+            this.reg.close();
+            this.reg = null;
+        }
+        if (this.submitted != null) {
+            try {
+                this.submitted.get();
+            } catch (final InterruptedException | ExecutionException throwable) {
+                LOG.error("Write routes failed", throwable);
+            }
+        }
+        if (this.chain != null) {
+            this.chain.close();
+            this.chain = null;
+        }
+        this.prefixesReceived.values().forEach(LongAdder::reset);
+        this.prefixesInstalled.values().forEach(LongAdder::reset);
+    }
+
+    @Override
+    public long getPrefixedReceivedCount(final TablesKey tablesKey) {
+        final LongAdder counter = this.prefixesReceived.get(tablesKey);
+        if (counter == null) {
+            return 0;
+        }
+        return counter.longValue();
+    }
+
+    @Override
+    public Set<TablesKey> getTableKeys() {
+        return ImmutableSet.copyOf(this.prefixesReceived.keySet());
+    }
+
+    @Override
+    public boolean isSupported(final TablesKey tablesKey) {
+        return this.prefixesReceived.containsKey(tablesKey);
+    }
+
+    @Override
+    public long getPrefixedInstalledCount(final TablesKey tablesKey) {
+        final LongAdder counter = this.prefixesInstalled.get(tablesKey);
+        if (counter == null) {
+            return 0;
+        }
+        return counter.longValue();
+    }
+
+    @Override
+    public long getTotalPrefixesInstalled() {
+        return this.prefixesInstalled.values().stream().mapToLong(LongAdder::longValue).sum();
+    }
+
     @GuardedBy("this")
     private void changeDataTree(final DOMDataWriteTransaction tx, final YangInstanceIdentifier rootPath,
             final DataTreeCandidateNode root, final DataTreeCandidateNode table) {
@@ -244,28 +300,8 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
 
     private void deleteTable(final DOMDataWriteTransaction tx, final RIBSupportContext ribContext,
             final YangInstanceIdentifier effectiveTablePath, final DataTreeCandidateNode table) {
-        // 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);
+        onDeleteTable(ribContext.getRibSupport(), effectiveTablePath, table.getDataBefore());
         tx.delete(LogicalDatastoreType.OPERATIONAL, effectiveTablePath);
     }
 
@@ -278,8 +314,51 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
     private void writeTable(final DOMDataWriteTransaction tx, final RIBSupportContext ribContext,
             final YangInstanceIdentifier effectiveTablePath, final DataTreeCandidateNode table) {
         LOG.debug("Write Effective Table {}", effectiveTablePath);
-        ribContext.createEmptyTableStructure(tx, effectiveTablePath);
-        processTableChildren(tx, ribContext.getRibSupport(), effectiveTablePath, table.getChildNodes());
+        onDeleteTable(ribContext.getRibSupport(), effectiveTablePath, table.getDataBefore());
+
+        final Optional<NormalizedNode<?, ?>> maybeTableAfter = table.getDataAfter();
+        if (maybeTableAfter.isPresent()) {
+            final MapEntryNode tableAfter = extractMapEntry(maybeTableAfter);
+            ribContext.createEmptyTableStructure(tx, effectiveTablePath);
+
+            final Optional<DataContainerChild<?, ?>> maybeAttrsAfter = tableAfter.getChild(ATTRIBUTES);
+            if (maybeAttrsAfter.isPresent()) {
+                final ContainerNode attrsAfter = extractContainer(maybeAttrsAfter);
+                tx.put(LogicalDatastoreType.OPERATIONAL, effectiveTablePath.node(ATTRIBUTES), attrsAfter);
+            }
+
+            final RIBSupport<?, ?, ?, ?> ribSupport = ribContext.getRibSupport();
+            final Optional<NormalizedNode<?, ?>> maybeRoutesAfter = findRoutesMap(ribSupport, NormalizedNodes.findNode(
+                NormalizedNodes.findNode(tableAfter, ROUTES)));
+            if (maybeRoutesAfter.isPresent()) {
+                final YangInstanceIdentifier routesPath = routeMapPath(ribSupport, effectiveTablePath);
+                for (final MapEntryNode routeAfter : extractMap(maybeRoutesAfter).getValue()) {
+                    writeRoute(tx, ribSupport, routesPath.node(routeAfter.getIdentifier()), Optional.empty(),
+                        routeAfter);
+                }
+            }
+        }
+    }
+
+    // Performs house-keeping when the contents of a table is deleted
+    private void onDeleteTable(final RIBSupport<?, ?, ?, ?> ribSupport, final YangInstanceIdentifier effectiveTablePath,
+            final Optional<NormalizedNode<?, ?>> tableBefore) {
+        // Routes are special in that we need to process the to keep our counters accurate
+        final Optional<NormalizedNode<?, ?>> maybeRoutesBefore = findRoutesMap(ribSupport,
+                NormalizedNodes.findNode(tableBefore, ROUTES));
+        if (maybeRoutesBefore.isPresent()) {
+            final Collection<MapEntryNode> deletedRoutes = extractMap(maybeRoutesBefore).getValue();
+            if (ribSupport.getSafi() == RouteTargetConstrainSubsequentAddressFamily.class) {
+                final YangInstanceIdentifier routesPath = routeMapPath(ribSupport, effectiveTablePath);
+                for (final 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());
+        }
     }
 
     private void processTableChildren(final DOMDataWriteTransaction tx, final RIBSupport<?, ?, ?, ?> ribSupport,
@@ -362,11 +441,10 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
             final YangInstanceIdentifier routesPath, final DataTreeCandidateNode route) {
         LOG.debug("Process route {}", route.getIdentifier());
         final YangInstanceIdentifier routePath = ribSupport.routePath(routesPath, route.getIdentifier());
-        final TablesKey tablesKey = ribSupport.getTablesKey();
         switch (route.getModificationType()) {
             case DELETE:
             case DISAPPEARED:
-                deleteRoute(tx, ribSupport, routePath, route.getDataBefore().orElse(null), tablesKey);
+                deleteRoute(tx, ribSupport, routePath, route.getDataBefore().orElse(null));
                 break;
             case UNMODIFIED:
                 // No-op
@@ -374,45 +452,7 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
             case APPEARED:
             case SUBTREE_MODIFIED:
             case WRITE:
-                CountersUtil.increment(this.prefixesReceived.get(tablesKey), tablesKey);
-                // Lookup per-table attributes from RIBSupport
-                final ContainerNode advertisedAttrs = (ContainerNode) NormalizedNodes.findNode(route.getDataAfter(),
-                    ribSupport.routeAttributesIdentifier()).orElse(null);
-                final Attributes routeAttrs = ribSupport.attributeFromContainerNode(advertisedAttrs);
-                final Optional<Attributes> optEffAtt;
-                // In case we want to add LLGR_STALE we do not process route through policies since it may be
-                // considered as received with LLGR_STALE from peer which is not true.
-                final boolean longLivedStale = false;
-                if (longLivedStale) {
-                    // LLGR procedures are in effect. If the route is tagged with NO_LLGR, it needs to be removed.
-                    final List<Communities> effCommunities = routeAttrs.getCommunities();
-                    if (effCommunities != null && effCommunities.contains(CommunityUtil.NO_LLGR)) {
-                        deleteRoute(tx, ribSupport, routePath, route.getDataBefore().orElse(null), tablesKey);
-                        return;
-                    }
-                    optEffAtt = Optional.of(wrapLongLivedStale(routeAttrs));
-                } else {
-                    final Class<? extends AfiSafiType> afiSafiType
-                        = tableTypeRegistry.getAfiSafiType(ribSupport.getTablesKey()).get();
-                    optEffAtt = this.ribPolicies
-                        .applyImportPolicies(this.peerImportParameters, routeAttrs, afiSafiType);
-                }
-                if (!optEffAtt.isPresent()) {
-                    deleteRoute(tx, ribSupport, routePath, route.getDataBefore().orElse(null), tablesKey);
-                    return;
-                }
-                final NormalizedNode<?, ?> routeChanged = route.getDataAfter().get();
-                handleRouteTarget(ModificationType.WRITE, ribSupport, routePath, routeChanged);
-                tx.put(LogicalDatastoreType.OPERATIONAL, routePath, routeChanged);
-                CountersUtil.increment(this.prefixesInstalled.get(tablesKey), tablesKey);
-
-                final YangInstanceIdentifier attPath = routePath.node(ribSupport.routeAttributesIdentifier());
-                final Attributes attToStore = optEffAtt.get();
-                if(!attToStore.equals(routeAttrs)) {
-                    final ContainerNode finalAttribute = ribSupport.attributeToContainerNode(attPath, attToStore);
-                    tx.put(LogicalDatastoreType.OPERATIONAL, attPath, finalAttribute);
-                }
-                break;
+                writeRoute(tx, ribSupport, routePath, route.getDataBefore(), route.getDataAfter().get());
             default:
                 LOG.warn("Ignoring unhandled route {}", route);
                 break;
@@ -420,13 +460,57 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
     }
 
     private void deleteRoute(final DOMDataWriteTransaction tx, final RIBSupport<?, ?, ?, ?> ribSupport,
-            final YangInstanceIdentifier routeIdPath, final NormalizedNode<?, ?> route, final TablesKey tablesKey) {
+            final YangInstanceIdentifier routeIdPath, final NormalizedNode<?, ?> route) {
         handleRouteTarget(ModificationType.DELETE, ribSupport, routeIdPath, route);
         tx.delete(LogicalDatastoreType.OPERATIONAL, routeIdPath);
         LOG.debug("Route deleted. routeId={}", routeIdPath);
+        final TablesKey tablesKey = ribSupport.getTablesKey();
         CountersUtil.decrement(this.prefixesInstalled.get(tablesKey), tablesKey);
     }
 
+    private void writeRoute(final DOMDataWriteTransaction tx, final RIBSupport<?, ?, ?, ?> ribSupport,
+            final YangInstanceIdentifier routePath, final Optional<NormalizedNode<?, ?>> routeBefore,
+            final NormalizedNode<?, ?> routeAfter) {
+        final TablesKey tablesKey = ribSupport.getTablesKey();
+        CountersUtil.increment(this.prefixesReceived.get(tablesKey), tablesKey);
+        // Lookup per-table attributes from RIBSupport
+        final ContainerNode advertisedAttrs = (ContainerNode) NormalizedNodes.findNode(routeAfter,
+            ribSupport.routeAttributesIdentifier()).orElse(null);
+        final Attributes routeAttrs = ribSupport.attributeFromContainerNode(advertisedAttrs);
+        final Optional<Attributes> optEffAtt;
+        // In case we want to add LLGR_STALE we do not process route through policies since it may be
+        // considered as received with LLGR_STALE from peer which is not true.
+        final boolean longLivedStale = false;
+        if (longLivedStale) {
+            // LLGR procedures are in effect. If the route is tagged with NO_LLGR, it needs to be removed.
+            final List<Communities> effCommunities = routeAttrs.getCommunities();
+            if (effCommunities != null && effCommunities.contains(CommunityUtil.NO_LLGR)) {
+                deleteRoute(tx, ribSupport, routePath, routeBefore.orElse(null));
+                return;
+            }
+            optEffAtt = Optional.of(wrapLongLivedStale(routeAttrs));
+        } else {
+            final Class<? extends AfiSafiType> afiSafiType
+                = tableTypeRegistry.getAfiSafiType(ribSupport.getTablesKey()).get();
+            optEffAtt = this.ribPolicies
+                .applyImportPolicies(this.peerImportParameters, routeAttrs, afiSafiType);
+        }
+        if (!optEffAtt.isPresent()) {
+            deleteRoute(tx, ribSupport, routePath, routeBefore.orElse(null));
+            return;
+        }
+        handleRouteTarget(ModificationType.WRITE, ribSupport, routePath, routeAfter);
+        tx.put(LogicalDatastoreType.OPERATIONAL, routePath, routeAfter);
+        CountersUtil.increment(this.prefixesInstalled.get(tablesKey), tablesKey);
+
+        final YangInstanceIdentifier attPath = routePath.node(ribSupport.routeAttributesIdentifier());
+        final Attributes attToStore = optEffAtt.get();
+        if (!attToStore.equals(routeAttrs)) {
+            final ContainerNode finalAttribute = ribSupport.attributeToContainerNode(attPath, attToStore);
+            tx.put(LogicalDatastoreType.OPERATIONAL, attPath, finalAttribute);
+        }
+    }
+
     private void addRouteTarget(final RouteTargetConstrainRoute rtc) {
         final RouteTarget rtMembership = RouteTargetMembeshipUtil.getRT(rtc);
         if (PeerRole.Ebgp != this.peerImportParameters.getFromPeerRole()) {
@@ -483,6 +567,7 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
                 .path.attributes.AttributesBuilder(attrs).setCommunities(newCommunities).build();
     }
 
+    // XXX: this should be moved to YangInstanceIdentifier at some point
     private static YangInstanceIdentifier concat(final YangInstanceIdentifier parent, final List<PathArgument> args) {
         YangInstanceIdentifier ret = parent;
         for (PathArgument arg : args) {
@@ -495,57 +580,31 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn
         return this.effRibTables.node(TABLES).node(tableKey);
     }
 
-    @Override
-    public synchronized void close() {
-        if (this.reg != null) {
-            this.reg.close();
-            this.reg = null;
-        }
-        if (this.submitted != null) {
-            try {
-                this.submitted.get();
-            } catch (final InterruptedException | ExecutionException throwable) {
-                LOG.error("Write routes failed", throwable);
-            }
-        }
-        if (this.chain != null) {
-            this.chain.close();
-            this.chain = null;
-        }
-        this.prefixesReceived.values().forEach(LongAdder::reset);
-        this.prefixesInstalled.values().forEach(LongAdder::reset);
-    }
-
-    @Override
-    public long getPrefixedReceivedCount(final TablesKey tablesKey) {
-        final LongAdder counter = this.prefixesReceived.get(tablesKey);
-        if (counter == null) {
-            return 0;
-        }
-        return counter.longValue();
+    private static YangInstanceIdentifier routeMapPath(final RIBSupport<?, ?, ?, ?> ribSupport,
+            final YangInstanceIdentifier tablePath) {
+        return concat(tablePath.node(ROUTES), ribSupport.relativeRoutesPath());
     }
 
-    @Override
-    public Set<TablesKey> getTableKeys() {
-        return ImmutableSet.copyOf(this.prefixesReceived.keySet());
+    private static Optional<NormalizedNode<?, ?>> findRoutesMap(final RIBSupport<?, ?, ?, ?> ribSupport,
+            final Optional<NormalizedNode<?, ?>> optRoutes) {
+        return NormalizedNodes.findNode(optRoutes, ribSupport.relativeRoutesPath());
     }
 
-    @Override
-    public boolean isSupported(final TablesKey tablesKey) {
-        return this.prefixesReceived.containsKey(tablesKey);
+    private static ContainerNode extractContainer(final Optional<? extends NormalizedNode<?, ?>> optNode) {
+        final NormalizedNode<?, ?> node = optNode.get();
+        verify(node instanceof ContainerNode, "Expected ContainerNode, got %s", node);
+        return (ContainerNode) node;
     }
 
-    @Override
-    public long getPrefixedInstalledCount(final TablesKey tablesKey) {
-        final LongAdder counter = this.prefixesInstalled.get(tablesKey);
-        if (counter == null) {
-            return 0;
-        }
-        return counter.longValue();
+    private static MapNode extractMap(final Optional<? extends NormalizedNode<?, ?>> optNode) {
+        final NormalizedNode<?, ?> node = optNode.get();
+        verify(node instanceof MapNode, "Expected MapNode, got %s", node);
+        return (MapNode) node;
     }
 
-    @Override
-    public long getTotalPrefixesInstalled() {
-        return this.prefixesInstalled.values().stream().mapToLong(LongAdder::longValue).sum();
+    private static MapEntryNode extractMapEntry(final Optional<? extends NormalizedNode<?, ?>> optNode) {
+        final NormalizedNode<?, ?> node = optNode.get();
+        verify(node instanceof MapEntryNode, "Expected MapEntryNode, got %s", node);
+        return (MapEntryNode) node;
     }
 }