Idea #3898
closed
[API] Job model has a single state attribute that can be updated and read instead of paying attention to running and success flags.
Description
- If state has changed to Running, set started_at to now
- If state has changed to Failed or Complete, set finished_at to now
- If state has changed to Cancelled, set cancelled_at to now
- If state has changed at all, set running and success (see below)
- If state has not changed, and any of {running, success, cancelled_at} have changed, update state accordingly
- If state is nil, update state according to running, success, cancelled_at
- If state has changed to Running, and is_locked_by_uuid is nil, return false
| running | success | cancelled_at | state |
| any | any | not nil | Cancelled |
| any | false | nil | Failed |
| any | true | nil | Complete |
| true | nil | nil | Running |
| false | nil | nil | Queued |
| state | running | success |
| Queued | false | nil |
| Running | true | nil |
| Cancelled | false | false |
| Failed | false | false |
| Complete | false | true |
Updated by Tom Clegg over 9 years ago
- Target version changed from Arvados Future Sprints to 2014-10-08 sprint
Updated by Peter Amstutz over 9 years ago
- Assigned To changed from Peter Amstutz to Radhika Chippada
Updated by Radhika Chippada over 9 years ago
- Status changed from New to In Progress
- Allowed States:
Queued, Running, Cancelled, Failed, Complete
- Use the following to determine state from current success and running attributes:
| Running | Success | State | |
| any | False | Failed | |
| any | True | Complete | |
| True | nil | Running | |
| Queued | started_at, is_locked_by_uuid, cancelled_at and success are all nil | ||
| Cancelled | cancelled_at attribute is set |
- Fail migration if Job cannot enter any of these 5 states.
Updated by Radhika Chippada over 9 years ago
I set started_at, cancelled_at, and finished_at to Time.now only when they are not set. Peter thinks the crunch job would be setting them, and better to set them if they are not already set.
Updated by Radhika Chippada over 9 years ago
- Status changed from In Progress to Resolved
- % Done changed from 50 to 100
Applied in changeset arvados|commit:3cc699f70a514878b7ce7adbc3c96e73f92fdf82.
Updated by Ward Vandewege over 9 years ago
- Status changed from Resolved to In Progress
Updated by Radhika Chippada over 9 years ago
Review comments for branch: 3898-job-state-attr-fixes
LGTM. I was also initially concerned about including !is_locked_by_uuid in checking the status when state is null. This seems to have been problematic and I do not see any harm in omitting this in this check.
Thanks for addressing this.
Updated by Tom Clegg over 9 years ago
+ # race condition for jobs that have just been grabbed by crunch-dispatch but haven't been marked as started yet...
- It's crunch-job that does the grabbing, not crunch-dispatch
- crunch-job sets
started_atandis_locked_by_uuidin the same update transaction, so is this race really possible?
I think the bigger problem to fix here is that Job is now trying to manipulate the old state fields in interesting ways, even when clients are not using the new state field. This seems likely to create/expose new bugs in code paths that we are just about to replace, which seems a dubious use of time...
Updated by Anonymous over 9 years ago
- Status changed from In Progress to Resolved
- % Done changed from 75 to 100
Applied in changeset arvados|commit:d6478bb286c0bb1e7b8af67fb4800db792200022.