-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-24899][runtime] Enable data compression for blocking shuffle by default #17814
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
[FLINK-24899][runtime] Enable data compression for blocking shuffle by default #17814
Conversation
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit e3269ea (Wed Nov 17 03:00:01 UTC 2021) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. DetailsThe Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
|
Thanks for preparing this PR, I will take a look soon. |
hililiwei
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some comments. :-)
flink-core/src/main/java/org/apache/flink/configuration/NettyShuffleEnvironmentOptions.java
Show resolved
Hide resolved
|
@hililiwei, thanks for your detailed review and I have addressed the above comment. IMO, the boolean type option has no the definition in NettyShuffleEnvironmentOptions and thus I have not added this definition. |
|
Hi, thanks a lot for the PR and review. I will take look after finishing this discussion: https://lists.apache.org/thread/pt2b1f17x2l5rlvggwxs6m265lo4ly7p. |
|
@SteNicholas Thanks for your contribution, I have left a small comment. |
flink-tests/src/test/java/org/apache/flink/test/runtime/ShuffleCompressionITCase.java
Outdated
Show resolved
Hide resolved
|
The description for taskmanager.network.sort-shuffle.min-parallelism need also to be updated. |
@wsry , I would like to update the description for taskmanager.network.sort-shuffle.min-parallelism. |
|
@SteNicholas Thanks for the update, changes LGTM. I will help to merge it after the CI passes. |
…y default This closes apache#17814.
What is the purpose of the change
Shuffle data compression is currently not enabled by default. Shuffle data compression is important for blocking data shuffle and enabling shuffle data compression by default can improve the usability.
taskmanager.network.blocking-shuffle.compression.enabledshould be true by default.Brief change log
taskmanager.network.blocking-shuffle.compression.enabledto true.Verifying this change
testSortMergeShuffleConfigOptionsCorrelationofNettyShuffleEnvironmentConfigurationTestadds the test to verify whether the defaule value oftaskmanager.network.blocking-shuffle.compression.enabledis true.Does this pull request potentially affect one of the following parts:
@Public(Evolving): (yes / no)Documentation