Skip to content
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

[Serve] User custom class name for replica class #26574

Merged
merged 4 commits into from
Jul 15, 2022

Conversation

simon-mo
Copy link
Contributor

Why are these changes needed?

The actor class name for replica wrappers are long and hard to read. This PR uses type to create the wrapper class so the class name is more informative. This behavior is also compatible with previous approach using __name__. But previous approach only affect the proctitle of worker process; while this approach change both proctitle and how Ray recognize the class name.

This make Serve looks way better through observability api.

## before
(base) ➜  ray list actors
---
-   actor_id: 36eaf5c0b16cacee19eed7b101000000
    class_name: ServeController
    name: SERVE_CONTROLLER_ACTOR
    pid: 43088
    state: ALIVE
-   actor_id: a68ec943681d5305aec6c24201000000
    class_name: create_replica_wrapper.<locals>.RayServeWrappedReplica
    name: SERVE_REPLICA::b#ZVpdBy
    pid: 43104
    state: ALIVE
-   actor_id: e4f7395a757050e6f49ce2df01000000
    class_name: HTTPProxyActor
    name: SERVE_CONTROLLER_ACTOR:SERVE_PROXY_ACTOR-node:127.0.0.1-0
    pid: 43098
    state: ALIVE

## after
(base) ➜ ray list actors
---
-   actor_id: 341f10755762b5c428eb54ec01000000
    class_name: ServeController
    name: SERVE_CONTROLLER_ACTOR
    pid: 44665
    state: ALIVE
-   actor_id: 44d9d9286c8783862898e0da01000000
    class_name: ServeReplicaClass:b
    name: SERVE_REPLICA::b#souBqw
    pid: 44672
    state: ALIVE
-   actor_id: 56968306b7f8c2f79a56b6e701000000
    class_name: HTTPProxyActor
    name: SERVE_CONTROLLER_ACTOR:SERVE_PROXY_ACTOR-node:127.0.0.1-0
    pid: 44667
    state: ALIVE

Related issue number

None

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@simon-mo simon-mo marked this pull request as ready for review July 14, 2022 19:58
@rickyyx
Copy link
Contributor

rickyyx commented Jul 14, 2022


via GIPHY

@edoakes edoakes added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Jul 14, 2022
@edoakes
Copy link
Contributor

edoakes commented Jul 14, 2022

@simon-mo nice, I noticed this while using the actors API too, you beat me to it ;)

@rkooo567
Copy link
Contributor

This is awesome :)!

Signed-off-by: simon-mo <[email protected]>
Signed-off-by: simon-mo <[email protected]>
Signed-off-by: simon-mo <[email protected]>
@simon-mo simon-mo merged commit df9f891 into ray-project:master Jul 15, 2022
truelegion47 pushed a commit to truelegion47/ray that referenced this pull request Jul 16, 2022
xwjiang2010 pushed a commit to xwjiang2010/ray that referenced this pull request Jul 19, 2022
avnishn pushed a commit to smorad/ray that referenced this pull request Jul 20, 2022
klwuibm pushed a commit to yuanchi2807/ray that referenced this pull request Jul 27, 2022
franklsf95 pushed a commit to franklsf95/ray that referenced this pull request Aug 2, 2022
gramhagen pushed a commit to gramhagen/ray that referenced this pull request Aug 15, 2022
gramhagen pushed a commit to gramhagen/ray that referenced this pull request Aug 15, 2022
Stefan-1313 pushed a commit to Stefan-1313/ray_mod that referenced this pull request Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants