Add a per client config tracker in the exporter

Currently, we have a config-tracker inside IFMapClient. Moving this to the
exporter in a vector indexed by client-index. If the number of clients exceeds
the initial estimate, the vector is resized.

We create the config-set when the client is created and delete it when the
client is being destroyed. Before we destroy the client, the exporter will
clean the client's index from all the 'states' that the client has touched.

This is required for cases where a link-delete removes the link from the graph
and creates 2 partitions, one reachable via the client's VR and the other not
reachable. The link-delete has yet to be seen by the exporter and a
client-delete is processed. In this case, client cleanup while deleting the
client will not be able to reach the second partition. The graph-walker will
not be able to clean up the client's bit in the second partition either when it
finally sees the link-delete since the client would be destroyed by the time he
runs and the graph-walker needs the clients name to be able to do any walk for
him.

This config-tracker can also be used in LinkDeleteWalkBatchEnd() in the
graph-walker. Today we walk the entire graph to cleanup the client's bit. But,
with the per-client-config-tracker, we only need to walk each node in the
client's set. In scaled scenarios this can be huge time saver.

Closes-Bug: #1472807

Change-Id: I2b3c5c0b4cbad6df81faa4b0ee17d835b2b31faa

src/ifmap/ifmap_client.cc

@@ -32,45 +32,3 @@ std::vector<std::string> IFMapClient::vm_list() const {
     return vm_list;
 }
 
-void IFMapClient::ConfigTrackerAdd(IFMapState *state) {
-    config_tracker_.insert(state);
-}
-
-void IFMapClient::ConfigTrackerDelete(IFMapState *state) {
-    CtSz_t num = config_tracker_.erase(state);
-    assert(num == 1);
-}
-
-bool IFMapClient::ConfigTrackerHasState(IFMapState *state) {
-    ConfigTracker::iterator iter = config_tracker_.find(state);
-    return (iter == config_tracker_.end() ? false : true);
-}
-
-bool IFMapClient::ConfigTrackerEmpty() {
-    return config_tracker_.empty();
-}
-
-size_t IFMapClient::ConfigTrackerSize() {
-    return config_tracker_.size();
-}
-
-void IFMapClient::ConfigDbCleanup() {
-    BitSet rm_bs;
-    rm_bs.set(index_);
-
-    for (ConfigTracker::iterator iter = config_tracker_.begin();
-            iter != config_tracker_.end(); ++iter) {
-        IFMapState *state = *iter;
-        state->InterestReset(rm_bs);
-        state->AdvertisedReset(rm_bs);
-    }
-    config_tracker_.clear();
-}
-
-void IFMapClient::ResetLinkDeleteClients() {
-    BitSet rm_bs;
-    rm_bs.set(index_);
-
-    exporter_->ResetLinkDeleteClients(rm_bs);
-}
-

src/ifmap/ifmap_client.h

@@ -18,8 +18,6 @@ class IFMapState;
 class IFMapClient {
 public:
     typedef std::map<std::string, std::string> VmMap;
-    typedef boost::unordered_set<IFMapState *> ConfigTracker;
-    typedef ConfigTracker::size_type CtSz_t;
     static const int kIndexInvalid = -1;
 
     IFMapClient();
@@ -68,14 +66,6 @@ class IFMapClient {
     // return vm_map_ as a list of strings
     std::vector<std::string> vm_list() const;
 
-    void ConfigTrackerAdd(IFMapState *state);
-    void ConfigTrackerDelete(IFMapState *state);
-    bool ConfigTrackerHasState(IFMapState *state);
-    bool ConfigTrackerEmpty();
-    size_t ConfigTrackerSize();
-    void ConfigDbCleanup();
-    void ResetLinkDeleteClients();
-
 private:
     int index_;
     IFMapExporter *exporter_;
@@ -87,7 +77,6 @@ class IFMapClient {
     bool send_is_blocked_;
     VmMap vm_map_;
     std::string name_;
-    ConfigTracker config_tracker_;
 };
 
 #endif

src/ifmap/ifmap_exporter.cc

@@ -67,6 +67,14 @@ void IFMapExporter::Initialize(DB *db) {
 }
 
 void IFMapExporter::Shutdown() {
+    for (size_t index = 0; index < client_config_tracker_.size(); ++index) {
+        ConfigSet *set = client_config_tracker_[index];
+        if (set) {
+            set->clear();
+            delete set;
+            client_config_tracker_[index] = NULL;
+        }
+    }
     for (TableMap::iterator iter = table_map_.begin(); iter != table_map_.end();
          ++iter) {
         DBTable *table = iter->first;
@@ -607,21 +615,73 @@ bool IFMapExporter::ConfigChanged(IFMapNode *node) {
     return changed;
 }
 
+void IFMapExporter::AddClientConfigTracker(int index) {
+    if (index >= (int)client_config_tracker_.size()) {
+        client_config_tracker_.resize(index + 1, NULL);
+    }
+    assert(client_config_tracker_[index] == NULL);
+    ConfigSet *set = new ConfigSet();
+    client_config_tracker_[index] = set;
+}
+
+void IFMapExporter::DeleteClientConfigTracker(int index) {
+    ConfigSet *set = client_config_tracker_.at(index);
+    assert(set);
+    delete set;
+    client_config_tracker_[index] = NULL;
+}
+
 void IFMapExporter::UpdateClientConfigTracker(IFMapState *state,
         const BitSet& client_bits, bool add) {
-    IFMapClient *client = NULL;
     for (size_t pos = client_bits.find_first(); pos != BitSet::npos;
             pos = client_bits.find_next(pos)) {
-        client = server_->GetClient(pos);
-        assert(client);
+        ConfigSet *set = client_config_tracker_.at(pos);
+        assert(set);
         if (add) {
-            client->ConfigTrackerAdd(state);
+            set->insert(state);
         } else {
-            client->ConfigTrackerDelete(state);
+            CsSz_t num = set->erase(state);
+            assert(num == 1);
         }
     }
 }
 
+void IFMapExporter::CleanupClientConfigTrackedEntries(int index) {
+    ConfigSet *set = client_config_tracker_.at(index);
+    assert(set);
+    BitSet rm_bs;
+    rm_bs.set(index);
+    for (ConfigSet::iterator iter = set->begin(); iter != set->end(); ++iter) {
+        IFMapState *state = *iter;
+        state->InterestReset(rm_bs);
+        state->AdvertisedReset(rm_bs);
+    }
+}
+
+bool IFMapExporter::ClientHasConfigTracker(int index) {
+    ConfigSet *set = client_config_tracker_.at(index);
+    return ((set != NULL) ? true : false);
+}
+
+bool IFMapExporter::ClientConfigTrackerHasState(int index, IFMapState *state) {
+    ConfigSet *set = client_config_tracker_.at(index);
+    assert(set);
+    ConfigSet::iterator iter = set->find(state);
+    return (iter == set->end() ? false : true);
+}
+
+bool IFMapExporter::ClientConfigTrackerEmpty(int index) {
+    ConfigSet *set = client_config_tracker_.at(index);
+    assert(set);
+    return set->empty();
+}
+
+size_t IFMapExporter::ClientConfigTrackerSize(int index) {
+    ConfigSet *set = client_config_tracker_.at(index);
+    assert(set);
+    return set->size();
+}
+
 void IFMapExporter::StateInterestSet(IFMapState *state,
                                      const BitSet& interest_bits) {
     // Add the node to the config-tracker of all clients that just became

src/ifmap/ifmap_exporter.h

@@ -11,6 +11,7 @@
 #include <string>
 #include <boost/crc.hpp>      // for boost::crc_32_type
 #include <boost/scoped_ptr.hpp>
+#include <boost/unordered_set.hpp>
 
 #include "db/db_table.h"
 
@@ -39,6 +40,9 @@ struct IFMapTypenameWhiteList;
 // must come after the links that refer to the node have been deleted.
 class IFMapExporter {
 public:
+    typedef boost::unordered_set<IFMapState *> ConfigSet;
+    typedef ConfigSet::size_type CsSz_t;
+    typedef std::vector<ConfigSet *> ClientConfigTracker;
     typedef boost::crc_32_type::value_type crc32type;
     explicit IFMapExporter(IFMapServer *server);
     ~IFMapExporter();
@@ -61,8 +65,16 @@ class IFMapExporter {
 
     bool FilterNeighbor(IFMapNode *lnode, IFMapNode *rnode);
 
+    void AddClientConfigTracker(int index);
+    void DeleteClientConfigTracker(int index);
     void UpdateClientConfigTracker(IFMapState *state, const BitSet& client_bits,
                                    bool add);
+    void CleanupClientConfigTrackedEntries(int index);
+    bool ClientHasConfigTracker(int index);
+    bool ClientConfigTrackerHasState(int index, IFMapState *state);
+    bool ClientConfigTrackerEmpty(int index);
+    size_t ClientConfigTrackerSize(int index);
+
     void StateInterestSet(IFMapState *state, const BitSet& interest_bits);
     void StateInterestOr(IFMapState *state, const BitSet& interest_bits);
     void StateInterestReset(IFMapState *state, const BitSet& interest_bits);
@@ -113,6 +125,7 @@ class IFMapExporter {
     TableMap table_map_;
 
     DBTable *link_table_;
+    ClientConfigTracker client_config_tracker_;
 };
 
 #endif

src/ifmap/ifmap_server.cc

@@ -238,9 +238,9 @@ void IFMapServer::ClientUnregister(IFMapClient *client) {
 bool IFMapServer::ProcessClientWork(bool add, IFMapClient *client) {
     if (add) {
         ClientRegister(client);
+        ClientExporterSetup(client);
         ClientGraphDownload(client);
     } else {
-        ClientConfigDbCleanup(client);
         RemoveSelfAddedLinksAndObjects(client);
         CleanupUuidMapper(client);
         ClientExporterCleanup(client);
@@ -334,12 +334,17 @@ void IFMapServer::ClientGraphDownload(IFMapClient *client) {
     }
 }
 
-void IFMapServer::ClientConfigDbCleanup(IFMapClient *client) {
-    client->ConfigDbCleanup();
+void IFMapServer::ClientExporterSetup(IFMapClient *client) {
+    exporter_->AddClientConfigTracker(client->index());
 }
 
 void IFMapServer::ClientExporterCleanup(IFMapClient *client) {
-    client->ResetLinkDeleteClients();
+    exporter_->CleanupClientConfigTrackedEntries(client->index());
+    exporter_->DeleteClientConfigTracker(client->index());
+
+    BitSet rm_bs;
+    rm_bs.set(client->index());
+    exporter_->ResetLinkDeleteClients(rm_bs);
 }
 
 IFMapClient *IFMapServer::FindClient(const std::string &id) {

src/ifmap/ifmap_server.h

@@ -105,6 +105,7 @@ class IFMapServer {
     friend class IFMapRestartTest;
     friend class ShowIFMapXmppClientInfo;
     friend class XmppIfmapTest;
+    friend class IFMapExporterTest;
 
     enum QueueOp {
         ADD = 1,
@@ -116,9 +117,9 @@ class IFMapServer {
     };
     bool ClientWorker(QueueEntry work_entry);
     void ClientGraphDownload(IFMapClient *client);
-    void ClientConfigDbCleanup(IFMapClient *client);
     void RemoveSelfAddedLinksAndObjects(IFMapClient *client);
     void CleanupUuidMapper(IFMapClient *client);
+    void ClientExporterSetup(IFMapClient *client);
     void ClientExporterCleanup(IFMapClient *client);
     bool StaleNodesProcTimeout();
     const ClientMap &GetClientMap() const { return client_map_; }

src/ifmap/ifmap_server_show.cc

@@ -878,15 +878,15 @@ bool ShowIFMapPerClientNodes::CopyNode(IFMapPerClientNodesShowInfo *dest,
         } else {
             dest->sent = "No";
         }
-        IFMapClient *client = server->GetClient(client_index);
-        if (client) {
-            if (client->ConfigTrackerHasState(state)) {
+        if (server->exporter()->ClientHasConfigTracker(client_index)) {
+            if (server->exporter()->ClientConfigTrackerHasState(client_index,
+                                                                state)) {
                 dest->tracked = "Yes";
             } else {
                 dest->tracked = "No";
             }
         } else {
-            dest->tracked = "Client unknown";
+            dest->tracked = "No tracker";
         }
         return true;
     } else {
@@ -1070,15 +1070,15 @@ bool ShowIFMapPerClientLinkTable::CopyNode(IFMapPerClientLinksShowInfo *dest,
         } else {
             dest->sent = "No";
         }
-        IFMapClient *client = server->GetClient(client_index);
-        if (client) {
-            if (client->ConfigTrackerHasState(state)) {
+        if (server->exporter()->ClientHasConfigTracker(client_index)) {
+            if (server->exporter()->ClientConfigTrackerHasState(client_index,
+                                                                state)) {
                 dest->tracked = "Yes";
             } else {
                 dest->tracked = "No";
             }
         } else {
-            dest->tracked = "Client unknown";
+            dest->tracked = "No tracker";
         }
         return true;
     } else {