-
Notifications
You must be signed in to change notification settings - Fork 270
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
[gbsyncd] Add asic db prefix for channel NOTIFICATIONS #1129
Conversation
lib/sairediscommon.h
Outdated
/** | ||
* @brief Table which will be used to forward notifications from gbsyncd. | ||
*/ | ||
#define REDIS_TABLE_GBNOTIFICATIONS "GBNOTIFICATIONS" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why GB needs different communication channel? channels are based on databases, for example ASIC_DB, so if GB will have different database then it will have separate communication channel for notifications and they will not colide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also this could be generalizaed by chaning notifications channel name by prefixing it by db name
for example dbAsic + REDIS_TABLE_NOTIFICATIONS, then it would be more generic, and i think this would be proffered way to make this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The redis channel is global per instance. Do you prefer to the below?
#define REDIS_GB_ASIC_NOTIFICATIONS "GB_ASIC_NOTIFICATIONS"
"NOTIFICATIONS" has been widely referred as asic channel. I'll not rename it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think more preffered way would be to use that asic db prefix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add REDIS_TABLE_NOTIFICATIONS_PER_DB(dbName)
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Fix #1131. The communication channel between orchagent and gbsyncd needs different from the channel "NOTIFICATIONS" between orchagent and syncd.
2022-10-21 b7c85ca: [gbsyncd] Add asic db prefix for channel NOTIFICATIONS (sonic-net/sonic-sairedis#1129) (Junhua Zhai)
Fix sonic-net#1131. The communication channel between orchagent and gbsyncd needs different from the channel "NOTIFICATIONS" between orchagent and syncd.
Fix #1131. The communication channel between orchagent and gbsyncd needs different from the channel "NOTIFICATIONS" between orchagent and syncd.
Fix sonic-net#1131. The communication channel between orchagent and gbsyncd needs different from the channel "NOTIFICATIONS" between orchagent and syncd.
Fix sonic-net#1131. The communication channel between orchagent and gbsyncd needs different from the channel "NOTIFICATIONS" between orchagent and syncd.
Fix #1131.
The communication channel between orchagent and gbsyncd needs different from the channel "NOTIFICATIONS" between orchagent and syncd.