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

[direct call] changes raylet to push tasks to worker #5140

Merged
merged 32 commits into from
Jul 11, 2019

Conversation

zhijunfu
Copy link
Contributor

@zhijunfu zhijunfu commented Jul 8, 2019

What do these changes do?

The previous change adds a grpc server to worker, this pr changes raylet to assign tasks instead of waiting for workers to get tasks.

Related issue number

#5029

Linter

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

@zhijunfu
Copy link
Contributor Author

zhijunfu commented Jul 8, 2019

@raulchen @jiangzihao2009

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

src/ray/core_worker/task_execution.cc Outdated Show resolved Hide resolved
src/ray/rpc/server_call.h Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.h Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
Copy link
Contributor Author

@zhijunfu zhijunfu left a comment

Choose a reason for hiding this comment

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

Thanks. Updated accordingly.

src/ray/core_worker/task_execution.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.h Show resolved Hide resolved
src/ray/rpc/server_call.h Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/core_worker/task_execution.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.h 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/15199/
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/15202/
Test PASSed.

@stephanie-wang stephanie-wang self-assigned this Jul 8, 2019
src/ray/core_worker/core_worker.cc Outdated Show resolved Hide resolved
src/ray/core_worker/mock_worker.cc Outdated Show resolved Hide resolved
src/ray/core_worker/core_worker.h Outdated Show resolved Hide resolved
src/ray/core_worker/task_execution.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.h 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/15222/
Test FAILed.

@raulchen
Copy link
Contributor

raulchen commented Jul 9, 2019

I just merged a PR. Can you resolve the conflicts?

@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/1545/
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/1546/
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, this is looking great! I left a couple comments that I would like addressed before we merge, but otherwise looks good.

src/ray/core_worker/transport/raylet_transport.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Show resolved Hide resolved
src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved
src/ray/raylet/node_manager.cc Show resolved Hide resolved
@zhijunfu
Copy link
Contributor Author

Thanks for reviewing! I think all comments are addressed.

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

@zhijunfu
Copy link
Contributor Author

travis, retest this please

@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/1570/
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/15260/
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/1575/
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/15265/
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/1582/
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/15272/
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-PRB/15307/
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/1616/
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/15312/
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/1621/
Test FAILed.

@stephanie-wang stephanie-wang merged commit 1649f13 into ray-project:master Jul 11, 2019
@stephanie-wang
Copy link
Contributor

I couldn't trigger a travis build for some reason...

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

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.

4 participants