-
Notifications
You must be signed in to change notification settings - Fork 276
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 RESTARTQUERY #1302
Conversation
aspell test check failed, please fix |
Fixed |
* it on any level, including database numbers." | ||
*/ | ||
#define SYNCD_NOTIFICATION_CHANNEL_RESTARTQUERY_PER_DB(dbName) \ | ||
((dbName) == "ASIC_DB" ? \ |
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.
Instead of hardcoding asic_db here you could use it define it should be in redisconfig.h
@@ -9,6 +9,18 @@ | |||
|
|||
#define SYNCD_NOTIFICATION_CHANNEL_RESTARTQUERY "RESTARTQUERY" |
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 would also remove this since from this point we always want to have specified dB name and probably we can always join dB name to restart query, no need for special case
Unit test from multi ASIC line card. command : systemctl stop gbsyncd@0 |
Config reload log snippet: |
what about those logs ? syncd is not behaving in the expected way after this change ? @jimmyzhai seems like syncd is getting SIGTERM instead of gracefully exits, like the notification is not received byt syncd |
@jaganbal-a since you are having gbsyncd1 here you probably need to specify -g and -x parameters in syncd_request_shutdown command, please take a read of wiki https://github.com/sonic-net/sonic-sairedis/wiki/Context-configuration, and you will probably need to call shutdown twice for each global config 0 and 1 if you have 2 syncd processes running currently i don't know how shutdown scripts in syncd are prepared, but i think automatically supporting shutdown both syncd processes is not yet supported in scripts |
@kcudnik , I see the config reload/gbsyncd docker restart/shutdown works as expected. Nov 16 18:23:41.446085 sonic NOTICE root: Stopped swss0 service... |
Is that not expected behavior? |
@jimmyzhai could you update ADO in the descritption? |
@StormLiangMS : Updated |
* Use different channel RESTARTQUERY for syncd/gbsyncd * change restartquery to restartQuery due to aspell error
Cherry-pick PR to 202305: #1337 |
* Use different channel RESTARTQUERY for syncd/gbsyncd * change restartquery to restartQuery due to aspell error
@jimmyzhai can you help create 202311 cherry-pick PR? |
Change is already in 202311 at creation time. |
Fix the issue sonic-net/sonic-buildimage#16608, together with sonic-net/sonic-buildimage#16812.
Need separated channel for graeceful shutdown of syncd and gbsyncd.
ADO: 26284737