Topology Manager to skip redundant edge updates 23/1623/1
authorAlessandro Boch <aboch@cisco.com>
Wed, 2 Oct 2013 21:22:38 +0000 (14:22 -0700)
committerAlessandro Boch <aboch@cisco.com>
Wed, 2 Oct 2013 21:31:07 +0000 (14:31 -0700)
ISSUE: In a cluster of controllers when one controller is brought down, when the switches
that were actively controlled by this node switch-over to another node in the cluster, on the
new node Connection Manager let openflow plugin Controller know the cluster view has changed.
OF plugin Controller then replay the switch added event for all the local tcp conencted
switches to InventoryShim. InventoryShim checks with CM service if the switches are now
to be considered local to this controller, they are, so it updates inventory service listeners
inside OF plugin (these are cluster unaware classes which need to know the switch is there now,
like data packet serv, discovery, of stats mgr, ...). These moduels inside the plugin during their
processing because this (to them) newly added switches generate notifications toward SAL in their
respective service. Listeners above SAL are cluster aware, they are already aware of those switches,
their properties and edges and so on, they should discard these updates. But in most of the cases
they don't, they accept these redundant updates upfront. As a consequence they generate unnecessary
updates toward their service listeners, causing unnecessary chains of events and recomputations.

CHANGE: This change is to fix the above issue in the topology service, in topology manager bundle.

Change-Id: I04d1fa81758238dfb9ada9940abc3d07ed1fc9cb
Signed-off-by: Alessandro Boch <aboch@cisco.com>
opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImpl.java

index b905a8982e83113e10b23261ab2d3045f1142c54..526ba41c354c3166db9ba26f18e55b1483c1ab36 100644 (file)
@@ -69,7 +69,7 @@ import org.slf4j.LoggerFactory;
  * topology database and notifies all the listeners of topology changes.
  */
 public class TopologyManagerImpl implements
-        ICacheUpdateAware,
+        ICacheUpdateAware<Object, Object>,
         ITopologyManager,
         IConfigurationContainerAware,
         IListenTopoUpdates,
@@ -200,7 +200,7 @@ public class TopologyManagerImpl implements
         notifyThread = new Thread(new TopologyNotify(notifyQ));
     }
 
-    @SuppressWarnings({ "unchecked", "deprecation" })
+    @SuppressWarnings({ "unchecked" })
     private void allocateCaches() {
         try {
             this.edgesDB =
@@ -243,7 +243,7 @@ public class TopologyManagerImpl implements
         }
     }
 
-    @SuppressWarnings({ "unchecked", "deprecation" })
+    @SuppressWarnings({ "unchecked" })
     private void retrieveCaches() {
         if (this.clusterContainerService == null) {
             log.error("Cluster Services is null, can't retrieve caches.");
@@ -531,6 +531,11 @@ public class TopologyManagerImpl implements
     private TopoEdgeUpdate edgeUpdate(Edge e, UpdateType type, Set<Property> props) {
         switch (type) {
         case ADDED:
+            // Avoid redundant update as notifications trigger expensive tasks
+            if (edgesDB.containsKey(e)) {
+                log.trace("Skipping redundant edge addition: {}", e);
+                return null;
+            }
             // Ensure that both tail and head node connectors exist.
             if (!nodeConnectorsExist(e)) {
                 log.warn("Ignore edge that contains invalid node connector: {}", e);
@@ -896,7 +901,7 @@ public class TopologyManagerImpl implements
     public void entryUpdated(final Object key, final Object new_value, final String cacheName, final boolean originLocal) {
         if (cacheName.equals(TOPOEDGESDB)) {
             final Edge e = (Edge) key;
-            log.trace("Edge {} CHANGED isLocal:{}", e, originLocal);
+            log.trace("Edge {} UPDATED isLocal:{}", e, originLocal);
             final Set<Property> props = (Set<Property>) new_value;
             edgeUpdateClusterWide(e, UpdateType.CHANGED, props, originLocal);
         }