-
Notifications
You must be signed in to change notification settings - Fork 530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[QoS] Resolve an issue in the sequence where a referenced object removed and then the referencing object deleting and then re-adding #2210
Conversation
Don't quite understand the coverage report. Looks like many sentences covered by the mock test were eventually identified as uncovered. |
@theasianpianist , could you please help @stephenxs with the coverage info? |
Uncovered lines reported for
Coverage of line 164:
Coverage of L1260
|
@stephenxs I'm looking in to the coverage discrepancies, hope to have it resolved soon. Don't believe it's a problem on your end. |
Signed-off-by: Stephen Sun <stephens@nvidia.com>
…r to retry Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Cover the case where a referenced object is created when it is pending remove Signed-off-by: Stephen Sun <stephens@nvidia.com>
…nce 2018 Signed-off-by: Stephen Sun <stephens@nvidia.com>
a728183
to
8713aaa
Compare
Signed-off-by: Stephen Sun <stephens@nvidia.com>
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
Mock test has been added for buffer orch. Now all the changes have been covered |
@neethajohn could you please help to review? |
@theasianpianist, @neethajohn could you please help to signoff? |
LGTM. but code coverage still fails |
Signed-off-by: Stephen Sun <stephens@nvidia.com>
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
@theasianpianist |
List of the coverage of all the code changes (by gdb)
|
as discussed offline, coverage is ok for new code and the error is on different test. not relevant for these changes. |
…ved and then the referencing object deleting and then re-adding (#2210) - What I did Resolve an issue in the following scenario Suppose object a in table A references object b in table B. When a user is going to remove items in table A and B, the notifications can be received in the following order: The notification of removing object b Then the notification of removing object a And then the notification of re-adding object a The notification of re-adding object b. Object b can not be removed in the 1st step because it is still referenced by object a. In case the system is busy, the notification of removing a remains in m_toSync when the notification of re-adding it is coming, which results in both notifications being handled together and the reference to object b not being cleared. As a result, notification of removing b will never be handled and remain in m_toSync forever. Solution: Introduce a flag pending remove indicating whether an object is about to be removed but pending on some reference. pending remove is set once a DEL notification is received for an object with non-zero reference. When resolving references in step 3, a pending remove object will be skipped and the notification will remain in m_toSync. SET operation will not be carried out in case there is a pending remove flag on the object to be set. By doing so, when object a is re-added in step 3, it can not retrieve the dependent object b. And step 1 will be handled and drained successfully. - Why I did it Fix bug. - How I verified it Mock test and manually test (eg. config qos reload) Signed-off-by: Stephen Sun <stephens@nvidia.com>
…ved and then the referencing object deleting and then re-adding (sonic-net#2210) - What I did Resolve an issue in the following scenario Suppose object a in table A references object b in table B. When a user is going to remove items in table A and B, the notifications can be received in the following order: The notification of removing object b Then the notification of removing object a And then the notification of re-adding object a The notification of re-adding object b. Object b can not be removed in the 1st step because it is still referenced by object a. In case the system is busy, the notification of removing a remains in m_toSync when the notification of re-adding it is coming, which results in both notifications being handled together and the reference to object b not being cleared. As a result, notification of removing b will never be handled and remain in m_toSync forever. Solution: Introduce a flag pending remove indicating whether an object is about to be removed but pending on some reference. pending remove is set once a DEL notification is received for an object with non-zero reference. When resolving references in step 3, a pending remove object will be skipped and the notification will remain in m_toSync. SET operation will not be carried out in case there is a pending remove flag on the object to be set. By doing so, when object a is re-added in step 3, it can not retrieve the dependent object b. And step 1 will be handled and drained successfully. - Why I did it Fix bug. - How I verified it Mock test and manually test (eg. config qos reload) Signed-off-by: Stephen Sun <stephens@nvidia.com>
What I did
Resolve an issue in the following scenario
Suppose object
a
in tableA
references objectb
in tableB
. When a user is going to remove items in tableA
andB
, the notifications can be received in the following order:b
a
a
b
.Object
b
can not be removed in the 1st step because it is still referenced by objecta
. In case the system is busy, the notification of removinga
remains inm_toSync
when the notification of re-adding it is coming, which results in both notifications being handled together and the reference to objectb
not being cleared.As a result, notification of removing
b
will never be handled and remain inm_toSync
forever.Solution:
pending remove
indicating whether an object is about to be removed but pending on some reference.pending remove
is set once aDEL
notification is received for an object with non-zero reference.pending remove
object will be skipped and the notification will remain inm_toSync
.SET
operation will not be carried out in case there is apending remove
flag on the object to be set.By doing so, when object
a
is re-added in step 3, it can not retrieve the dependent objectb
. And step 1 will be handled and drained successfully.Why I did it
Fix bug.
How I verified it
Mock test and manually test (eg.
config qos reload
)Details if related
Orch::parseReference
skips a pending-remove object which it is resolving a reference.task_process_status::task_need_retry
when a pending-remove object is about to be created. This is to prevent step4
, which is re-adding the pending-remove object, from being executed.pendingRemove
tofalse
on creating an objectpendingRemove
totrue
when an object can not be removed because of non empty referencesincluding the following objects
QosOrch::ResolveMapAndApplyToPort
. I remove it in this PR because