Optimizations to reduce table walks from RoutePathReplicator

Don't walk table when joining/leaving if the table is empty. This is handy when tables are being created upon receiving configuration for routing-instances - the tables are always empty at this point. Change-Id: I050dda29ad1311915d0f62d3fe098d9c6e245b5f Partial-Bug: 1456284
Juniper · Jun 1, 2015 · 2e42cc2 · 2e42cc2
1 parent 7f52b5c
commit 2e42cc2
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 14 deletions.
diff --git a/src/bgp/routing-instance/routepath_replicator.cc b/src/bgp/routing-instance/routepath_replicator.cc
@@ -195,6 +195,15 @@ void RoutePathReplicator::DeleteTableState(BgpTable *table) {
     delete ts;
 }
 
+void RoutePathReplicator::UnregisterTableState(BgpTable *table) {
+    CHECK_CONCURRENCY("bgp::Config", "db::DBTable");
+    const TableState *ts = FindTableState(table);
+    if (!ts->empty())
+        return;
+    unreg_table_list_.insert(table);
+    unreg_trigger_->Set();
+}
+
 TableState *RoutePathReplicator::FindTableState(BgpTable *table) {
     TableStateList::iterator loc = table_state_list_.find(table);
     return (loc != table_state_list_.end() ? loc->second : NULL);
@@ -280,10 +289,7 @@ RoutePathReplicator::BulkReplicationDone(DBTableBase *dbtable) {
     }
     delete bulk_sync_state;
     bulk_sync_.erase(loc);
-    const TableState *ts = FindTableState(table);
-    if (ts->empty())
-        unreg_table_list_.insert(table);
-    unreg_trigger_->Set();
+    UnregisterTableState(table);
 }
 
 void RoutePathReplicator::JoinVpnTable(RtGroup *group) {
@@ -326,16 +332,18 @@ void RoutePathReplicator::Join(BgpTable *table, const RouteTarget &rt,
         first = group->AddImportTable(family(), table);
         server()->rtarget_group_mgr()->NotifyRtGroup(rt);
         BOOST_FOREACH(BgpTable *sec_table, group->GetExportTables(family())) {
-            if (sec_table->IsVpnTable())
+            if (sec_table->IsVpnTable() || sec_table->empty())
                 continue;
             RequestWalk(sec_table);
         }
         walk_trigger_->Set();
     } else {
         first = group->AddExportTable(family(), table);
         AddTableState(table, group);
-        RequestWalk(table);
-        walk_trigger_->Set();
+        if (!table->empty()) {
+            RequestWalk(table);
+            walk_trigger_->Set();
+        }
     }
 
     // Join the vpn table when group is created.
@@ -360,16 +368,20 @@ void RoutePathReplicator::Leave(BgpTable *table, const RouteTarget &rt,
         group->RemoveImportTable(family(), table);
         server()->rtarget_group_mgr()->NotifyRtGroup(rt);
         BOOST_FOREACH(BgpTable *sec_table, group->GetExportTables(family())) {
-            if (sec_table->IsVpnTable())
+            if (sec_table->IsVpnTable() || sec_table->empty())
                 continue;
             RequestWalk(sec_table);
         }
         walk_trigger_->Set();
     } else {
         group->RemoveExportTable(family(), table);
         RemoveTableState(table, group);
-        RequestWalk(table);
-        walk_trigger_->Set();
+        if (!table->empty()) {
+            RequestWalk(table);
+            walk_trigger_->Set();
+        } else {
+            UnregisterTableState(table);
+        }
     }
 
     // Leave the vpn table when the last VRF has left the group.

diff --git a/src/bgp/routing-instance/routepath_replicator.h b/src/bgp/routing-instance/routepath_replicator.h
@@ -272,6 +272,7 @@ class RoutePathReplicator {
     TableState *AddTableState(BgpTable *table, RtGroup *group = NULL);
     void RemoveTableState(BgpTable *table, RtGroup *group);
     void DeleteTableState(BgpTable *table);
+    void UnregisterTableState(BgpTable *table);
     TableState *FindTableState(BgpTable *table);
     const TableState *FindTableState(BgpTable *table) const;
 

diff --git a/src/bgp/test/bgp_xmpp_inetvpn_test.cc b/src/bgp/test/bgp_xmpp_inetvpn_test.cc
@@ -706,8 +706,8 @@ TEST_F(BgpXmppInetvpn2ControlNodeTest, MultipleRouteAddDelete3) {
     // Verify table walk count for blue.inet.0.
     BgpTable *blue_x = VerifyTableExists(bs_x_, "blue.inet.0");
     BgpTable *blue_y = VerifyTableExists(bs_y_, "blue.inet.0");
-    TASK_UTIL_EXPECT_EQ(1, blue_x->walk_complete_count());
-    TASK_UTIL_EXPECT_EQ(1, blue_y->walk_complete_count());
+    TASK_UTIL_EXPECT_EQ(0, blue_x->walk_complete_count());
+    TASK_UTIL_EXPECT_EQ(0, blue_y->walk_complete_count());
     task_util::WaitForIdle();
 
     // Register to blue instance
@@ -716,8 +716,8 @@ TEST_F(BgpXmppInetvpn2ControlNodeTest, MultipleRouteAddDelete3) {
     task_util::WaitForIdle();
 
     // Verify that subscribe completed.
-    TASK_UTIL_EXPECT_EQ(2, blue_x->walk_complete_count());
-    TASK_UTIL_EXPECT_EQ(2, blue_y->walk_complete_count());
+    TASK_UTIL_EXPECT_EQ(1, blue_x->walk_complete_count());
+    TASK_UTIL_EXPECT_EQ(1, blue_y->walk_complete_count());
     task_util::WaitForIdle();
 
     // Pause update generation on X.

diff --git a/src/db/db_table.cc b/src/db/db_table.cc
@@ -290,6 +290,11 @@ DBEntry *DBTable::Find(const DBRequestKey *key) {
     return tbl_partition->Find(key);
 }
 
+//
+// Concurrency: called from task that's mutually exclusive with db::DBTable.
+//
+// Calculate the size across all partitions.
+//
 size_t DBTable::Size() const {
     size_t total = 0;
     for (vector<DBTablePartition *>::const_iterator iter = partitions_.begin();

diff --git a/src/db/db_table.h b/src/db/db_table.h
@@ -87,7 +87,9 @@ class DBTableBase {
     void AddToDBStateCount(ListenerId listener, int count);
 
     // Calculate the size across all partitions.
+    // Must be called from Task which is mutually exclusive with db::DBTable.
     virtual size_t Size() const { return 0; }
+    bool empty() const { return (Size() == 0); }
 
     // Suspended deletion resume hook for user function
     virtual void RetryDelete() { }