Project

General

Profile

Actions

Bug #19624

closed

Better documentation of interaction between priority 0 and container state

Added by Peter Amstutz over 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Crunch
Target version:
Story points:
0.5
Release relationship:
Auto

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

Subtasks 1 (0 open1 closed)

Task #19643: Review 19624-priority-docResolvedTom Clegg02/02/2023Actions
Actions #2

Updated by Tom Clegg over 1 year ago

  • Description updated (diff)
Actions #3

Updated by Tom Clegg over 1 year ago

  • Assigned To set to Tom Clegg
  • Description updated (diff)
Actions #4

Updated by Peter Amstutz over 1 year ago

  • Target version changed from 2022-11-09 sprint to 2022-11-23 sprint
Actions #5

Updated by Peter Amstutz over 1 year ago

  • Target version changed from 2022-11-23 sprint to 2022-12-07 Sprint
Actions #6

Updated by Peter Amstutz over 1 year ago

  • Target version changed from 2022-12-07 Sprint to 2022-12-21 Sprint
Actions #7

Updated by Peter Amstutz over 1 year ago

  • Target version changed from 2022-12-21 Sprint to 2023-01-18 sprint
Actions #8

Updated by Peter Amstutz over 1 year ago

  • Target version changed from 2023-01-18 sprint to 2023-02-01 sprint
Actions #9

Updated by Peter Amstutz over 1 year ago

  • Target version changed from 2023-02-01 sprint to 2023-02-15 sprint
Actions #10

Updated by Peter Amstutz over 1 year ago

  • Target version changed from 2023-02-15 sprint to Future
Actions #11

Updated by Peter Amstutz over 1 year ago

  • Release set to 59
  • Story points set to 0.5
  • Target version deleted (Future)
  • Assigned To deleted (Tom Clegg)
Actions #12

Updated by Peter Amstutz over 1 year ago

  • Target version set to To be scheduled
Actions #13

Updated by Peter Amstutz about 1 year ago

  • Target version changed from To be scheduled to 2023-02-01 sprint
Actions #14

Updated by Peter Amstutz about 1 year ago

  • Assigned To set to Tom Clegg
Actions #15

Updated by Tom Clegg about 1 year ago

  • Status changed from New to In Progress
Actions #16

Updated by Tom Clegg about 1 year ago

Draft for discussion

Actions #17

Updated by Tom Clegg about 1 year ago

Another possibility

Actions #18

Updated by Peter Amstutz about 1 year 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".

Actions #19

Updated by Brett Smith about 1 year 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 with priority=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.
Actions #20

Updated by Tom Clegg about 1 year ago

I gave up trying to coerce the graphviz layout. Using color to indicate the end states seems to work OK though.

Actions #21

Updated by Brett Smith about 1 year 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.
Actions #22

Updated by Peter Amstutz about 1 year ago

  • Target version changed from 2023-02-01 sprint to 2023-02-15 sprint
Actions #23

Updated by Tom Clegg about 1 year ago

Actions #24

Updated by Tom Clegg about 1 year ago

Actions #25

Updated by Brett Smith about 1 year ago

If you like this version I can just tack it onto the branch, let me know.

Actions #26

Updated by Brett Smith about 1 year 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.

Actions #27

Updated by Tom Clegg about 1 year 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.

Actions #28

Updated by Brett Smith about 1 year 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.

Actions #29

Updated by Tom Clegg about 1 year ago

  • Status changed from In Progress to Resolved
Actions #30

Updated by Peter Amstutz about 1 year ago

  • Release set to 57
Actions

Also available in: Atom PDF