-
Notifications
You must be signed in to change notification settings - Fork 684
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
Fix db_migrate.py show error and back trace while loading configuration on Linecard #3257
Fix db_migrate.py show error and back trace while loading configuration on Linecard #3257
Conversation
scripts/db_migrator.py
Outdated
@@ -1277,7 +1276,14 @@ def main(): | |||
socket_path = args.socket | |||
namespace = args.namespace | |||
|
|||
load_db_config() | |||
# Can't load global config base on the resoult of is_multi_asic(), because on multi-asic device, when db_migrate.py |
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.
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.
Fixed.
@@ -1277,7 +1276,14 @@ def main(): | |||
socket_path = args.socket | |||
namespace = args.namespace | |||
|
|||
load_db_config() |
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.
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.
This is not a general issue, according to the code of this script and Judy's comments, this script seems designed to be run before ASIC instance database0 and database1 generate database-config.json.
For other code using load_db_config this issue should not happen.
SonicDBConfig.load_sonic_global_db_config(namespace=args.namespace) | ||
else: | ||
if not SonicDBConfig.isInit(): | ||
SonicDBConfig.load_sonic_db_config() | ||
|
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 feel it is more like a timing issue as you pointed out, Can we call db_migrator, after /var/run/redis0/sonic-db/database-config.json is created ?
Here, https://github.com/sonic-net/sonic-swss-common/blob/7db6ccf5fb427d4832d6c54ab1f485ddd87b6697/common/dbconnector.h#L107 -- it doesn't make a difference if we pass/don't pass the namespace parameter to load_sonic_global_db_config()
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 search sonic code but not found any code use the '-n' parameter of db_migrator.py to pass the namespace parameter.
I will add comments in the sonic-net/sonic-buildimage#18389 to ask author check if this can fix by your suggestion.
For load_sonic_global_db_config(namespace=args.namespace), I think it's better keep the parameter to keep the compatibility, incase we change the code of load_sonic_global_db_config in future.
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.
@liuh-80 @judyjoseph all instance [email protected] have dependency "Requires=database.service
After=database.service". db_mihgrator script in local host database will not be able to wait for all other instance ready unless we change the dependency.
…nfig or initialize(). This is a porting fix effort from master sonic-net#3257.
…GlobalConfig or initialize(). This is a porting fix effort from master sonic-net#3257." This reverts commit 5b281bb.
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.
Please check with other reviewers.
…on on Linecard (sonic-net#3257) Fix db_migrate.py show error and back trace while loading configuration on Linecard #### Why I did it Fix [issue @](sonic-net/sonic-buildimage#18389) #### How I did it Revert code change by sonic-net#3100 Check DB config initialize state and ignore when initialized. #### How to verify it Pass all UT. Manually test. ##### Work item tracking - Microsoft ADO **(number only)**: 27384235 #### Which release branch to backport (provide reason below if selected) N/A #### Description for the changelog Fix db_migrate.py show error and back trace while loading configuration on Linecard #### A picture of a cute animal (not mandatory but encouraged)
Fix db_migrate.py show error and back trace while loading configuration on Linecard
Why I did it
How I did it
How to verify it
Work item tracking
Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)