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] Allow split huge data ShuffleDataFlushEvent to multiple small events #398

Open
3 tasks done
zuston opened this issue Dec 12, 2022 · 13 comments
Open
3 tasks done

Comments

@zuston
Copy link
Member

zuston commented Dec 12, 2022

Code of Conduct

Search before asking

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

What would you like to be improved?

[Improvement] Allow split huge data ShuffleDataFlushEvent to multiple small events.

As the problem mentioned by #378 (comment), the small event will benefit more after the PR of #396 is applied.

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 Dec 12, 2022

cc @jerqi

@jerqi
Copy link
Contributor

jerqi commented Dec 12, 2022

Could we avoid huge event by using this pr #176 ?

@zuston
Copy link
Member Author

zuston commented Dec 12, 2022

Could we avoid huge event by using this pr #176 ?

Yes. But I don't hope so, detailed reason could be found in #378 (comment) of second case.

@jerqi
Copy link
Contributor

jerqi commented Dec 12, 2022

Could we avoid huge event by using this pr #176 ?

Yes. But I don't hope so, detailed reason could be found in #378 (comment) of second case.

I can't get your point.

@zuston
Copy link
Member Author

zuston commented Dec 12, 2022

If the single buffer flush mechanism is enabled, it will be applied all partition data. But when the memory is free and the concurrent apps number is small, there is unnecessary to flush data to localfile or disk. In my view, it will make the local disk free if the single buffer flush size > cold storage threshold size. And it will make memory free if the single buffer flush size < cold storage threshold size.

Overall, this mechanism will not make full use of memory and then leads to performance regression.

@jerqi
Copy link
Contributor

jerqi commented Dec 13, 2022

We find that it won't matter. Because if we write data to storage, and then we read the data quickly, the data may be in the page cache. It has similar performance with the data in memory. Single buffer mechnism can make full use of network card.

@zuston
Copy link
Member Author

zuston commented Dec 14, 2022

Because if we write data to storage, and then we read the data quickly, the data may be in the page cache.

But in the most cases, the read wont happen quickly after writing, especially for many tasks.

@jerqi
Copy link
Contributor

jerqi commented Dec 14, 2022

Because if we write data to storage, and then we read the data quickly, the data may be in the page cache.

But in the most cases, the read wont happen quickly after writing, especially for many tasks.

Maybe we should some tests prove the effect.

@zuston
Copy link
Member Author

zuston commented Dec 14, 2022

I have tested in our online cluster #378 (comment).

@jerqi
Copy link
Contributor

jerqi commented Dec 14, 2022

I have tested in our online cluster #378 (comment).

The test don't have the pr d3aa5dc

@zuston
Copy link
Member Author

zuston commented Dec 14, 2022

It's still useless after #396. Single huge event only can be handled by single thread to flush to HDFS, which don't benefit from concurrent written mechanism.

@jerqi
Copy link
Contributor

jerqi commented Dec 14, 2022

It's still useless after #396. Single huge event only can be handled by single thread to flush to HDFS, which don't benefit from concurrent written mechanism.

If you use single buffer limit at the same time, you will benefit the feature.

@zuston
Copy link
Member Author

zuston commented Dec 15, 2022

If you use single buffer limit at the same time, you will benefit the feature.

I know this, but I don't hope all large buffer flush to persist storage, especially for having enough free memory.

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

No branches or pull requests

2 participants