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

[mellanox|ffb] use system level warm reboot for Mellanox fastfast boot #413

Merged
merged 5 commits into from
Jan 7, 2019

Conversation

stepanblyschak
Copy link
Contributor

@stepanblyschak stepanblyschak commented Dec 11, 2018

Signed-off-by: Stepan Blyschak stepanb@mellanox.com

- What I did

  • use system warmboot for Mellanox fastfast boot flow.
  • 'backup_datebase' -> 'backup_database'
  • don't fail if 'pkill -USR1 teamd' failed because no teamd processes are running in teamd docker

- How I did it

- How to verify it

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

-->

Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
backup_datebase
# Warm reboot: dump state to host disk
if [[ "$REBOOT_TYPE" = "fastfast-reboot" ]]; then
redis-cli -n 1 FLUSHDB &> /dev/null
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 11, 2018

Choose a reason for hiding this comment

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

& [](start = 31, length = 1)

Do you intend to run the command in background? If yes, please also wait until all of them finished. #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

&> is piping stdout and stderr together to some file, in this case /dev/null. It won't cause background execution.

backup_datebase
# Warm reboot: dump state to host disk
if [[ "$REBOOT_TYPE" = "fastfast-reboot" ]]; then
redis-cli -n 1 FLUSHDB &> /dev/null
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 11, 2018

Choose a reason for hiding this comment

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

FLUSHDB [](start = 23, length = 7)

Similar flush code is in files files/scripts/{swss,syncd}.sh. Could you add your branch there? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not put flush there because of two reasons

  1. if system warm restart flag is enabled and platform mellanox swss.sh flushes counters db so it will affect docker level swss restart
  2. I don't think there is need to backup something that will be flushed later

Copy link
Contributor

@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

@yxieca
Copy link
Contributor

yxieca commented Dec 12, 2018

Thanks for muting these background outputs.

There is one more at https://github.com/Azure/sonic-utilities/blob/b9c6840debc01b45c6a653fe605d2654481abccb/scripts/fast-reboot#L231, check_restart could also print something. Can you mute this call as well?

Other than that, looks good to me.

Thanks,
Ying

Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
scripts/fast-reboot Outdated Show resolved Hide resolved
Stepan Blyschak added 2 commits December 14, 2018 12:56
…wn requests

Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
@@ -163,27 +164,17 @@ case "$REBOOT_TYPE" in
REBOOT_TYPE="fastfast-reboot"
Copy link
Contributor

Choose a reason for hiding this comment

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

REBOOT_TYPE="fastfast-reboot" [](start = 12, length = 29)

We'd better treat fastfast-reboot as Mellanox implementation of warm-reboot, and keep REBOOT_TYPE as warm-reboot.

Copy link
Collaborator

Choose a reason for hiding this comment

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

it is a different flow and i think we should keep it different just in case warmboot will have flows which we should not use.
I prefer keeping it that way

Copy link
Contributor

@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.

Sorry for late comment. Please check

@lguohan lguohan dismissed qiluo-msft’s stale review January 7, 2019 18:04

issue addressed

@lguohan lguohan merged commit 3ce8952 into sonic-net:master Jan 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants