Skip to content
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 mutidb warmboot support - restoring database #5773

Merged
merged 7 commits into from
Dec 10, 2020

Conversation

dzhangalibaba
Copy link
Collaborator

@dzhangalibaba dzhangalibaba commented Nov 2, 2020

@dzhangalibaba
Copy link
Collaborator Author

dzhangalibaba commented Nov 3, 2020

@qiluo-msft These two PRs are for multiDB warmboot support as proposed earlier.

Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comments

qiluo-msft pushed a commit to sonic-net/sonic-utilities that referenced this pull request Dec 3, 2020
* lua script to backup all database into one database and create rdb file - following earlier multiDB warmboot design at https://github.com/Azure/SONiC/blob/master/doc/database/multi_database_instances.md
* copy this rdb file as before to WARM_DIR
* restoring database part is in another PR at sonic-buildimage  sonic-net/sonic-buildimage#5773, they depend on each other
@dzhangalibaba
Copy link
Collaborator Author

@qiluo-msft I saw another pr is merged sonic-net/sonic-utilities#1205

We need this PR ready as well if we want to update sonic-utilities sub module, otherwise warm boot will be broken. Please take a look on this as well

@qiluo-msft
Copy link
Collaborator

qiluo-msft commented Dec 4, 2020

Offline discussed.
sonic-net/sonic-utilities#1205 is backward-compatible. So it is safe to merge that first. #Closed

qiluo-msft
qiluo-msft previously approved these changes Dec 4, 2020
mkdir -p /var/lib/$inst
if [[ -f $DUMPFILE ]]; then
# copy warmboot rdb file into each new instance location
cp $DUMPFILE /var/lib/$inst/dump.rdb
Copy link
Collaborator

@qiluo-msft qiluo-msft Dec 4, 2020

Choose a reason for hiding this comment

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

If cp source file is same as the destination, there will be error message and exit code. #Closed

Copy link
Collaborator Author

@dzhangalibaba dzhangalibaba Dec 5, 2020

Choose a reason for hiding this comment

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

I tried it on DUT earlier, it just shows a msg telling A and B are the same file, it didn't affect the function, also we don't use the exit code in docker-database-init.sh.

Copy link
Collaborator

@qiluo-msft qiluo-msft Dec 5, 2020

Choose a reason for hiding this comment

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

cp -n fits this use case better.


In reply to: 536449280 [](ancestors = 536449280)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agree. Updated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I revisit the behavior, to not change today’s behavior, cp -u is correct instead of -n. There is a case we need to overwrite. For example, after warm boot, we do a cold boot, an empty rdb file wil overwrite the previous warm boot rdb file. So -u is correct, only newer source will be copied.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the insight and sorry for the misleading. Even cp -u is not exactly what you truly need, considering the extreme the case the time adjusted backward. So either option is ok:

  1. revert back to bare cp, and keep error messages
  2. check $DUMPFILE != /var/lib/$inst/dump.rdb before cp

In reply to: 536868596 [](ancestors = 536868596,536449280)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am checking if src and dst file is the same. Updated

Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

One more comment

qiluo-msft
qiluo-msft previously approved these changes Dec 5, 2020
@dzhangalibaba
Copy link
Collaborator Author

@qiluo-msft looks everything passed. Could we merg this and at the same time , I'll open another for updating sonic-utilities

@qiluo-msft qiluo-msft merged commit b2a3de5 into sonic-net:master Dec 10, 2020
@dzhangalibaba dzhangalibaba deleted the multidbwb branch December 10, 2020 20:03
qiluo-msft pushed a commit that referenced this pull request Dec 11, 2020
…acking up database (#6181)

update sonic-utilites submodule, adding MultiDB warmboot backing up database support:

*    [MultiDB] add MultiDB warmboot support - backing up database (#1205) 
    * lua script to backup all database into one database and create rdb file - following earlier multiDB warmboot design at https://github.com/Azure/SONiC/blob/master/doc/database/multi_database_instances.md
    * copy this rdb file as before to WARM_DIR
    * restoring database part is in another PR at sonic-buildimage  #5773, they depend on each other
malletvapid23 added a commit to malletvapid23/Sonic-Utility that referenced this pull request Aug 3, 2023
* lua script to backup all database into one database and create rdb file - following earlier multiDB warmboot design at https://github.com/Azure/SONiC/blob/master/doc/database/multi_database_instances.md
* copy this rdb file as before to WARM_DIR
* restoring database part is in another PR at sonic-buildimage  sonic-net/sonic-buildimage#5773, they depend on each other
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants