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.

Sign up for GitHub

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

Jump to bottom

Remove storage service from supervisor #3254

Merged
merged 4 commits into from
Sep 14, 2022
Merged

Remove storage service from supervisor #3254

merged 4 commits into from
Sep 14, 2022

Conversation

vcfgv
Copy link
Contributor

@vcfgv vcfgv commented Sep 13, 2022

What do these changes do?

  1. Remove storage service from supervisor. Now storage service on supervisor will remain empty.
  2. Under mars backend, large UDF will be serialized and saved as op.func while tiling and will be deserialized while executing.
  3. Under ray backend (oscar and dag), large UDF will be put into ObjectStore while tiling and brought back while executing.

Related issue number

#3193

Check code requirements

  • tests added / passed (if needed)
  • Ensure all linting tests pass, see here for how to run them

@vcfgv vcfgv changed the title Remove storage service from supervisor and corresponding refinements on operand closure cleanup Remove storage service from supervisor Sep 13, 2022
@vcfgv vcfgv force-pushed the feature/refine_closure_cleanup branch from 03128cf to 551f020 Compare September 13, 2022 09:47
@vcfgv vcfgv mentioned this pull request Sep 14, 2022
Merged
2 tasks
hekaisheng
hekaisheng reviewed Sep 14, 2022
View reviewed changes
Copy link
Contributor

@hekaisheng hekaisheng left a comment

Choose a reason for hiding this comment

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

I left two comments, this PR is acceptable overall for now, looking forward the refactor in following PRs.

mars/dataframe/utils.py Outdated Show resolved Hide resolved
mars/dataframe/utils.py Show resolved Hide resolved
chaokunyang
chaokunyang previously approved these changes Sep 14, 2022
View reviewed changes
Copy link
Contributor

@chaokunyang chaokunyang left a comment

Choose a reason for hiding this comment

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

LGTM

@vcfgv vcfgv marked this pull request as ready for review September 14, 2022 03:48
@vcfgv vcfgv requested a review from a team as a code owner September 14, 2022 03:48
@vcfgv vcfgv force-pushed the feature/refine_closure_cleanup branch from 3c92a4b to 25e136d Compare September 14, 2022 05:20
aresnow1
aresnow1 approved these changes Sep 14, 2022 •
edited
Loading
View reviewed changes
hekaisheng
hekaisheng approved these changes Sep 14, 2022
View reviewed changes
Copy link
Contributor

@hekaisheng hekaisheng left a comment

Choose a reason for hiding this comment

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

LGTM

chaokunyang
chaokunyang approved these changes Sep 14, 2022
View reviewed changes
Copy link
Contributor

@chaokunyang chaokunyang left a comment

Choose a reason for hiding this comment

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

LGTM

@hekaisheng hekaisheng merged commit 71b1035 into mars-project:master Sep 14, 2022
@hekaisheng hekaisheng mentioned this pull request Sep 14, 2022
Merged
2 tasks
qianduoduo0904 pushed a commit to qianduoduo0904/mars that referenced this pull request Sep 22, 2022
@vcfgv @qianduoduo0904
Remove storage service from supervisor (mars-project#3254)
af4e24a 
(cherry picked from commit 71b1035)
UranusSeven pushed a commit to xorbitsai/mars that referenced this pull request Sep 22, 2022
@vcfgv @UranusSeven
Remove storage service from supervisor (mars-project#3254)
5dd726e 
(cherry picked from commit 71b1035)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chaokunyang chaokunyang chaokunyang approved these changes

@hekaisheng hekaisheng hekaisheng approved these changes

@aresnow1 aresnow1 aresnow1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vcfgv @chaokunyang @hekaisheng @aresnow1