-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Enhancement] adaptive enable the exchange compression #54956
[Enhancement] adaptive enable the exchange compression #54956
Conversation
937897d
to
bc15ba8
Compare
Signed-off-by: Murphy <[email protected]>
bc15ba8
to
4bd1e16
Compare
Signed-off-by: Murphy <[email protected]>
Signed-off-by: Murphy <[email protected]>
Quality Gate passedIssues Measures |
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 2 / 2 (100.00%) file detail
|
[BE Incremental Coverage Report]✅ pass : 40 / 44 (90.91%) file detail
|
@VariableMgr.VarAttr(name = TRANSMISSION_COMPRESSION_TYPE) | ||
private String transmissionCompressionType = "NO_COMPRESSION"; | ||
@VariableMgr.VarAttr(name = TRANSMISSION_COMPRESSION_TYPE) | ||
private String transmissionCompressionType = "AUTO"; |
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.
will it be compatibility issue if FE upgraded to the newer version while the BE still stays in old version, especially during the upgrade.
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.
yes. it's a session variable, so the behavior on BE is totally driven by FE
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.
that means during the upgrade, FE sends to BE with "AUTO" but BE doesn't recognize and fail the request? or BE just fallback to the no_compression behavior?
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.
the best practice is upgrading the BE first
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.
isn't wise to count on the "best practice" which doesn't exist on any doc.
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.
well, actually it's a common practice of StarRocks, otherwise it would be over-complicated to add some new stuff in the codebase. the common practice is to use a session variable to control the behavior, so before upgrading the FE this new feature will not be enabled.
@Mergifyio backport branch-3.4 |
✅ Backports have been created
|
Signed-off-by: Murphy <[email protected]> (cherry picked from commit b92f139)
Why I'm doing:
What I'm doing:
Change the default value of the variable
transmission_compression_type
fromNO_COMPRESSION
toAUTO
, which means:Rationale:
For workloads that involve transferring large amounts of data, such as shuffling, network bandwidth can become a bottleneck. In such cases, data compression can significantly improve performance.
While StarRocks already employs data encoding techniques to reduce data size, there are specific scenarios where additional compression can further optimize data transmission.
The effectiveness of compression depends on various factors, such as data type, data distribution and network utilization. Therefore, it is preferable to adopt an adaptive strategy rather than relying on a hard-coded approach. In this case, we utilize Thompson Sampling, a reward-based algorithm that dynamically adjusts decisions based on observed outcomes.
Evaluation on TPCDS:
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: