Skip to content

[zmqorch]: Restore drain-until-empty consumer for non-DPU/fabric#4578

Merged
prsunny merged 3 commits into
sonic-net:masterfrom
venkit-nexthop:fix-zmqorch-route-perf-regression
May 27, 2026
Merged

[zmqorch]: Restore drain-until-empty consumer for non-DPU/fabric#4578
prsunny merged 3 commits into
sonic-net:masterfrom
venkit-nexthop:fix-zmqorch-route-perf-regression

Conversation

@venkit-nexthop

Copy link
Copy Markdown
Contributor

What I did

PR #3910 changed ZmqConsumer::execute() to perform a single batch pop per invocation. This change introduces a ZmqRouteConsumer subclass that restores the prior drain-until-empty loop in execute(). ZmqOrch::addConsumer() selects ZmqRouteConsumer for non-DPU / non-fabric switch types, while DPU and fabric continue to use ZmqConsumer.

Also defines gMySwitchType in orchagent/p4orch/tests/test_main.cpp so p4orch_tests links once zmqorch.cpp references the global. Production (main.cpp) and mock tests (mock_orchagent_main.cpp) already define it.

Why I did it

PR #3910's single-batch pop was primarily motivated by reducing peak memory in DASH (DPU) workloads. For non-DPU / non-fabric switches receiving high volumes of APPL_DB route updates from fpmsyncd, the change regressed throughput: each execute() call no longer drains the consumer table, so updates take many more polls to apply.

This is a targeted temporary fix that preserves the bounded-memory behavior added in #3910 for DPU/fabric, while restoring the earlier drain semantics for the workloads that depend on them.

How I verified it

  • Built the corresponding internal change end-to-end (incl. p4orch_tests linkage validated by CI).
  • Symbol resolution for gMySwitchType: production orchagent resolves via main.cpp, mock_tests via mock_orchagent_main.cpp, and p4orch_tests via the new definition in test_main.cpp.

Details if related

@venkit-nexthop
venkit-nexthop requested a review from prsunny as a code owner May 15, 2026 17:38
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@venkit-nexthop

Copy link
Copy Markdown
Contributor Author

Comment thread orchagent/zmqorch.cpp Outdated
Comment thread orchagent/zmqorch.cpp
@venkit-nexthop
venkit-nexthop force-pushed the fix-zmqorch-route-perf-regression branch from c18add4 to 847130e Compare May 15, 2026 19:28
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@venkit-nexthop
venkit-nexthop force-pushed the fix-zmqorch-route-perf-regression branch from 847130e to 5b7b4b3 Compare May 15, 2026 19:34
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@venkit-nexthop
venkit-nexthop force-pushed the fix-zmqorch-route-perf-regression branch from 5b7b4b3 to a842a56 Compare May 15, 2026 19:41
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Comment thread orchagent/zmqorch.cpp Outdated
@venkit-nexthop
venkit-nexthop force-pushed the fix-zmqorch-route-perf-regression branch from a842a56 to f8bf093 Compare May 15, 2026 20:32
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@vivekrnv

Copy link
Copy Markdown
Contributor

LGTM

@venkit-nexthop
venkit-nexthop force-pushed the fix-zmqorch-route-perf-regression branch from f8bf093 to 489cad7 Compare May 16, 2026 16:06
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

PR sonic-net#3910 changed ZmqConsumer::execute() to perform a single batch pop
per invocation, primarily to reduce peak memory in DASH (DPU) workloads.
For non-DPU / non-fabric switches that receive high volumes of APPL_DB
route updates from fpmsyncd, this regressed throughput: each execute()
call no longer drains the consumer table, so updates take many more
polls to apply.

Introduce a ZmqRouteConsumer subclass that restores the prior
drain-until-empty loop in execute(). ZmqOrch::addConsumer() selects
ZmqRouteConsumer for non-DPU / non-fabric switch types; DPU and fabric
continue to use ZmqConsumer to preserve the bounded-memory behavior
added in sonic-net#3910.

Also define gMySwitchType in orchagent/p4orch/tests/test_main.cpp so
p4orch_tests links once zmqorch.cpp references the global.

Signed-off-by: Venkit Kasiviswanathan <venkit@nexthop.ai>
@venkit-nexthop
venkit-nexthop force-pushed the fix-zmqorch-route-perf-regression branch from 30fc27f to d63ed1c Compare May 18, 2026 16:33
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Venkit Kasiviswanathan <venkit@nexthop.ai>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@prabhataravind

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@venkit-nexthop

Copy link
Copy Markdown
Contributor Author

@prabhataravind @qiluo-msft @prsunny @vivekrnv @deepak-singhal0408
could you approve this PR. Now all the checks have passed.

@venkit-nexthop

Copy link
Copy Markdown
Contributor Author

thanks @vivekrnv
@prsunny @prabhataravind can you help with merging this.

@prabhataravind prabhataravind left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@prsunny
prsunny merged commit f7ea472 into sonic-net:master May 27, 2026
19 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in SONiC Routing Dashboard May 27, 2026
@abdosi

abdosi commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

@venkit-nexthop : @deepak-singhal0408 : this is reversing the changes done here: #3269

Have we verified we are not regressing the previous behavior ?

@venkit-nexthop

Copy link
Copy Markdown
Contributor Author

@venkit-nexthop : @deepak-singhal0408 : this is reversing the changes done here: #3269

Have we verified we are not regressing the previous behavior ?

@abdosi sorry for the late response. I am catching up after my leave.
My change is only in ZmqOrch and the PR you pointed to appears to be for the regular orch (which handles redis).
The problem I am fixing has route download speed implications. But the most important thing is that memory grows unboundedly and causes OOM situation with this (due to lack of coalescing).
Please refer to the HLD I had written with proposal for a more complete fix here => sonic-net/SONiC#2328

Yogapriya-cisco pushed a commit to prhoskot/sonic-swss that referenced this pull request Jul 13, 2026
…ic-net#4578)

* [zmqorch]: Restore drain-until-empty consumer for non-DPU/fabric

PR sonic-net#3910 changed ZmqConsumer::execute() to perform a single batch pop
per invocation, primarily to reduce peak memory in DASH (DPU) workloads.
For non-DPU / non-fabric switches that receive high volumes of APPL_DB
route updates from fpmsyncd, this regressed throughput: each execute()
call no longer drains the consumer table, so updates take many more
polls to apply.

Introduce a ZmqRouteConsumer subclass that restores the prior
drain-until-empty loop in execute(). ZmqOrch::addConsumer() selects
ZmqRouteConsumer for non-DPU / non-fabric switch types; DPU and fabric
continue to use ZmqConsumer to preserve the bounded-memory behavior
added in sonic-net#3910.

Also define gMySwitchType in orchagent/p4orch/tests/test_main.cpp so
p4orch_tests links once zmqorch.cpp references the global.

Signed-off-by: Venkit Kasiviswanathan <venkit@nexthop.ai>
Signed-off-by: Yogapriya Mohankumar <ymohanku@cisco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants