[Java] Support direct actor call in Java worker by kfstorm · Pull Request #5504 · ray-project/ray

kfstorm · 2019-08-22T03:53:43Z

Why are these changes needed?

Worker/driver can submit task directly to an actor without going through raylet for better performance.

What do these changes do?

#5183 introduced direct actor call in core worker as a transport. This PR adds the option isDirectCall in ActorCreationOptoins to make it possible to turn on direct actor call mode for actors.

TODO:

Support actor checkpoint.
Fix tests currently skipped. I created issue Fix skipped Java test cases in direct actor call mode #5559 to address remaining tests to be fixed.

Related issue number

#5029

Linter

I've run scripts/format.sh to lint the changes in this PR.

AmplabJenkins · 2019-08-22T07:12:31Z

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

AmplabJenkins · 2019-08-22T15:36:21Z

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

…_call

AmplabJenkins · 2019-08-23T15:40:33Z

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

AmplabJenkins · 2019-08-23T17:47:58Z

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

AmplabJenkins · 2019-08-23T17:54:04Z

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

AmplabJenkins · 2019-08-23T21:08:30Z

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

AmplabJenkins · 2019-08-24T06:59:13Z

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

AmplabJenkins · 2019-08-26T16:34:06Z

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

AmplabJenkins · 2019-08-26T16:43:04Z

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

AmplabJenkins · 2019-08-27T11:28:49Z

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

…_actor_direct_call

AmplabJenkins · 2019-09-06T17:35:20Z

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

AmplabJenkins · 2019-09-06T20:32:54Z

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

AmplabJenkins · 2019-09-06T23:05:59Z

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

AmplabJenkins · 2019-09-06T23:09:24Z

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

AmplabJenkins · 2019-09-07T07:54:35Z

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

AmplabJenkins · 2019-09-07T08:09:28Z

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

raulchen

Thanks. Looks good overall. We can merge this when some small comments are addressed.

raulchen · 2019-09-07T08:56:18Z

java/api/src/main/java/org/ray/api/options/ActorCreationOptions.java

  public static final int NO_RECONSTRUCTION = 0;
  public static final int INFINITE_RECONSTRUCTIONS = (int) Math.pow(2, 30);
+  public static final boolean DEFAULT_IS_DIRECT_CALL = "1"
+      .equals(System.getenv("ACTOR_CREATION_OPTIONS_DEFAULT_IS_DIRECT_CALL"));


document that this env var is only used for tests. Users should use setIsDirectCall.

raulchen · 2019-09-07T08:57:32Z

java/api/src/main/java/org/ray/api/options/ActorCreationOptions.java


+    // Since direct call is not fully supported yet, users are not allowed to set the option to true.
+    // TODO (kfstorm): uncomment when direct call is ready.
+//    public Builder setIsDirectCall(boolean isDirectCall) {


Can you add the issue number to the TODO comment?

raulchen · 2019-09-07T09:04:03Z

src/ray/core_worker/context.h


  const ActorID &GetCurrentActorID() const;

+  bool IsDirectCallActor() const;


Suggested change

bool IsDirectCallActor() const;

bool CurrentActorUseDirectCall() const;

raulchen · 2019-09-07T09:12:50Z

src/ray/raylet/actor_registration.cc

+    const auto actor_handle_id = task->GetTaskSpecification().ActorHandleId();
+    const auto dummy_object = task->GetTaskSpecification().ActorDummyObject();
+    // Extend its frontier to include the most recent task.
+    // Note(hchen): this is needed because this method is called before


Suggested change

// Note(hchen): this is needed because this method is called before

// NOTE(hchen): For non-direct-call actors, this is needed because this method is called before

raulchen · 2019-09-07T09:13:52Z

src/ray/raylet/actor_registration.cc

+    // Extend its frontier to include the most recent task.
+    // Note(hchen): this is needed because this method is called before
+    // `FinishAssignedTask`, which will be called when the worker tries to fetch
+    // the next task.


Suggested change

// the next task.

// the next task. For direct-call actors, checkpoint data doesn't contain frontier info, so we don't need to do `ExtendFrontier` here.

AmplabJenkins · 2019-09-07T12:57:24Z

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

…_call

AmplabJenkins · 2019-09-07T15:00:06Z

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

AmplabJenkins · 2019-09-07T18:15:45Z

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

AmplabJenkins · 2019-09-07T21:29:46Z

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

…_call

AmplabJenkins · 2019-09-08T10:51:23Z

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

kfstorm added 2 commits August 20, 2019 20:45

Support direct actor call in Java worker

3c78c68

Skip some tests

9530f16

kfstorm requested review from ericl, raulchen, robertnishihara and zhijunfu August 22, 2019 03:53

kfstorm changed the title ~~[WIP] Support direct actor call in Java worker~~ [WIP] [Java] Support direct actor call in Java worker Aug 22, 2019

Support pass large object by value to a direct call actor.

9fc1e24

kfstorm added 5 commits August 23, 2019 15:48

Merge remote-tracking branch 'origin/master' into java_actor_direct_call

05d5c9d

Support actor checkpointing for direct call

0c474b9

Merge remote-tracking branch 'origin/master' into java_actor_direct_call

5798637

Fix test case testActorProcessDying

7a5801b

Merge remote-tracking branch 'upstream/master' into java_actor_direct…

73bef52

…_call

Update ActorTest.java

748938d

Fix duplicated put in memory store provider

fe0ea89

kfstorm mentioned this pull request Aug 24, 2019

Support metadata for passing by value task arguments #5527

Merged

1 task

kfstorm added 2 commits August 26, 2019 20:45

Direct actor call subscribe to individual actor updates

82ca22b

Fix IsActorAlive

c0da338

Skip testActorProcessDying because it hangs

779c49c

Add todos for skipped tests.

a1600bc

kfstorm added 4 commits September 6, 2019 22:52

Merge remote-tracking branch 'linux/java_actor_direct_call' into java…

f4d3e65

…_actor_direct_call

Cleanup

a932190

Add is_direct_call to actor creation task and ActorTableData

f5570c7

Update comment

130e667

skip some tests not relevant to direct call

3586456

kfstorm force-pushed the java_actor_direct_call branch from 4f953f7 to 3586456 Compare September 6, 2019 19:40

kfstorm added 2 commits September 7, 2019 11:43

Use IAlterSuiteListener

534efdf

Minor fix about missing a test

9613575

raulchen approved these changes Sep 7, 2019

View reviewed changes

kfstorm added 3 commits September 7, 2019 17:19

Rename is_direct_call to use_direct_call in Java

e3d77b7

Address comments

ec33ab8

Fix

dc0a86f

Merge remote-tracking branch 'upstream/master' into java_actor_direct…

56428e2

…_call

Fix test

80e64ac

kfstorm added 2 commits September 8, 2019 12:18

Merge remote-tracking branch 'upstream/master' into java_actor_direct…

5114af9

…_call

Update ArgumentsBuilder

99837ac

zhijunfu approved these changes Sep 9, 2019

View reviewed changes

raulchen merged commit ed76190 into ray-project:master Sep 9, 2019

raulchen deleted the java_actor_direct_call branch September 9, 2019 06:59


		const ActorID &GetCurrentActorID() const;

		bool IsDirectCallActor() const;

	bool IsDirectCallActor() const;
	bool CurrentActorUseDirectCall() const;

	// Note(hchen): this is needed because this method is called before
	// NOTE(hchen): For non-direct-call actors, this is needed because this method is called before

	// the next task.
	// the next task. For direct-call actors, checkpoint data doesn't contain frontier info, so we don't need to do `ExtendFrontier` here.

Conversation

kfstorm commented Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

What do these changes do?

Related issue number

Linter

Uh oh!

AmplabJenkins commented Aug 22, 2019

Uh oh!

AmplabJenkins commented Aug 22, 2019

Uh oh!

AmplabJenkins commented Aug 23, 2019

Uh oh!

AmplabJenkins commented Aug 23, 2019

Uh oh!

AmplabJenkins commented Aug 23, 2019

Uh oh!

AmplabJenkins commented Aug 23, 2019

Uh oh!

AmplabJenkins commented Aug 24, 2019

Uh oh!

AmplabJenkins commented Aug 26, 2019

Uh oh!

AmplabJenkins commented Aug 26, 2019

Uh oh!

AmplabJenkins commented Aug 27, 2019

Uh oh!

AmplabJenkins commented Sep 6, 2019

Uh oh!

AmplabJenkins commented Sep 6, 2019

Uh oh!

AmplabJenkins commented Sep 6, 2019

Uh oh!

AmplabJenkins commented Sep 6, 2019

Uh oh!

AmplabJenkins commented Sep 7, 2019

Uh oh!

AmplabJenkins commented Sep 7, 2019

Uh oh!

raulchen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Sep 7, 2019

Uh oh!

AmplabJenkins commented Sep 7, 2019

Uh oh!

AmplabJenkins commented Sep 7, 2019

Uh oh!

AmplabJenkins commented Sep 7, 2019

Uh oh!

AmplabJenkins commented Sep 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kfstorm commented Aug 22, 2019 •

edited

Loading