-
Notifications
You must be signed in to change notification settings - Fork 656
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
[MultiDB] add MultiDB warmboot support - backing up database #1205
Conversation
@qiluo-msft These two PRs are for multiDB warmboot support as proposed earlier. |
scripts/migrate_warmboot_database
Outdated
args = parser.parse_args() | ||
|
||
if args.target_db: | ||
try: |
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.
[](start = 0, length = 12)
too much indentation. #Closed
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
scripts/fast-reboot
Outdated
docker exec -i database rm /var/lib/redis/$REDIS_FILE | ||
|
||
# move all db data into instances where APPL_DB locates | ||
migrate_warmboot_database APPL_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.
migrate_warmboot_database [](start = 4, length = 25)
The name is misleading. It is not directly related to warmboot.
Suggestion centralize_database
#Closed
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.
updated
scripts/migrate_warmboot_database
Outdated
dbport = swsssdk.SonicDBConfig.get_port(dbname) | ||
# if the db is on the same instance, no need to migrate | ||
if dbport == target_dbport: | ||
continue |
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.
Is it enough to determine "the same instance" ? #Closed
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.
good catch. hostname plus port is enough to determine the "unique instance" . Updated
scripts/fast-reboot
Outdated
# move all db data into instances where APPL_DB locates | ||
migrate_warmboot_database APPL_DB | ||
|
||
inst=`/usr/bin/python -c "import swsssdk; print(swsssdk.SonicDBConfig.get_instancename('APPL_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.
inst [](start = 4, length = 4)
Instead of using a new process to locate APPL_DB, you may just print it to stdout in above command. #Closed
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.
updated
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.
As comments
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.
Can you point me to the code where the centralized DB are redistributed?
* restoring each database with all data before warmboot and then flush unused data in each instance, following the multiDB warmboot design at https://github.com/Azure/SONiC/blob/master/doc/database/multi_database_instances.md * restore needs to be done in database docker since we need to know the database_config.json in new version * copy all data rdb file into each instance restoration location andthen flush unused database * other logic is the same as before * backing up database part is in another PR at sonic-utilities sonic-net/sonic-utilities#1205, they depend on each other
Signed-off-by: Dong Zhang d.zhang@alibaba-inc.com