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

Remove dependencies from TaskExecutionSpecification #5166

Merged
merged 13 commits into from
Jul 16, 2019

Conversation

edoakes
Copy link
Contributor

@edoakes edoakes commented Jul 10, 2019

What do these changes do?

Removes the dependencies field from TaskExecutionSpecification in favor of a new PreviousActorTaskDummyObjectId field in TaskSpecification.

This was an artifact left over from a previous design, and the dependencies were never actually updated during runtime. Rather, it was only used to specify dependencies for actor tasks on the previously run actor task. Now, there is a dedicated field for this use case that is only specified as part of actor task submissions.

The change has a nice byproduct of simplifying the task submission interfaces in the core worker.

Related issue number

Linter

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

@edoakes edoakes changed the title [wip] Remove dependencies from TaskExecutionSpecification Remove dependencies from TaskExecutionSpecification Jul 10, 2019
@stephanie-wang stephanie-wang self-assigned this Jul 11, 2019
@edoakes
Copy link
Contributor Author

edoakes commented Jul 11, 2019

Note that this touches many of the same files as #5147

@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-Perf-Integration-PRB/1599/
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/15289/
Test PASSed.

@edoakes edoakes force-pushed the actor-task-dep branch 2 times, most recently from 7a0803d to 559f42f Compare July 11, 2019 16:15
@edoakes
Copy link
Contributor Author

edoakes commented Jul 11, 2019

Updated to rebase off of the changes merged in #5147

Copy link
Contributor

@stephanie-wang stephanie-wang left a comment

Choose a reason for hiding this comment

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

A few tiny things, but looks great!

src/ray/core_worker/context.cc Outdated Show resolved Hide resolved
src/ray/core_worker/task_interface.cc Outdated Show resolved Hide resolved
python/ray/includes/task.pxi 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-Perf-Integration-PRB/1622/
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-Perf-Integration-PRB/1623/
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/15313/
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/15314/
Test PASSed.

@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-Perf-Integration-PRB/1627/
Test FAILed.

Copy link
Contributor

@stephanie-wang stephanie-wang left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

Please try to avoid force-pushing in the future, as it makes it hard to see what the new changes are.

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

@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-Perf-Integration-PRB/1633/
Test FAILed.

src/ray/protobuf/common.proto 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/15346/
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/15353/
Test PASSed.

@pcmoritz
Copy link
Contributor

This is almost ready, there is another Travis error:

Use --sandbox_debug to see verbose messages from the sandbox
src/ray/raylet/lib/java/org_ray_runtime_raylet_RayletClientImpl.cc: In function 'void Java_org_ray_runtime_raylet_RayletClientImpl_nativeSubmitTask(JNIEnv*, jclass, jlong, jbyteArray, jbyteArray)':
src/ray/raylet/lib/java/org_ray_runtime_raylet_RayletClientImpl.cc:78:76: error: no matching function for call to 'RayletClient::SubmitTask(std::vector<ray::ObjectID>&, ray::TaskSpecification&)'
   auto status = raylet_client->SubmitTask(execution_dependencies, task_spec);
                                                                            ^
In file included from src/ray/raylet/lib/java/org_ray_runtime_raylet_RayletClientImpl.cc:6:0:
src/ray/raylet/raylet_client.h:85:15: note: candidate: ray::Status RayletClient::SubmitTask(const ray::TaskSpecification&)
   ray::Status SubmitTask(const ray::TaskSpecification &task_spec);
               ^
src/ray/raylet/raylet_client.h:85:15: note:   candidate expects 1 argument, 2 provided
INFO: Elapsed time: 327.299s, Critical Path: 26.18s
INFO: 2021 processes: 1947 remote cache hit, 73 linux-sandbox, 1 worker.
FAILED: Build did NOT complete successfully
FAILED: Build did NOT complete successfully

@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-Perf-Integration-PRB/1654/
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-Perf-Integration-PRB/1657/
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/15377/
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/15388/
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/15390/
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/15393/
Test PASSed.

@pcmoritz pcmoritz merged commit e5be5fd into ray-project:master Jul 16, 2019
@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-Perf-Integration-PRB/1681/
Test FAILed.

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.

5 participants