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 call Python actor method use actor.call #7614

Merged
merged 7 commits into from
Mar 17, 2020
Merged

Java call Python actor method use actor.call #7614

merged 7 commits into from
Mar 17, 2020

Conversation

fyrestone
Copy link
Contributor

Why are these changes needed?

Java use actor.call to call actor method, but Java call Python use Ray.callPy(pyActor, ...). These APIs are inconsistent.

Related issue number

Checks

  • [ x ] I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://ray.readthedocs.io/en/latest/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failure rates at https://ray-travis-tracker.herokuapp.com/.
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested (please justify below)

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

kfstorm
kfstorm reviewed Mar 16, 2020
View reviewed changes
python/ray/scripts/scripts.py
@@ -268,7 +268,7 @@ def dashboard(cluster_config_file, cluster_name, port):
@click.option(
"--internal-config",
default=None,
type=str,
type=json.loads,
Copy link
Member

Choose a reason for hiding this comment

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

What's this change for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it's a new bug in master. If I don't fix it, the Java tests can't start a local cluster.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kfstorm this seems related to your previous RP? Java CI is failing on master.

...ava/streaming-runtime/src/main/java/org/ray/streaming/runtime/schedule/JobSchedulerImpl.java Outdated Show resolved Hide resolved
java/api/src/main/java/org/ray/api/runtime/RayRuntime.java Show resolved Hide resolved
java/api/src/main/java/org/ray/api/ActorCall.java Outdated Show resolved Hide resolved
@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/23223/
Test FAILed.

@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/23226/
Test PASSed.

@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/23228/
Test PASSed.

raulchen
raulchen approved these changes Mar 16, 2020
View reviewed changes
java/api/src/main/java/org/ray/api/RayPyActor.java Outdated Show resolved Hide resolved
java/runtime/src/main/java/org/ray/runtime/util/generator/RayCallGenerator.java Outdated Show resolved Hide resolved
python/ray/scripts/scripts.py
@@ -268,7 +268,7 @@ def dashboard(cluster_config_file, cluster_name, port):
@click.option(
"--internal-config",
default=None,
type=str,
type=json.loads,
Copy link
Contributor

Choose a reason for hiding this comment

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

@kfstorm this seems related to your previous RP? Java CI is failing on master.

java/api/src/main/java/org/ray/api/PyActorCall.java Outdated Show resolved Hide resolved
@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/23258/
Test PASSed.

@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/23259/
Test PASSed.

@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/23263/
Test PASSed.

@raulchen raulchen merged commit 7697ea2 into ray-project:master Mar 17, 2020
@raulchen raulchen deleted the update_java_cross_language_api branch March 17, 2020 06:52
stephanie-wang pushed a commit to stephanie-wang/ray that referenced this pull request Mar 17, 2020
@fyrestone @stephanie-wang
Java call Python actor method use actor.call (ray-project#7614)
6dec079
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 <[email protected]>

* 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 <[email protected]>

* [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

@kfstorm kfstorm kfstorm approved these changes

@zhijunfu zhijunfu zhijunfu 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.
5 participants
@fyrestone @AmplabJenkins @raulchen @kfstorm @zhijunfu