-
Notifications
You must be signed in to change notification settings - Fork 6.5k
[Core/Observability 1/N] Add a "running" state to task status #24651
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/ray/protobuf/common.proto
Outdated
// The task is submitted to a worker process to execute. | ||
SUBMITTED_TO_WORKER = 4; | ||
// The task has failed to be scheduled and being rescheduled. | ||
RESCHEDULED = 5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: Maybe it is unnecessary, and we can reuse SCHEDULED state here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Btw, one question I have is if "SUBMITTED_TO_WORKER" necessary. I originally wanted to implement "RUNNING", and I realized the current implementation cannot actually become "RUNNING" because for things like actor tasks, we have the execution side queuing (which means we can know the exact state only from the execution side, not a caller (owner) side). This is the full picture
|
src/ray/protobuf/common.proto
Outdated
// The task is submitted to a worker process to execute. | ||
SUBMITTED_TO_WORKER = 4; | ||
// The task has failed to be scheduled and being rescheduled. | ||
RESCHEDULED = 5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
We need a bit more work on |
<
8000
/tr>
Failed test should be unrelated |
This PR adds 2 more states into TaskStatus enum TaskStatus { // The task is scheduled properly and waiting for execution. // It includes time to deliver the task to the remote worker + queueing time // from the execution side. WAITING_FOR_EXECUTION = 5; // The task that is running. RUNNING = 6; }
Why are these changes needed?
This PR adds 2 more states into
TaskStatus
Related issue number
Checks
scripts/format.sh
to lint the changes in this PR.