[fdborch] Batch-drain FDB notifications in FdbOrch::doTask#4604
Closed
manish1-arista wants to merge 1 commit into
Closed
[fdborch] Batch-drain FDB notifications in FdbOrch::doTask#4604manish1-arista wants to merge 1 commit into
manish1-arista wants to merge 1 commit into
Conversation
Replace the single consumer.pop() with consumer.pops() so each main-loop iteration drains a batch of pending FDB notifications instead of just one, preventing the queue from growing unboundedly under FDB event storms. Signed-off-by: manish1 <manish1@arista.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
Our Batch Drain FDB Notification consumer queue approach targeted fdborch only, drain the batch of pending entries on the notification channel per main-loop iteration instead of just one. The Other PRs (sonic-net/sonic-swss-common#1191, #4586 and sonic-net/sonic-sairedis#1899) fixes this memory growth for other orch consumer queue also by addressing the problem at the infrastructure layer rather than a single orch, by adding allowlist for each orchagent and Dedup queues for multiple orchangents. Closing this PR as other PRs had merged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
In
FdbOrch::doTask(NotificationConsumer&), replace the singleconsumer.pop()withconsumer.pops()to drain a batch of pending entries on the notification channel per main-loop iteration instead of just one. The per-entry processing logic is factored unchanged into a new handleNotification() helper that doTask() dispatches each popped entry to.No behavioral change for any individual notification, only the rate at which the queue is drained.
Why I did it
Under FDB event storms (large MAC moves etc), the producer side pushes notifications into the queue faster than the previous one-at-a-time drain could consume them. The backlog grew unboundedly, which on our deployments showed up as:
NotificationConsumer::pops() already exists exactly for this pattern and is what some other orch agents (e.g. portsorch) use.
How I verified it
Built sonic-swss against the change cleanly.
The Queue length remains in check under mac storm.
Details if related