From d4119d0c65ec3552e837bb9a597d8b2c10fc770c Mon Sep 17 00:00:00 2001 From: "Claudio D. Gasparini" Date: Mon, 26 Oct 2015 10:26:35 +0100 Subject: [PATCH] BGP Logs improvement Logs in LocRibWriter and EffectiveRibInWriter were not providing useful information, therefore they are mnodified. Change-Id: I3ee623d287debf33a12c704b031c11d5fe0f5bad Signed-off-by: Claudio D. Gasparini --- .../bgp/rib/impl/AbstractPeerRoleTracker.java | 5 ++- .../bgp/rib/impl/EffectiveRibInWriter.java | 37 ++++++++++++------- .../protocol/bgp/rib/impl/LocRibWriter.java | 4 +- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractPeerRoleTracker.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractPeerRoleTracker.java index 880333f622..85a5c07955 100644 --- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractPeerRoleTracker.java +++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractPeerRoleTracker.java @@ -32,10 +32,11 @@ abstract class AbstractPeerRoleTracker { private static final Logger LOG = LoggerFactory.getLogger(AbstractPeerRoleTracker.class); public void onDataTreeChanged(final DataTreeCandidateNode change, final YangInstanceIdentifier peerPath) { - LOG.debug("Data Changed for Peer role {} path {}", change, peerPath); - // Check for removal final Optional> maybePeerRole = change.getDataAfter(); + LOG.debug("Data Changed for Peer role {} path {}, dataBefore {}, dataAfter {}", change.getIdentifier(), + peerPath , change.getDataBefore(), maybePeerRole); + final PeerRole role; if (maybePeerRole.isPresent()) { final LeafNode peerRoleLeaf = (LeafNode) maybePeerRole.get(); 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 d8c35f432c..818de4cc11 100644 --- 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 @@ -7,6 +7,7 @@ */ package org.opendaylight.protocol.bgp.rib.impl; +import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.base.Verify; import java.util.Collection; @@ -32,9 +33,11 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdent 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.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodes; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -80,7 +83,7 @@ final class EffectiveRibInWriter implements AutoCloseable { } private void processRoute(final DOMDataWriteTransaction tx, final RIBSupport ribSupport, final AbstractImportPolicy policy, final YangInstanceIdentifier routesPath, final DataTreeCandidateNode route) { - LOG.debug("Process route {}", route); + LOG.debug("Process route {}", route.getIdentifier()); final YangInstanceIdentifier routeId = ribSupport.routePath(routesPath, route.getIdentifier()); switch (route.getModificationType()) { case DELETE: @@ -123,31 +126,34 @@ final class EffectiveRibInWriter implements AutoCloseable { final AbstractImportPolicy policy = EffectiveRibInWriter.this.peerPolicyTracker.policyFor(IdentifierUtils.peerId(peerKey)); for (final DataTreeCandidateNode child : children) { - LOG.debug("Process table {} type {}", child, child.getModificationType()); - final YangInstanceIdentifier childPath = tablePath.node(child.getIdentifier()); + final PathArgument childIdentifier = child.getIdentifier(); + final Optional> childDataAfter = child.getDataAfter(); + LOG.debug("Process table {} type {}, dataAfter {}, dataBefore {}", childIdentifier, child + .getModificationType(), childDataAfter, child.getDataBefore()); + final YangInstanceIdentifier childPath = tablePath.node(childIdentifier); switch (child.getModificationType()) { case DELETE: case DISAPPEARED: - tx.delete(LogicalDatastoreType.OPERATIONAL, tablePath.node(child.getIdentifier())); + tx.delete(LogicalDatastoreType.OPERATIONAL, tablePath.node(childIdentifier)); break; case UNMODIFIED: // No-op break; case SUBTREE_MODIFIED: - if (TABLE_ROUTES.equals(child.getIdentifier())) { + if (TABLE_ROUTES.equals(childIdentifier)) { for (final DataTreeCandidateNode route : ribSupport.changedRoutes(child)) { processRoute(tx, ribSupport, policy, childPath, route); } } else { - tx.put(LogicalDatastoreType.OPERATIONAL, childPath, child.getDataAfter().get()); + tx.put(LogicalDatastoreType.OPERATIONAL, childPath, childDataAfter.get()); } break; case APPEARED: case WRITE: - tx.put(LogicalDatastoreType.OPERATIONAL, childPath, child.getDataAfter().get()); + tx.put(LogicalDatastoreType.OPERATIONAL, childPath, childDataAfter.get()); // Routes are special, as they may end up being filtered. The previous put conveniently // ensured that we have them in at target, so a subsequent delete will not fail :) - if (TABLE_ROUTES.equals(child.getIdentifier())) { + if (TABLE_ROUTES.equals(childIdentifier)) { for (final DataTreeCandidateNode route : ribSupport.changedRoutes(child)) { processRoute(tx, ribSupport, policy, childPath, route); } @@ -180,7 +186,8 @@ final class EffectiveRibInWriter implements AutoCloseable { final YangInstanceIdentifier tablePath = effectiveTablePath(peerKey, tableKey); // Create an empty table - ribSupport.clearTable(tx,tablePath); + LOG.trace("Create Empty table", tablePath); + ribSupport.clearTable(tx, tablePath); processTableChildren(tx, ribSupport.getRibSupport(), peerKey, tablePath, table.getChildNodes()); } @@ -208,12 +215,12 @@ final class EffectiveRibInWriter implements AutoCloseable { // filter out any change outside AdjRibsIn final DataTreeCandidateNode ribIn = root.getModifiedChild(ADJRIBIN_NID); if (ribIn == null) { - LOG.debug("Skipping change {}", tc.getRootNode()); + LOG.debug("Skipping change {}", root.getIdentifier()); continue; } final DataTreeCandidateNode tables = ribIn.getModifiedChild(TABLES_NID); if (tables == null) { - LOG.debug("Skipping change {}", tc.getRootNode()); + LOG.debug("Skipping change {}", root.getIdentifier()); continue; } for (final DataTreeCandidateNode table : tables.getChildNodes()) { @@ -233,12 +240,14 @@ final class EffectiveRibInWriter implements AutoCloseable { final PathArgument lastArg = table.getIdentifier(); Verify.verify(lastArg instanceof NodeIdentifierWithPredicates, "Unexpected type %s in path %s", lastArg.getClass(), rootPath); final NodeIdentifierWithPredicates tableKey = (NodeIdentifierWithPredicates) lastArg; - - switch (root.getModificationType()) { + final ModificationType modificationType = root.getModificationType(); + switch (modificationType) { case DELETE: case DISAPPEARED: + final YangInstanceIdentifier effectiveTablePath = effectiveTablePath(peerKey, tableKey); + LOG.debug("Delete Effective Table {} modification type {}, ", effectiveTablePath, modificationType); // delete the corresponding effective table - tx.delete(LogicalDatastoreType.OPERATIONAL, effectiveTablePath(peerKey, tableKey)); + tx.delete(LogicalDatastoreType.OPERATIONAL, effectiveTablePath); break; case SUBTREE_MODIFIED: modifyTable(tx, peerKey, tableKey, table); 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 0c208f3b1b..ee2496b615 100644 --- 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 @@ -158,12 +158,12 @@ final class LocRibWriter implements AutoCloseable, DOMDataTreeChangeListener { // filter out any change outside EffRibsIn final DataTreeCandidateNode ribIn = rootNode.getModifiedChild(EFFRIBIN_NID); if (ribIn == null) { - LOG.debug("Skipping change {}", tc.getRootNode()); + LOG.debug("Skipping change {}", rootNode.getIdentifier()); continue; } final DataTreeCandidateNode table = ribIn.getModifiedChild(TABLES_NID).getModifiedChild(this.tableKey); if (table == null) { - LOG.debug("Skipping change {}", tc.getRootNode()); + LOG.debug("Skipping change {}", rootNode.getIdentifier()); continue; } final NodeIdentifierWithPredicates peerKey = IdentifierUtils.peerKey(rootPath); -- 2.36.6