Bug #19624
closedBetter documentation of interaction between priority 0 and container state
Added by Peter Amstutz about 2 years ago. Updated over 1 year ago.
Description
current comment at https://doc.arvados.org/main/api/methods/container_requests.html is
Clients are expected to submit container requests with zero priority in order to preview the container that will be used to satisfy it. Priority can be null if and only if state!=“Committed”. See below for more details .
"below" is
Priority 0 means no container should run on behalf of this request, and containers already running will be terminated (setting container priority to 0 is the cancel operation.)
Priority 1 is the lowest priority.
Priority 1000 is the highest priority.
Let's add a diagram showing the possible states and state changes.
Files
7da8a84dd.png (72.9 KB) 7da8a84dd.png | Tom Clegg, 01/30/2023 08:44 PM | ||
32ec983b8.png (55.9 KB) 32ec983b8.png | Tom Clegg, 01/30/2023 10:47 PM | ||
PXL_20230131_144714264.jpg (1.31 MB) PXL_20230131_144714264.jpg | Brett Smith, 01/31/2023 02:52 PM | ||
d2c770ed6.png (66.8 KB) d2c770ed6.png | Tom Clegg, 01/31/2023 09:29 PM | ||
e6da4efc4.png (69.7 KB) e6da4efc4.png | Tom Clegg, 02/01/2023 07:16 PM | ||
ContainerRequestLifecycle.dot (2.76 KB) ContainerRequestLifecycle.dot | Brett Smith, 02/01/2023 09:14 PM | ||
ContainerRequestLifecycle.dot.svg (11.8 KB) ContainerRequestLifecycle.dot.svg | Brett Smith, 02/01/2023 09:14 PM |
Updated by Tom Clegg about 2 years ago
- Assigned To set to Tom Clegg
- Description updated (diff)
Updated by Peter Amstutz about 2 years ago
- Target version changed from 2022-11-09 sprint to 2022-11-23 sprint
Updated by Peter Amstutz about 2 years ago
- Target version changed from 2022-11-23 sprint to 2022-12-07 Sprint
Updated by Peter Amstutz about 2 years ago
- Target version changed from 2022-12-07 Sprint to 2022-12-21 Sprint
Updated by Peter Amstutz almost 2 years ago
- Target version changed from 2022-12-21 Sprint to 2023-01-18 sprint
Updated by Peter Amstutz almost 2 years ago
- Target version changed from 2023-01-18 sprint to 2023-02-01 sprint
Updated by Peter Amstutz almost 2 years ago
- Target version changed from 2023-02-01 sprint to 2023-02-15 sprint
Updated by Peter Amstutz almost 2 years ago
- Target version changed from 2023-02-15 sprint to Future
Updated by Peter Amstutz almost 2 years ago
- Release set to 59
- Story points set to 0.5
- Target version deleted (
Future) - Assigned To deleted (
Tom Clegg)
Updated by Peter Amstutz almost 2 years ago
- Target version set to To be scheduled
Updated by Peter Amstutz almost 2 years ago
- Target version changed from To be scheduled to 2023-02-01 sprint
Updated by Peter Amstutz almost 2 years ago
Tom Clegg wrote in #note-17:
Another possibility
Yes, I like this one better.
Might be worth noting you can go to "Cancelled" state from any of the Committed states, and then making it clearer that "Cancelled" can happen as a result of either "system error" or "priority set to 0".
Updated by Brett Smith almost 2 years ago
Putting up my own idea. Apologies for my chicken scratch. Here are the key things I think are relevant:
- We present the lifecycle as basically linear, and then throughout that process there are a series of off-ramps that all lead to the container request having state=Final. I think this lends itself well to a two-column presentation, where the first column represents the full normal lifecycle and then the second column are the end states that the different off-ramps take you to.
- Boxes represent object state, arrows represent different actors changing those objects' state. Boxes document what object is changing (container request and/or container) and the key field changes that occurred from the action. Arrows document both who is acting on an object (user, API server independently, Crunch) and what the change is.
- IMO the two-layer presentation of container request state and container state in Tom's suggestion is a little too clever and dense. I would rather have each box document both the state of the container request and its associated container (if any).
- I used a lot of abbreviations for space. We should spell everything out. CR = container request. c_uuid =
container_uuid
. "User cancels CR" = User updates container request withpriority
=0. My descriptions of the transitions between Queued→Locked→Running might be a little off, please correct, I more wanted to get the structure out than exactly right. - We could highlight other key fields that change like
log_uuid
,output_uuid
, etc. - I forgot to draw an arrow for "Container runs to completion," oops.
- We could probably add judicious color outlines to distinguish end states with no container, cancelled container, failed container, and successful container.
Updated by Tom Clegg almost 2 years ago
- File d2c770ed6.png d2c770ed6.png added
I gave up trying to coerce the graphviz layout. Using color to indicate the end states seems to work OK though.
Updated by Brett Smith almost 2 years ago
I like this version best so far. I appreciate the simplification of "user does something" vs. "Arvados does something" without getting into specific components, I think that's a good improvement, thanks.
Without specifying the presentation too much here are things I worry about:
- I'm not sure how I feel about the "assigned" shorthand. Do we use that anywhere else? For newer Arvados users I'm not sure if it would be clear enough.
- For some of the arrow labels, especially labels that are rendered between two arrows, I don't think it's obvious which arrow they go with.
- In general the way the whole graph tends to lean left just feels really awkward. Even if we could just get it to lean right instead, I feel like that would be an improvement, since it matches the direction of our text.
Updated by Peter Amstutz almost 2 years ago
- Target version changed from 2023-02-01 sprint to 2023-02-15 sprint
Updated by Tom Clegg almost 2 years ago
19624-priority-doc @ e6da4efc43f0b913964eb570c0f52336573abbe8
Updated by Brett Smith almost 2 years ago
- File ContainerRequestLifecycle.dot.svg ContainerRequestLifecycle.dot.svg added
- File ContainerRequestLifecycle.dot ContainerRequestLifecycle.dot added
If you like this version I can just tack it onto the branch, let me know.
Updated by Brett Smith almost 2 years ago
Tom Clegg wrote in #note-24:
19624-priority-doc @ e6da4efc43f0b913964eb570c0f52336573abbe8
The text and the way the diagram is included look good to me. Let me know what you think about the edited diagram and I think we're good either way. Thanks.
Updated by Tom Clegg almost 2 years ago
Brett Smith wrote in #note-25:
If you like this version I can just tack it onto the branch, let me know.
Yes please. That layout is definitely better.
Updated by Brett Smith almost 2 years ago
Tom Clegg wrote in #note-27:
Yes please. That layout is definitely better.
f196aa21e83d75d8fb2e77c9119a650034cb5cac - Unless you see any issues I think this is good to merge, thanks.
Updated by Tom Clegg almost 2 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|e75f2cd097eb9c8e541576fadce46e09c51c7dab.