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

Let driver own pcollections #41

Merged
merged 5 commits into from
Sep 20, 2022
Merged

Let driver own pcollections #41

merged 5 commits into from
Sep 20, 2022

Conversation

jjyao
Copy link
Contributor

@jjyao jjyao commented Sep 7, 2022

Signed-off-by: Jiajun Yao jeromeyjj@gmail.com

With this PR, driver will own all the data of pcollections and handle the object lose automatically.

Closes #34

Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
@jjyao jjyao requested review from pabloem, iasoon and pdames September 7, 2022 15:57
Copy link
Member

@iasoon iasoon left a comment

Choose a reason for hiding this comment

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

Very nice @jjyao!

As the reason for this packing/unpacking of the arguments is a bit non-obvious in my opinion, I'm wondering what the best way would be to make these kinds of design decisions discoverable to readers of the code. Any thoughts? (Certainly not blocking to merge this)

ray_beam_runner/portability/execution.py Outdated Show resolved Hide resolved
Signed-off-by: Pablo <pabloem@users.noreply.github.com>
Signed-off-by: Pablo <pabloem@users.noreply.github.com>
Signed-off-by: Pablo <pabloem@users.noreply.github.com>
@pabloem
Copy link
Collaborator

pabloem commented Sep 20, 2022

I'll merge after tests pass

Signed-off-by: Pablo <pabloem@users.noreply.github.com>
@pabloem pabloem merged commit d2d11ce into master Sep 20, 2022
@pabloem
Copy link
Collaborator

pabloem commented Sep 20, 2022

thanks everyone : )

@jjyao jjyao deleted the jjyao/ownership branch September 21, 2022 16:05
@jjyao
Copy link
Contributor Author

jjyao commented Sep 21, 2022

@iasoon, sorry for the late reply.

I'll apply ray-project/ray#28291 to this PR so we no longer need to calculate num_returns beforehand.

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.

Refactor how we manage pcollections
3 participants