Fix tables handling race condition in buffermgr (sonic-net#484)

* Fix tables handling race condition in buffermgr Signed-off-by: Andriy Moroz <[email protected]> * Fixed status lose in loop Signed-off-by: Andriy Moroz <[email protected]>
zhenggen-xu · Apr 24, 2018 · ea34b92 · ea34b92
1 parent 53831be
commit ea34b92
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 13 deletions.
diff --git a/cfgmgr/buffermgr.cpp b/cfgmgr/buffermgr.cpp
@@ -72,9 +72,10 @@ void BufferMgr::readPgProfileLookupFile(string file)
     infile.close();
 }
 
-void BufferMgr::doCableTask(string port, string cable_length)
+task_process_status BufferMgr::doCableTask(string port, string cable_length)
 {
     m_cableLenLookup[port] = cable_length;
+    return task_process_status::task_success;
 }
 
 string BufferMgr::getPgPoolMode()
@@ -108,15 +109,15 @@ Create/update two tables: profile (in m_cfgBufferProfileTable) and port buffer (
         }
     }
 */
-void BufferMgr::doSpeedUpdateTask(string port, string speed)
+task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed)
 {
     vector<FieldValueTuple> fvVector;
     string cable;
 
     if (m_cableLenLookup.count(port) == 0)
     {
-        SWSS_LOG_ERROR("Unable to create/update PG profile for port %s. Cable length is not set", port.c_str());
-        return;
+        SWSS_LOG_WARN("Unable to create/update PG profile for port %s. Cable length is not set", port.c_str());
+        return task_process_status::task_need_retry;
     }
 
     cable = m_cableLenLookup[port];
@@ -125,7 +126,7 @@ void BufferMgr::doSpeedUpdateTask(string port, string speed)
     {
         SWSS_LOG_ERROR("Unable to create/update PG profile for port %s. No PG profile configured for speed %s and cable length %s",
                        port.c_str(), speed.c_str(), cable.c_str());
-        return;
+        return task_process_status::task_failed;
     }
 
     // Crete record in BUFFER_PROFILE table
@@ -143,8 +144,8 @@ void BufferMgr::doSpeedUpdateTask(string port, string speed)
         if (mode.empty())
         {
             // this should never happen if switch initialized properly
-            SWSS_LOG_ERROR("PG lossless pool is not yet created");
-            return;
+            SWSS_LOG_WARN("PG lossless pool is not yet created");
+            return task_process_status::task_need_retry;
         }
 
         // profile threshold field name
@@ -170,6 +171,7 @@ void BufferMgr::doSpeedUpdateTask(string port, string speed)
     string profile_ref = string("[") + CFG_BUFFER_PROFILE_TABLE_NAME + CONFIGDB_TABLE_NAME_SEPARATOR + buffer_profile_key + "]";
     fvVector.push_back(make_pair("profile", profile_ref));
     m_cfgBufferPgTable.set(buffer_pg_key, fvVector);
+    return task_process_status::task_success;
 }
 
 void BufferMgr::doTask(Consumer &consumer)
@@ -188,25 +190,41 @@ void BufferMgr::doTask(Consumer &consumer)
         string port(keys[0]);
 
         string op = kfvOp(t);
+        task_process_status task_status = task_process_status::task_success;
         if (op == SET_COMMAND)
         {
             for (auto i : kfvFieldsValues(t))
             {
                 if (table_name == CFG_PORT_CABLE_LEN_TABLE_NAME)
                 {
                     // receive and cache cable length table
-                    doCableTask(fvField(i), fvValue(i));
+                    task_status = doCableTask(fvField(i), fvValue(i));
                 }
                 // In case of PORT table update, Buffer Manager is interested in speed update only
                 if (table_name == CFG_PORT_TABLE_NAME && fvField(i) == "speed")
                 {
                     // create/update profile for port
-                    doSpeedUpdateTask(port, fvValue(i));
+                    task_status = doSpeedUpdateTask(port, fvValue(i));
+                }
+                if (task_status != task_process_status::task_success)
+                {
+                    break;
                 }
             }
         }
 
-        it = consumer.m_toSync.erase(it);
-        continue;
+        switch (task_status)
+        {
+            case task_process_status::task_failed:
+                SWSS_LOG_ERROR("Failed to process table update");
+                return;
+            case task_process_status::task_need_retry:
+                SWSS_LOG_INFO("Unable to process table update. Will retry...");
+                ++it;
+                break;
+            default:
+                it = consumer.m_toSync.erase(it);
+                break;
+        }
     }
 }
diff --git a/cfgmgr/buffermgr.h b/cfgmgr/buffermgr.h
@@ -44,8 +44,8 @@ class BufferMgr : public Orch
     port_cable_length_t m_cableLenLookup;
     std::string getPgPoolMode();
     void readPgProfileLookupFile(std::string);
-    void doCableTask(string port, string cable_length);
-    void doSpeedUpdateTask(string port, string speed);
+    task_process_status doCableTask(string port, string cable_length);
+    task_process_status doSpeedUpdateTask(string port, string speed);
 
     void doTask(Consumer &consumer);
 };