Skip to content

Comments

[syncd] Remove syncd redis objects if using ZMQ notifications#1694

Merged
saiarcot895 merged 37 commits intosonic-net:masterfrom
croos12:redis_disabled_if_ZMQ
Jan 15, 2026
Merged

[syncd] Remove syncd redis objects if using ZMQ notifications#1694
saiarcot895 merged 37 commits intosonic-net:masterfrom
croos12:redis_disabled_if_ZMQ

Conversation

@croos12
Copy link
Contributor

@croos12 croos12 commented Nov 13, 2025

Turns off creating entries in redis for syncd if ZMQ is enabled. This new behavior only occurs when ZMQ_sync is enabled, which is currently only used in the nvidia-bluefield platforms (DPUs in SmartSwitch Scenario).

This PR reduces size of ASIC_DB to prevent an issue where running show techsupport with large numbers of ENIs would be degraded and lead to syncd crashing. With enough ENIs (roughly 63) SONiC would run out of free memory altogether and crash. This also leads to performance improvements as the redis entries are redundant as the SaiRedis notifications are redundant as the notifications are already received over the ZMQ message queue in the syncd ZMQ_sync mode.

VS Switches will need to have the old behavior for unit testing otherwise swss tests will fail

croos12 and others added 7 commits October 27, 2025 20:29
Signed-off-by: Connor Roos <croos@nvidia.com>
Signed-off-by: Connor Roos <croos@nvidia.com>
Signed-off-by: Connor Roos <croos@nvidia.com>
Signed-off-by: Connor Roos <croos@nvidia.com>
Since we no longer use redis in zmq_sync mode there is no buffer to sync

Signed-off-by: Connor Roos <croos@nvidia.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@croos12 croos12 force-pushed the redis_disabled_if_ZMQ branch from d0df908 to f252482 Compare November 13, 2025 16:54
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@croos12 croos12 force-pushed the redis_disabled_if_ZMQ branch from 0ee1ea9 to 541ef03 Compare November 13, 2025 17:38
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Connor Roos <croos@nvidia.com>
@croos12 croos12 force-pushed the redis_disabled_if_ZMQ branch from be9ca92 to d945839 Compare November 13, 2025 18:37
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@croos12
Copy link
Contributor Author

croos12 commented Jan 14, 2026

@prabhataravind I addressed the Copilot comments

prabhataravind
prabhataravind previously approved these changes Jan 14, 2026
Copy link
Contributor

@prabhataravind prabhataravind left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Connor Roos <croos@nvidia.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@lolyu lolyu left a comment

Choose a reason for hiding this comment

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

LGTM

@lolyu
Copy link
Contributor

lolyu commented Jan 15, 2026

Hi @saiarcot895, could you please review again?

@prsunny
Copy link
Contributor

prsunny commented Jan 15, 2026

@saiarcot895 , please merge

@prsunny
Copy link
Contributor

prsunny commented Jan 15, 2026

@croos12 , thanks for the PR

@saiarcot895 saiarcot895 merged commit 914bda7 into sonic-net:master Jan 15, 2026
16 checks passed
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to msft-202506: Azure/sonic-sairedis.msft#100

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202511: #1738

croos12 added a commit to croos12/sonic-sairedis that referenced this pull request Jan 15, 2026
…net#1694)

* Remove syncd redis objects if using ZMQ notifications

Signed-off-by: Connor Roos <croos@nvidia.com>

* Remove trailing whitespace in RedisClient.cpp

Signed-off-by: Connor Roos <croos@nvidia.com>

* Ignore Syncd::applyView() if ZMQ if enabled

Signed-off-by: Connor Roos <croos@nvidia.com>

* Create empty applyview.log for unittest if ZMQ

Signed-off-by: Connor Roos <croos@nvidia.com>

* Remove zmq_sync buffer_pools tests

Since we no longer use redis in zmq_sync mode there is no buffer to sync

Signed-off-by: Connor Roos <croos@nvidia.com>

* Set Redis entries to be created in vs ASIC

Signed-off-by: Connor Roos <croos@nvidia.com>

* Set default value and flag for is virtual switch

Signed-off-by: Connor Roos <croos@nvidia.com>

* Undo flag changes

Signed-off-by: Connor Roos <croos@nvidia.com>

* Switch to checking VendorSAI type

Signed-off-by: Connor Roos <croos@nvidia.com>

* Link with SAILIB

Signed-off-by: Connor Roos <croos@nvidia.com>

* Try to use profile map instead of vendorSai type

Signed-off-by: Connor Roos <croos@nvidia.com>

* Set test for disabled RedisClient

Signed-off-by: Connor Roos <croos@nvidia.com>

* Revert zmq_sync buffer_pool test removal

Signed-off-by: Connor Roos <croos@nvidia.com>

* Add isRedisDisabled method

Makes redis disable more legible and adds methods to determine whether
or not to create error logs

Signed-off-by: Connor Roos <croos@nvidia.com>

* Use dependency injection to disable

Signed-off-by: Connor Roos <croos@nvidia.com>

* Included RedisClient Header file

Signed-off-by: Connor Roos <croos@nvidia.com>

* Add one more missing #include

Signed-off-by: Connor Roos <croos@nvidia.com>

* Remove whitespace

Signed-off-by: Connor Roos <croos@nvidia.com>

* Address copilot comments

Signed-off-by: Connor Roos <croos@nvidia.com>

* Add getVidsForRids change to DisabledRedisClient

Signed-off-by: Connor Roos <croos@nvidia.com>

* Fix isRedisEnabled calls

Signed-off-by: Connor Roos <croos@nvidia.com>

* Fix whitespace

Signed-off-by: Connor Roos <croos@nvidia.com>

* Address Copilot comments for test

Signed-off-by: Connor Roos <croos@nvidia.com>

---------

Signed-off-by: Connor Roos <croos@nvidia.com>
Co-authored-by: prabhataravind <108555774+prabhataravind@users.noreply.github.com>
Co-authored-by: Longxiang Lyu <35479537+lolyu@users.noreply.github.com>
croos12 added a commit to croos12/sonic-sairedis that referenced this pull request Jan 15, 2026
…net#1694)

* Remove syncd redis objects if using ZMQ notifications

Signed-off-by: Connor Roos <croos@nvidia.com>

* Remove trailing whitespace in RedisClient.cpp

Signed-off-by: Connor Roos <croos@nvidia.com>

* Ignore Syncd::applyView() if ZMQ if enabled

Signed-off-by: Connor Roos <croos@nvidia.com>

* Create empty applyview.log for unittest if ZMQ

Signed-off-by: Connor Roos <croos@nvidia.com>

* Remove zmq_sync buffer_pools tests

Since we no longer use redis in zmq_sync mode there is no buffer to sync

Signed-off-by: Connor Roos <croos@nvidia.com>

* Set Redis entries to be created in vs ASIC

Signed-off-by: Connor Roos <croos@nvidia.com>

* Set default value and flag for is virtual switch

Signed-off-by: Connor Roos <croos@nvidia.com>

* Undo flag changes

Signed-off-by: Connor Roos <croos@nvidia.com>

* Switch to checking VendorSAI type

Signed-off-by: Connor Roos <croos@nvidia.com>

* Link with SAILIB

Signed-off-by: Connor Roos <croos@nvidia.com>

* Try to use profile map instead of vendorSai type

Signed-off-by: Connor Roos <croos@nvidia.com>

* Set test for disabled RedisClient

Signed-off-by: Connor Roos <croos@nvidia.com>

* Revert zmq_sync buffer_pool test removal

Signed-off-by: Connor Roos <croos@nvidia.com>

* Add isRedisDisabled method

Makes redis disable more legible and adds methods to determine whether
or not to create error logs

Signed-off-by: Connor Roos <croos@nvidia.com>

* Use dependency injection to disable

Signed-off-by: Connor Roos <croos@nvidia.com>

* Included RedisClient Header file

Signed-off-by: Connor Roos <croos@nvidia.com>

* Add one more missing #include

Signed-off-by: Connor Roos <croos@nvidia.com>

* Remove whitespace

Signed-off-by: Connor Roos <croos@nvidia.com>

* Address copilot comments

Signed-off-by: Connor Roos <croos@nvidia.com>

* Add getVidsForRids change to DisabledRedisClient

Signed-off-by: Connor Roos <croos@nvidia.com>

* Fix isRedisEnabled calls

Signed-off-by: Connor Roos <croos@nvidia.com>

* Fix whitespace

Signed-off-by: Connor Roos <croos@nvidia.com>

* Address Copilot comments for test

Signed-off-by: Connor Roos <croos@nvidia.com>

---------

Signed-off-by: Connor Roos <croos@nvidia.com>
Co-authored-by: prabhataravind <108555774+prabhataravind@users.noreply.github.com>
Co-authored-by: Longxiang Lyu <35479537+lolyu@users.noreply.github.com>
@mssonicbld
Copy link
Collaborator

@croos12 cherry pick PR didn't pass PR checker. Please check!!!
#1738

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants