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.

Sign up for GitHub

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

Jump to bottom

[Java] Make both RayActor and RayPyActor inheriting from BaseActor #7462

Merged
merged 12 commits into from
Mar 17, 2020
Merged

[Java] Make both RayActor and RayPyActor inheriting from BaseActor #7462

merged 12 commits into from
Mar 17, 2020

Conversation

kfstorm
Copy link
Member

@kfstorm kfstorm commented Mar 5, 2020 •
edited

Why are these changes needed?

RayActor originally represents a Java actor handle but inherited by RayPyActor after cross-languages features were added. It's confusing that RayActor can represent both Java and Python actor handles but there's no dedicated interface for Java actor handles. And in RayRuntime interface we have RayObject callActor(RayFunc func, RayActor<?> actor, Object[] args), which we can't tell whether the actor parameter accepts only a Java actor handle based on the method signature.

In this PR,

  • Add the base interface BaseActor to represent an actor handle of any language. Both RayActor and RayPyActor inherit from it.
  • RayActor now represents a Java actor handle only.

Related issue number

#7291

Checks

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/22767/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/22768/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/22900/
Test FAILed.

jovany-wang
jovany-wang approved these changes Mar 16, 2020
View reviewed changes
Copy link
Contributor

@jovany-wang jovany-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@kfstorm kfstorm mentioned this pull request Mar 16, 2020
Merged
5 tasks
@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/23222/
Test FAILed.

@kfstorm kfstorm changed the title [Java] Add interface RayJavaActor and it inherits from RayActor [Java] Make both RayActor and RayPyActor inheriting from BaseActor Mar 16, 2020
@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/23227/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/23239/
Test FAILed.

@raulchen
Copy link
Contributor

The failed Java and GCS service CI jobs might be unrelated. But the streaming one should be.

@kfstorm
Copy link
Member Author

kfstorm commented Mar 17, 2020

@raulchen The streaming CI failure is caused by a maven package downloading error. I retied and it passed.

@raulchen
Copy link
Contributor

@kfstorm ok, can you rebase and resolve the conflict?

@raulchen raulchen merged commit 6b888b0 into ray-project:master Mar 17, 2020
@raulchen raulchen deleted the ray_actor_base_class_fix branch March 17, 2020 13:46
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/23283/
Test PASSed.

stephanie-wang pushed a commit to stephanie-wang/ray that referenced this pull request Mar 17, 2020
@kfstorm @stephanie-wang
[Java] Make both RayActor and RayPyActor inheriting from `BaseAct…
e6d17f4 
…or` (ray-project#7462)
stephanie-wang added a commit that referenced this pull request Mar 19, 2020
@stephanie-wang
Enable distributed ref counting by default (#7628)
b499100 
* enable

* Turn on eager eviction

* Shorten tests and drain ReferenceCounter

* Don't force kill actor handles that have gone out of scope, lint

* Fix locks

* Cleanup Plasma Async Callback (#7452)

* [rllib][tune] fix some nans (#7611)

* Change /tmp to platform-specific temporary directory (#7529)

* [Serve] UI Improvements (#7569)

* bugfix about test_dynres.py (#7615)

Co-authored-by: senlin.zsl <senlin.zsl@antfin.com>

* Java call Python actor method use actor.call (#7614)

* bug fix about useage of absl::flat_hash_map::erase and absl::flat_hash_set::erase (#7633)

Co-authored-by: senlin.zsl <senlin.zsl@antfin.com>

* [Java] Make both `RayActor` and `RayPyActor` inheriting from `BaseActor` (#7462)

* [Java] Fix the issue that the cached value in `RayObject` is serialized (#7613)

* Add failure tests to test_reference_counting (#7400)

* Fix typo in asyncio documentation (#7602)

* Fix segfault

* debug

* Force kill actor

* Fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raulchen raulchen raulchen approved these changes

@jovany-wang jovany-wang jovany-wang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@kfstorm @AmplabJenkins @raulchen @jovany-wang