[Core] Fix negative ALIVE actors metric #45718
Conversation
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
src/ray/core_worker/core_worker.h
Outdated
| } | ||
| ray::stats::STATS_actors.Record(-(running + in_get + in_wait), | ||
| ray::stats::STATS_actors.Record(alive, | ||
| {{"State", "ALIVE"}, |
There was a problem hiding this comment.
Should this actually be "IDLE" ?
There was a problem hiding this comment.
It should but it's a backward incompatible change I think.
|
Not a huge priority for the customer but just checking for any update on this one |
|
@jjyao any update on this one? I see it was blocked by some pre-merge tests. |
jjyao
changed the title
| @@ -59,7 +59,8 @@ DEFINE_stats( | |||
| DEFINE_stats(actors, | |||
| "Current number of actors currently in a particular state.", | |||
There was a problem hiding this comment.
In the long run, I wonder if it makes more sense to separate this metric cleanly rather than relying on the executor as a label. It seems tricky for users to know what the implications of source is.
There was a problem hiding this comment.
I feel if we document it correctly it should be fine? Although currently source is not documented at all.
| ) | ||
| ], | ||
| ), | ||
| Panel( |
There was a problem hiding this comment.
can we just replace the Scheduler Actor State graph completely? Or is it still useful?
There was a problem hiding this comment.
yes, I think it's still useful because it has states that's not available in the other graph.
Why are these changes needed?
Currently GCS emits the total number of ALIVE actors and each core worker emits
-1if its actor is ALIVE but not idle. The sum is total number of idle actors (we call it state ALIVE while it really means IDLE). Since GCS and core worker emits metrics at different time, the sum might be negative.This PR fixes this by letting gcs and core worker emits different actor states:
Metrics dashboard is updated accordingly.
With this change, state ALIVE actually means alive instead of the old (wrong) idle semantic and we have a new state IDLE to indicate whether the actor is idle or not.
Related issue number
Closes #46232
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.