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

Prototype distributed actor handles #1137

Merged

Conversation

stephanie-wang
Copy link
Contributor

Allow a task that creates an actor handle to pass it to another task. The passed handle will get forked, so that tasks that get submitted after the fork will get executed after tasks that were submitted before the fork. Tasks will also be serialized per fork of the actor handle. The actor is garbage-collected when the original handle goes out of scope.

The local scheduler schedules tasks FIFO by submission order, regardless of the actor handle it was submitted by.

/** The index of the task assigned to this actor. Set to -1 if no task is
* currently assigned. If the actor process reports back success for the
* assigned task execution, task_counter should be set to this value. */
int64_t assigned_task_counter;
ActorID assigned_task_handle_id;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you document this field and explain what it's purpose is?

"""
return ActorHandleWrapper(
actor_handle._ray_actor_id,
random_actor_handle_id(), # Generate a new handle ID.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need a deterministic scheme for generating actor handle IDs?

@AmplabJenkins
Copy link

Merged build finished. 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/2157/
Test FAILed.

@AmplabJenkins
Copy link

Merged build finished. 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/2160/
Test FAILed.

@AmplabJenkins
Copy link

Merged build finished. 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/2158/
Test PASSed.

@AmplabJenkins
Copy link

Merged build finished. 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/2165/
Test FAILed.

@robertnishihara
Copy link
Collaborator

Just pushed a commit which should fix the jenkins test. Basically, the PR changed the behavior so that now actor classes are exported to Redis as soon as the actor class is defined (before this happened as soon as the first actor object was instantiated). The problem is that we can only export to Redis as soon as ray.init() has been called, so if the actor class is defined before ray.init() is called, then we need to cache it and export it once ray.init() is called.

@AmplabJenkins
Copy link

Merged build finished. 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/2168/
Test FAILed.

@AmplabJenkins
Copy link

Merged build finished. 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/2171/
Test PASSed.

@AmplabJenkins
Copy link

Merged build finished. 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/2175/
Test PASSed.

@AmplabJenkins
Copy link

Merged build finished. 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/2177/
Test PASSed.

@AmplabJenkins
Copy link

Merged build finished. 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/2178/
Test PASSed.

@AmplabJenkins
Copy link

Merged build finished. 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/2179/
Test PASSed.

@robertnishihara
Copy link
Collaborator

Ok, I was going to try to disable defining actor definitions on workers, but I can't currently because of #1146 (doing so breaks hyperparameter search).

@AmplabJenkins
Copy link

Merged build finished. 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/2180/
Test PASSed.

@robertnishihara robertnishihara merged commit af47737 into ray-project:master Oct 20, 2017
@robertnishihara robertnishihara deleted the multiple-actor-handles branch October 20, 2017 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants