Conversation
888c284 to
5acc081
Compare
|
Note that this was already informally implemented in Sprocket, as I believe we just copied how Cromwell handles this key. Thus, there is no associated PR—you can just grab the latest published version of Sprocket and try it out. It should work for any TES server that implements pre-emptible instances at a minimum. |
|
There is considerable debate about exactly how this should be implemented. Given this, I think it's best to exclude from the WDL 1.3 release. At this point, I'm not even positive I feel this feature should be stabilized in this exact form—the problem begs for a more formal specification IMO. |
|
I agree @claymcleod This should be moved to a 1.4 maybe |
|
I've been thinking about this more, and I agree with Geoff's general direction. I also agree with Venkat that we want more flexibility in the future, but it's not a regression to introduce this approach first before a more complex solution—we need time to figure out what that should look like. Given that, and the fact that we can formalize what both Sprocket and Cromwell do today, I think it's okay to move forward. A future |
|
And, to be clear, I'm still in favor of just bumping this to 1.4. |
|
If this isn't also clear from the answer above, I do think there is a world where we don't need a more complex way to outline an execution plan. We'll have to find that out as we go through the design process. |
7cc8871 to
d0b5c5f
Compare
|
|
||
| A hint to the execution engine that a task _may_ be tried using a preemptible instance up to the specified number of times. If all preemptible attempts fail due to preemption, the engine must fall back to executing on a non-preemptible instance (up to the limit imposed by `max_retries`). Engines that are not configured to use or do not support preemptible instances may ignore this hint entirely. |
There was a problem hiding this comment.
I feel like we should be clear here what a "preemptible" instance is.. at one point it went without saying since the clouds defined that behaviour for us, but now I think it would be worthwhile specifying it.
| ##### ✨ `preemptible` | ||
|
|
||
| * Accepted types: `Int` | ||
| * Default value: `0` |
There was a problem hiding this comment.
should we say something about the absence of preemptible indicates is the same as the default value preemptible: 0
This PR proposes the formalization of the
preemptiblehint, which is already supported in both Sprocket and Cromwell (albiet as a runtime key).Briefly,
preemptibleaims to indicate the number of times a preemptible instance may be tried before using a non-preemptible instance. Failures other than preemption are proposed to count against themax_retriesrequirement instead.Before submitting this PR, please make sure:
README.mdor other documentation to account for thesechanges (when appropriate).
CHANGELOG.mddescribing the change and linking back to your pull request.CONTRIBUTING.mddocument.