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

[Improvement] Fine-grained memory control for spark client #706

Closed
3 tasks done
zuston opened this issue Mar 10, 2023 · 4 comments · Fixed by #714
Closed
3 tasks done

[Improvement] Fine-grained memory control for spark client #706

zuston opened this issue Mar 10, 2023 · 4 comments · Fixed by #714

Comments

@zuston
Copy link
Member

zuston commented Mar 10, 2023

Code of Conduct

Search before asking

  • I have searched in the issues and found no similar issues.

What would you like to be improved?

  1. [#706] Implement spill method to avoid memory deadlock #714
  2. Introduce dedicated memoryConsumer when fetching remote data, especially for after supporting async read. [FEATURE] Async Reader #291
  3. ...

How should we improve?

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
@zuston
Copy link
Member Author

zuston commented Mar 10, 2023

And I also find that some apps in our production failed due to requiring memory failed, but it works well with ESS, although the available memory is less in ESS.

@zuston
Copy link
Member Author

zuston commented Mar 10, 2023

cc @jerqi @advancedxy

@jerqi
Copy link
Contributor

jerqi commented Mar 10, 2023

Ok for me.

@zuston
Copy link
Member Author

zuston commented Mar 10, 2023

And I also find that some apps in our production failed due to requiring memory failed, but it works well with ESS, although the available memory is less in ESS.

And the available memory is tight, could we support append record to temp files and at the final step, send them to remote shuffle-server, rather than using memory to hold them.

zuston added a commit to zuston/incubator-uniffle that referenced this issue Mar 13, 2023
zuston added a commit to zuston/incubator-uniffle that referenced this issue Mar 16, 2023
zuston added a commit to zuston/incubator-uniffle that referenced this issue Mar 20, 2023
zuston added a commit to zuston/incubator-uniffle that referenced this issue Mar 28, 2023
zuston added a commit to zuston/incubator-uniffle that referenced this issue Mar 28, 2023
zuston added a commit that referenced this issue Mar 28, 2023
### What changes were proposed in this pull request?
1.  Introduce the `DataPusher` to replace the `eventLoop`, this could be as general part for spark2 and spark3.
2. Implement the `spill` method in `WriterBufferManager` to avoid memory deadlock.

### Why are the changes needed?
In current codebase, if having several `WriterBufferManagers`, when each other is acquiring memory, the deadlock will happen. To solve this, we should implement spill function to break this deadlock condition.

Fix: #706 

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
1. Existing UTs
2. Newly added UTs
xianjingfeng pushed a commit to xianjingfeng/incubator-uniffle that referenced this issue Apr 5, 2023
…che#714)

### What changes were proposed in this pull request?
1.  Introduce the `DataPusher` to replace the `eventLoop`, this could be as general part for spark2 and spark3.
2. Implement the `spill` method in `WriterBufferManager` to avoid memory deadlock.

### Why are the changes needed?
In current codebase, if having several `WriterBufferManagers`, when each other is acquiring memory, the deadlock will happen. To solve this, we should implement spill function to break this deadlock condition.

Fix: apache#706 

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
1. Existing UTs
2. Newly added UTs
jerqi added a commit to jerqi/incubator-uniffle that referenced this issue Apr 11, 2023
jerqi added a commit to jerqi/incubator-uniffle that referenced this issue Apr 11, 2023
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 a pull request may close this issue.

2 participants