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

[core] Use ActorState_Name to print ActorState #49483

Merged

Conversation

kevin85421
Copy link
Member

@kevin85421 kevin85421 commented Dec 29, 2024

Why are these changes needed?

enum ActorState {
// Actor info is registered in GCS. But its dependencies are not ready.
DEPENDENCIES_UNREADY = 0;
// Actor local dependencies are ready. This actor is being created.
PENDING_CREATION = 1;
// Actor is alive.
ALIVE = 2;
// Actor is dead, now being restarted.
// After reconstruction finishes, the state will become alive again.
RESTARTING = 3;
// Actor is already dead and won't be restarted.
DEAD = 4;
}

ActorState is an enum defined in Protobuf. If ActorState_Name is not called, an integer will be printed instead. For example, rpc::ActorTableData::DEAD would print as 4.

  • Without this PR: "with status 4"
    Screenshot 2024-12-28 at 8 16 07 PM

  • With this PR: "with status DEAD"
    Screenshot 2024-12-28 at 10 40 01 PM

I also considered implementing std::ostream &operator<<(std::ostream &out, ...). However, there are several issues:

  • ActorState can be printed directly without causing compilation errors, making it difficult to determine whether the overloaded operator is being called.
  • A file that references ActorState does not necessarily #include the utility function operator<<.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • 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 added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • 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 :(

Signed-off-by: Kai-Hsun Chen <[email protected]>
@kevin85421 kevin85421 marked this pull request as ready for review December 29, 2024 06:49
@kevin85421 kevin85421 requested a review from a team as a code owner December 29, 2024 06:49
@kevin85421 kevin85421 added the go add ONLY when ready to merge, run all tests label Dec 29, 2024
@rynewang rynewang merged commit 44dbb59 into ray-project:master Dec 29, 2024
5 checks passed
justinvyu pushed a commit to justinvyu/ray that referenced this pull request Dec 30, 2024
`ActorState` is an enum defined in Protobuf. If `ActorState_Name` is not
called, an integer will be printed instead. For example,
`rpc::ActorTableData::DEAD` would print as 4.

* Without this PR: "with status 4"
* With this PR: "with status DEAD"

I also considered implementing `std::ostream &operator<<(std::ostream
&out, ...)`. However, there are several issues:

* `ActorState` can be printed directly without causing compilation
errors, making it difficult to determine whether the overloaded operator
is being called.
* A file that references `ActorState` does not necessarily `#include`
the utility function `operator<<`.

Signed-off-by: Kai-Hsun Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants