[syncd/FlexCounter]:Fix last remove bug (#679)

Pterosaur · web-flow · commit 4d21a264d595 · 2020-10-24T12:42:08.000-07:00
- The bug description
When the last object in FlexCounter was removed, the FlexCounter is also removed from FlexCounterManager. At the next time, when a new observed object is added and a new FlexCounter will be created. But the new FlexCounter cannot get the configuration. Because the configuration message from FlexCounterDB has been consumed at the first time.

- How to fix it
Add a flag to indicate whether the configuration also be removed from the FlexCounterDB. If not, don't delete the FlexCounter from FlexCounterManager even no observed objects in the FlexCounter.

Signed-off-by: Ze Gan &lt;ganze718@gmail.com&gt;
diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp
@@ -26,6 +26,7 @@ FlexCounter::FlexCounter(
     SWSS_LOG_ENTER();
 
     m_enable = false;
+    m_isDiscarded = false;
 
     startFlexCounterThread();
 }
@@ -819,6 +820,8 @@ void FlexCounter::removeCounterPlugins()
     m_rifPlugins.clear();
     m_priorityGroupPlugins.clear();
     m_bufferPoolPlugins.clear();
+
+    m_isDiscarded = true;
 }
 
 void FlexCounter::addCounterPlugin(
@@ -828,6 +831,8 @@ void FlexCounter::addCounterPlugin(
 
     SWSS_LOG_ENTER();
 
+    m_isDiscarded = false;
+
     for (auto& fvt: values)
     {
         auto& field = fvField(fvt);
@@ -901,6 +906,11 @@ bool FlexCounter::isEmpty()
     return allIdsEmpty() && allPluginsEmpty();
 }
 
+bool FlexCounter::isDiscarded()
+{
+    return isEmpty() && m_isDiscarded;
+}
+
 bool FlexCounter::allIdsEmpty() const
 {
     SWSS_LOG_ENTER();
diff --git a/syncd/FlexCounter.h b/syncd/FlexCounter.h
@@ -48,6 +48,8 @@ namespace syncd
 
             bool isEmpty();
 
+            bool isDiscarded();
+
         private:
 
             void setPollInterval(
@@ -405,5 +407,7 @@ namespace syncd
             std::shared_ptr<sairedis::SaiInterface> m_vendorSai;
 
             std::string m_dbCounters;
+
+            bool m_isDiscarded;
     };
 }
diff --git a/syncd/FlexCounterManager.cpp b/syncd/FlexCounterManager.cpp
@@ -62,7 +62,7 @@ void FlexCounterManager::removeCounterPlugins(
 
     fc->removeCounterPlugins();
 
-    if (fc->isEmpty())
+    if (fc->isDiscarded())
     {
         removeInstance(instanceId);
     }
@@ -91,7 +91,7 @@ void FlexCounterManager::addCounter(
 
     fc->addCounter(vid, rid, values);
 
-    if (fc->isEmpty())
+    if (fc->isDiscarded())
     {
         removeInstance(instanceId);
     }
@@ -107,7 +107,7 @@ void FlexCounterManager::removeCounter(
 
     fc->removeCounter(vid);
 
-    if (fc->isEmpty())
+    if (fc->isDiscarded())
     {
         removeInstance(instanceId);
     }

Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,7 @@ FlexCounter::FlexCounter(`
`26`	`26`	`SWSS_LOG_ENTER();`
`27`	`27`
`28`	`28`	`m_enable = false;`
	`29`	`+ m_isDiscarded = false;`
`29`	`30`
`30`	`31`	`startFlexCounterThread();`
`31`	`32`	`}`
`@@ -819,6 +820,8 @@ void FlexCounter::removeCounterPlugins()`
`819`	`820`	`m_rifPlugins.clear();`
`820`	`821`	`m_priorityGroupPlugins.clear();`
`821`	`822`	`m_bufferPoolPlugins.clear();`
	`823`	`+`
	`824`	`+ m_isDiscarded = true;`
`822`	`825`	`}`
`823`	`826`
`824`	`827`	`void FlexCounter::addCounterPlugin(`
`@@ -828,6 +831,8 @@ void FlexCounter::addCounterPlugin(`
`828`	`831`
`829`	`832`	`SWSS_LOG_ENTER();`
`830`	`833`
	`834`	`+ m_isDiscarded = false;`
	`835`	`+`
`831`	`836`	`for (auto& fvt: values)`
`832`	`837`	`{`
`833`	`838`	`auto& field = fvField(fvt);`
`@@ -901,6 +906,11 @@ bool FlexCounter::isEmpty()`
`901`	`906`	`return allIdsEmpty() && allPluginsEmpty();`
`902`	`907`	`}`
`903`	`908`
	`909`	`+bool FlexCounter::isDiscarded()`
	`910`	`+{`
	`911`	`+ return isEmpty() && m_isDiscarded;`
	`912`	`+}`
	`913`	`+`
`904`	`914`	`bool FlexCounter::allIdsEmpty() const`
`905`	`915`	`{`
`906`	`916`	`SWSS_LOG_ENTER();`
Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ void FlexCounterManager::removeCounterPlugins(`
`62`	`62`
`63`	`63`	`fc->removeCounterPlugins();`
`64`	`64`
`65`		`- if (fc->isEmpty())`
	`65`	`+ if (fc->isDiscarded())`
`66`	`66`	`{`
`67`	`67`	`removeInstance(instanceId);`
`68`	`68`	`}`
`@@ -91,7 +91,7 @@ void FlexCounterManager::addCounter(`
`91`	`91`
`92`	`92`	`fc->addCounter(vid, rid, values);`
`93`	`93`
`94`		`- if (fc->isEmpty())`
	`94`	`+ if (fc->isDiscarded())`
`95`	`95`	`{`
`96`	`96`	`removeInstance(instanceId);`
`97`	`97`	`}`
`@@ -107,7 +107,7 @@ void FlexCounterManager::removeCounter(`
`107`	`107`
`108`	`108`	`fc->removeCounter(vid);`
`109`	`109`
`110`		`- if (fc->isEmpty())`
	`110`	`+ if (fc->isDiscarded())`
`111`	`111`	`{`
`112`	`112`	`removeInstance(instanceId);`
`113`	`113`	`}`