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

[Spot] Add cancelling state for the spot job #1785

Merged
merged 9 commits into from
Mar 17, 2023
Merged

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Mar 16, 2023

According to the #1745, we now set the CANCELLED state after the spot cluster is fully clean up. That will keep the spot job in the previous job for a while, which can feels unresponsive.
Thanks to @suquark 's great idea, we are now adding a CANCELLING state for the spot job to improve the UX.

This will also solve the issue #1745 (comment)
Because, previously, our spot job is not set to terminal state immediately, the log streaming will misclassified it as preempted.

TODOs:

  • Check and fix the smoke tests for cancellation

Tested (run the relevant ones):

  • Any manual or new tests for this PR (please specify below)
    • sky spot launch sleep 600 --cloud gcp and sky spot cancel -a cancel it during the starting period
    • sky spot launch sleep 600 --cloud gcp and sky spot cancel -a cancel it during the log is streaming
  • All smoke tests: pytest tests/test_smoke.py --managed-spot

@suquark suquark self-assigned this Mar 17, 2023
sky/spot/spot_state.py Outdated Show resolved Hide resolved
sky/spot/spot_state.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@suquark suquark left a comment

Choose a reason for hiding this comment

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

LGTM. This PR can be merged after fix the comment + tests passed

@Michaelvll
Copy link
Collaborator Author

Michaelvll commented Mar 17, 2023

Tested:

  • pytest tests/test_smoke.py --managed-spot

@Michaelvll Michaelvll merged commit 75775d3 into master Mar 17, 2023
@Michaelvll Michaelvll deleted the spot-cancelling branch March 17, 2023 05:47
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.

2 participants