Merge "Fix corner case in IFMapDependencyTracker" into R2.1

Juniper · Jul 9, 2015 · e4af211 · e4af211
2 parents 8956c04 + ed4053f
commit e4af211
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 20 deletions.
diff --git a/src/bgp/test/bgp_config_listener_test.cc b/src/bgp/test/bgp_config_listener_test.cc
@@ -245,9 +245,8 @@ TEST_F(BgpConfigListenerTest, IrrelevantNodeEvent) {
 }
 
 //
-// Multiple node events for the same node should not cause it to get on the
-// change list multiple times. However, it should get added to the node list
-// multiple times per the current implementation.
+// Multiple node events for same node should cause it to get on the change
+// list multiple times.
 //
 TEST_F(BgpConfigListenerTest, DuplicateNodeEvent) {
     string content = ReadFile("controller/src/bgp/testdata/config_listener_test_1.xml");
@@ -261,13 +260,13 @@ TEST_F(BgpConfigListenerTest, DuplicateNodeEvent) {
     ifmap_test_util::IFMapNodeNotify(&db_, "bgp-router", id_name);
     task_util::WaitForIdle();
 
-    TASK_UTIL_EXPECT_EQ(1, GetChangeListCount());
+    TASK_UTIL_EXPECT_EQ(2, GetChangeListCount());
     TASK_UTIL_EXPECT_EQ(2, GetNodeListCount());
     TASK_UTIL_EXPECT_EQ(0, GetEdgeListCount());
 
     PerformChangeListPropagation();
-    TASK_UTIL_EXPECT_EQ(4, GetChangeListCount());
-    TASK_UTIL_EXPECT_EQ(1, GetChangeListCount("bgp-router"));
+    TASK_UTIL_EXPECT_EQ(5, GetChangeListCount());
+    TASK_UTIL_EXPECT_EQ(2, GetChangeListCount("bgp-router"));
     TASK_UTIL_EXPECT_EQ(3, GetChangeListCount("bgp-peering"));
 
     ResumeChangeListPropagation();
@@ -350,12 +349,12 @@ TEST_F(BgpConfigListenerTest, DeletedNodeEvent3) {
         ifmap_test_util::IFMapNodeLookup(&db_, "virtual-network", "red");
     TASK_UTIL_EXPECT_TRUE(node->IsDeleted());
 
-    TASK_UTIL_EXPECT_EQ(1, GetChangeListCount());
+    TASK_UTIL_EXPECT_EQ(2, GetChangeListCount());
     TASK_UTIL_EXPECT_EQ(1, GetNodeListCount());
     TASK_UTIL_EXPECT_EQ(0, GetEdgeListCount());
 
     PerformChangeListPropagation();
-    TASK_UTIL_EXPECT_EQ(1, GetChangeListCount());
+    TASK_UTIL_EXPECT_EQ(2, GetChangeListCount());
 
     ResumeChangeListPropagation();
     TASK_UTIL_EXPECT_EQ(0, GetChangeListCount());

diff --git a/src/ifmap/ifmap_dependency_tracker.cc b/src/ifmap/ifmap_dependency_tracker.cc
@@ -104,7 +104,6 @@ void IFMapDependencyTracker::PropagateChanges() {
 // list.
 //
 void IFMapDependencyTracker::Clear() {
-    vertex_list_.clear();
     node_list_.clear();
     edge_list_.clear();
 }
@@ -209,15 +208,9 @@ void IFMapDependencyTracker::PropagateEdge(
 }
 
 //
-// Add the IFMapNode to the change list it's not already on there.
+// Add the IFMapNode to the change list.
 //
 void IFMapDependencyTracker::AddChangeEvent(IFMapNode *node) {
-    ostringstream identifier;
-    identifier << node->table()->Typename() << ':' << node->name();
-    if (vertex_list_.count(identifier.str()) > 0) {
-        return;
-    }
     observer_(node);
-    vertex_list_.insert(identifier.str());
 }
 
diff --git a/src/ifmap/ifmap_dependency_tracker.h b/src/ifmap/ifmap_dependency_tracker.h
@@ -70,9 +70,6 @@ class IFMapNode;
 // The propagation of the NodeList and EdgeDescriptorList happens in context
 // of the client task (e.g. bgp::Config) when the its requesst the ChangeList.
 //
-// The vertex list is used to make sure that we don't add duplicate entries to
-// the ChangeList.
-//
 // CONCURRENCY: Not thread-safe. The class assumes that the caller ensures
 // that only a single method can run at a time.
 //
@@ -126,7 +123,6 @@ class IFMapDependencyTracker {
     DBGraph *graph_;
     ChangeObserver observer_;
     NodeEventPolicy policy_;
-    std::set<std::string> vertex_list_;
     EdgeDescriptorList edge_list_;
     NodeList node_list_;
 };