feat: adds the preemptible task hint - #728
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.
|
@patmagee @geoffjentry @vsmalladi I have redesigned this proposal for WDL 1.4 by separating the preferred number of preemptible attempts from the requirement to use preemptible instances exclusively. The new Boolean Preemptions now count only against I chose two flat attributes to express the full range of options without introducing new type machinery. WDL cannot cleanly express The spec also defines “preemptible instance” and explicitly states that absence of |
727c22a to
128301d
Compare
There was a problem hiding this comment.
@claymcleod My major concern is the split between preemptible_only and preemptible in requirments and hints
If the specification states, "When preemptible_only is true, the engine must honor this hint and must not run the task on a non-preemptible instance," then classifying preemptible_only as a hint breaks the fundamental definition of what a "hint" is supposed to be.
A hint can safely ignore by an engine. But the way its written now. if an engine is forced to fail the task because a setting is true, that setting is no longer a hint. It becomes a hard requirement.
I would suggest move preemtible to the requirement section to be cleanest.
|
The intended distinction is that |
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.