Skip to content

Remove the shutdown grace period #2568

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

Merged

Conversation

dominiklohmann
Copy link
Member

@dominiklohmann dominiklohmann commented Sep 8, 2022

This removes the vast.shutdown-grace-period functionality and its underlying mechanism. This has two reasons:

  1. The mechanism was broken, and in case of yet unpersisted data taking longer than the grace period to persist on shutdown caused both data loss and an infinite hang.

  2. It is the purpose of a service manager to issue hard kills to hanging process, not of the hanging process itself.

Ultimately we decided that the internal hard kill causes more issues than it helps solves, so now the new grace period is infinite: VAST simply waits for as long as it takes to shut down properly.

📝 Reviewer Checklist

Review this pull request by ensuring the following items:

  • All user-facing changes have changelog entries
  • User-facing changes are reflected on vast.io

@dominiklohmann dominiklohmann added the bug Incorrect behavior label Sep 8, 2022
@dominiklohmann dominiklohmann force-pushed the story/sc-36673/terminator-no-hard-kill-attempt branch from b4792a5 to 44153df Compare September 8, 2022 11:13
This removes the `vast.shutdown-grace-period` functionality and its underlying
mechanism. This has two reasons:

1. The mechanism was broken, and in case of yet unpersisted data taking longer
   than the grace period to persist on shutdown caused both data loss and an
   infinite hang.

2. It is the purpose of a service manager to issue hard kills to hanging
   process, not of the hanging process itself.

Ultimately we decided that the internal hard kill causes more issues than it
helps solves, so now the new grace period is infinite: VAST simply waits for as
long as it takes to shut down properly.
@dominiklohmann dominiklohmann force-pushed the story/sc-36673/terminator-no-hard-kill-attempt branch from 44153df to ccfef54 Compare September 8, 2022 11:14
@dominiklohmann dominiklohmann requested a review from mavam September 8, 2022 11:14
Copy link
Member

@mavam mavam left a comment

Choose a reason for hiding this comment

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

I inspected the diff. Overall, the changes seem reasonable. This is hard to test, but since it's only removal of functionality and potential side effects, VAST will be easier to test moving forward.

@dominiklohmann dominiklohmann merged commit 59f8185 into master Sep 8, 2022
@dominiklohmann dominiklohmann deleted the story/sc-36673/terminator-no-hard-kill-attempt branch September 8, 2022 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants