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

fix: wait all jobs done #1010

Merged
merged 1 commit into from
Aug 26, 2021
Merged

fix: wait all jobs done #1010

merged 1 commit into from
Aug 26, 2021

Conversation

KevinWu0904
Copy link
Contributor

Signed-off-by: KevinWu0904 [email protected]

Signed-off-by: KevinWu0904 <[email protected]>
@KevinWu0904
Copy link
Contributor Author

Actually, the underlaying robig/cron lib's stop function will return a context for developers to wait. So if we want to stop all jobs gracefully, maybe we should wait the context done. Correct me if any problems, thanks!

image

@yvanoers
Copy link
Collaborator

What problem does this solve?

Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

LGTM

@vcastellm
Copy link
Member

@yvanoers I managed to reproduce the case when gracefully exiting the jobs doesn't write the finished status because the raft layer is already gone, but with this change and another small change, the final status is correctly recorded, see
image

So I will merge this and create a new PR with the extra change

@vcastellm vcastellm merged commit f5c9553 into distribworks:master Aug 26, 2021
@KevinWu0904
Copy link
Contributor Author

@Victorcoder Thanks !

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