[swss/syncd] race between orchagent removes RIF rate counters from DB and lua script fetching them #11621
Assignees
Labels
Comments
|
@sumanbrcm Could you please handle this issue? |
|
@stepanblyschak Sure, I will check and work on the fix accordingly . As per the previous discussion in PR (sonic-net/sonic-swss#2199) , the proposal was to move cleanup code to syncd . As per your suggestion >>the cleanup of COUNTERS RIF tables should be done in syncd while cleanup of the mapping should still be done in orchagent << |
|
@sumanbrcm is actively working on this issue. |
|
@sumanbrcm could you please provide ETA when the fix can be in 202205? |
|
@liat-grozovik ETA for this fix is 10th Oct. |
This was referenced Oct 13, 2022
liat-grozovik
pushed a commit
to sonic-net/sonic-swss
that referenced
this issue
Nov 8, 2022
The cleanup code for stale rif counters are now moved to syncd . Earlier as part of fix for issue #2193 the cleanup for stale rif counters was added. But it could create a race condition between orchagent removes RIF rate counters from DB and lua script fetching them. So as a fix all such cleanup has been moved to syncd. - What I did Fix for sonic-net/sonic-buildimage#11621 As a past fix which aimed at removing stale rif counters (#2199) , there is a chance of race condition and it leads to lua script reporting error. To handle this , the rif counters cleanup code(handled in cleanUpRifFromCounterDb) is now called from syncd ( removeCounter ) to avoid such race condition. - Why I did it The operations in Orchagent and syncd is not synchronous, so while Orchagent deletes the rif counters from Counters Db, the syncd could still access it. In race conditions the lua script trying to fetch rif counters will have errors syslog for such access as it was already deleted by orchagent. The cleanup code is removed from orchagent is added in syncd - it will make sure no such race condition would get hit. - How I verified it Followed the steps in (sonic-net/sonic-buildimage#11621) : Create RIF in SONiC, wait till RIF rates are populated in COUNTERS DB Remove RIF Repeat the steps multiple times and check if any error syslog is seen (No error syslog is seen) Also checked cleanup for rif counters. After RIF creation derived info of oid for RIF from "COUNTERS_RIF_NAME_MAP" 127) "Vlan100" 128) "oid:0x6000000000aa5" Checked all the tabled in COUNTER_DB which has same OID in keys 127.0.0.1:6379[2]> keys 6000000000aa5 1) "RATES:oid:0x6000000000aa5:RIF" 2) "COUNTERS:oid:0x6000000000aa5" 3) "RATES:oid:0x6000000000aa5" 127.0.0.1:6379[2]> Deleted the RIF by removing the ip on the intf. Checked COUNTER_DB again with same OID if there are stale entries or not. No stale entries exist now. 127.0.0.1:6379[2]> keys 6000000000aa5 (empty array) 127.0.0.1:6379[2]> Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>
yxieca
pushed a commit
to sonic-net/sonic-swss
that referenced
this issue
Nov 10, 2022
The cleanup code for stale rif counters are now moved to syncd . Earlier as part of fix for issue #2193 the cleanup for stale rif counters was added. But it could create a race condition between orchagent removes RIF rate counters from DB and lua script fetching them. So as a fix all such cleanup has been moved to syncd. - What I did Fix for sonic-net/sonic-buildimage#11621 As a past fix which aimed at removing stale rif counters (#2199) , there is a chance of race condition and it leads to lua script reporting error. To handle this , the rif counters cleanup code(handled in cleanUpRifFromCounterDb) is now called from syncd ( removeCounter ) to avoid such race condition. - Why I did it The operations in Orchagent and syncd is not synchronous, so while Orchagent deletes the rif counters from Counters Db, the syncd could still access it. In race conditions the lua script trying to fetch rif counters will have errors syslog for such access as it was already deleted by orchagent. The cleanup code is removed from orchagent is added in syncd - it will make sure no such race condition would get hit. - How I verified it Followed the steps in (sonic-net/sonic-buildimage#11621) : Create RIF in SONiC, wait till RIF rates are populated in COUNTERS DB Remove RIF Repeat the steps multiple times and check if any error syslog is seen (No error syslog is seen) Also checked cleanup for rif counters. After RIF creation derived info of oid for RIF from "COUNTERS_RIF_NAME_MAP" 127) "Vlan100" 128) "oid:0x6000000000aa5" Checked all the tabled in COUNTER_DB which has same OID in keys 127.0.0.1:6379[2]> keys 6000000000aa5 1) "RATES:oid:0x6000000000aa5:RIF" 2) "COUNTERS:oid:0x6000000000aa5" 3) "RATES:oid:0x6000000000aa5" 127.0.0.1:6379[2]> Deleted the RIF by removing the ip on the intf. Checked COUNTER_DB again with same OID if there are stale entries or not. No stale entries exist now. 127.0.0.1:6379[2]> keys 6000000000aa5 (empty array) 127.0.0.1:6379[2]> Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>
|
Fix merged |
yxieca
pushed a commit
to sonic-net/sonic-sairedis
that referenced
this issue
Feb 9, 2023
Changes for 202205 branch Fixing issue #11621 All the details of the fix in master branch is in the PR : - sairedis: Fixing race condition for rif counters #1136 swss: Fixing race condition for rif counters sonic-swss#2488 This change is already merged to master branch , but sonic-sairedis change (sairedis: Fixing race condition for rif counters #1136) could not be cherry picked to 202205 branch as the base file FlexCounter.cpp differs in both the branches. Also, the API removeDataFromCountersDB is not available in 202205 branch for the cleanup. Hence the current fix takes care of cleaning up the stale rif counters issue through newly added API cleanUpRifFromCounterDb . Unit Tests:- The steps followed are same as in sonic-net/sonic-buildimage#11621 Create RIF in SONiC, wait till RIF rates are populated in COUNTERS DB Remove RIF Repeat the steps multiple times and check if any error syslog is seen (No error syslog is seen) Also checked cleanup for rif counters. After RIF creation derived info of oid for RIF from "COUNTERS_RIF_NAME_MAP" 127) "Vlan100" 128) "oid:0x6000000000aa5" Checked all the tabled in COUNTER_DB which has same OID in keys 127.0.0.1:6379[2]> keys 6000000000aa5 "RATES:oid:0x6000000000aa5:RIF" "COUNTERS:oid:0x6000000000aa5" "RATES:oid:0x6000000000aa5" 127.0.0.1:6379[2]> Deleted the RIF by removing the ip on the intf. Checked COUNTER_DB again with same OID if there are stale entries or not. No stale entries exist now. 127.0.0.1:6379[2]> keys 6000000000aa5 (empty array) 127.0.0.1:6379[2]> Signed-off-by: Suman Kumar suman.kumar@broadcom.com Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
Errors in the log comming from RIF rate lua script due to orchagent removes RIF rates from COUNTERS DB.
Steps to reproduce the issue:
Describe the results you received:
Errors in the logs:
Describe the results you expected:
No errors
Output of
show version:Output of
show techsupport:Additional information you deem important (e.g. issue happens only occasionally):
The text was updated successfully, but these errors were encountered: