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

[BT-504] cromwell graceful restarts #6769

Merged
merged 3 commits into from
Jun 9, 2022

Conversation

mspector
Copy link
Contributor

No description provided.

@mspector mspector marked this pull request as ready for review May 25, 2022 18:40
@mspector mspector requested review from aednichols and breilly2 May 25, 2022 18:43
Copy link
Contributor

@cjllanwarne cjllanwarne left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@aednichols aednichols left a comment

Choose a reason for hiding this comment

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

Nice find!

The code absolutely does look good and it is plausible to me that it solves the problem. It would be nice to see before/after timing diagram screenshots of the same workflow to make sure we have hit the right target.

@mspector
Copy link
Contributor Author

I ran a job that simply sleeps for 120 seconds, then requested the timing diagram ("Before the fix"). Then I restarted Cromwell with the fix, and requested the timing diagram again ("After the fix").

Before the fix:
before fix

After the fix:
after fix

In the "after the fix" image, the purple bar is "cromwell starting overhead", and lasts about 60 seconds. The maroon bar is "RunningJob", and also last about 60 seconds. It seems like the Maroon bar should have also been "RunningJob".

@mspector mspector merged commit 7e067d5 into develop Jun 9, 2022
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.

4 participants