[improvement](partial-update) make partial-update on agg table use less memory #32200
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for your contribution to Apache Doris. |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
TeamCity be ut coverage result: |
hust-hhb
changed the title
morrySnow
reviewed
Mar 14, 2024
| @@ -1486,6 +1488,9 @@ public void setEnableLeftZigZag(boolean enableLeftZigZag) { | |||
| @VariableMgr.VarAttr(name = ENABLE_UNIQUE_KEY_PARTIAL_UPDATE, needForward = true) | |||
| public boolean enableUniqueKeyPartialUpdate = false; | |||
|
|
|||
| @VariableMgr.VarAttr(name = ENABLE_AGG_KEY_PARTIAL_UPDATE, needForward = true) | |||
| public boolean enableAggregateKeyPartialUpdate = false; | |||
There was a problem hiding this comment.
what's different between enableUniqueKeyPartialUpdate and enableAggregateKeyPartialUpdate? why not use one variable to control them?
In LogicalPlanBuilder and BindSink, we cannot distinguish them.
There was a problem hiding this comment.
For users, this is a new feature that should be controlled by a separate variable. If only use one variable to control them, may be the variable ENABLE_UNIQUE_KEY_PARTIAL_UPDATE should be change to ENABLE_PARTIAL_UPDATE, our documentation needs to be updated accordingly, and it may cause compatibility issues for users as their codes also need to be changed accordingly.
|
clang-tidy review says "All clean, LGTM! 👍" |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
TeamCity be ut coverage result: |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H: Total hot run time: 37909 ms |
TPC-DS: Total hot run time: 181604 ms |
|
Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
TeamCity be ut coverage result: |
hust-hhb
changed the title
|
clang-tidy review says "All clean, LGTM! 👍" |
zhannngchen
reviewed
Mar 26, 2024
zhannngchen
reviewed
Mar 26, 2024
zhannngchen
reviewed
Mar 26, 2024
zhannngchen
requested changes
Mar 26, 2024
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@zhannngchen update the patch, take a look please |
TPC-H: Total hot run time: 37847 ms |
TPC-DS: Total hot run time: 182329 ms |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 38826 ms |
TPC-DS: Total hot run time: 183192 ms |
ClickBench: Total hot run time: 30.42 s |
|
Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 38566 ms |
TPC-DS: Total hot run time: 181770 ms |
ClickBench: Total hot run time: 29.27 s |
|
Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
zhannngchen
reviewed
Apr 8, 2024
...ion-test/suites/unique_with_mow_p0/partial_update/test_agg_partial_update_insert_into.groovy
Show resolved
Hide resolved
|
clang-tidy review says "All clean, LGTM! 👍" |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
TeamCity be ut coverage result: |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H: Total hot run time: 40477 ms |
TPC-DS: Total hot run time: 172832 ms |
ClickBench: Total hot run time: 30.9 s |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
This pr make fe only send the specific column to be when using partial_columns= true header in streamload or set enable_agg_key_partial_update=true when using insert into on agg table, which can use less memory, in my test, it can reduce about 60% memory when insert 1000 columns table to 5000 columns table.
Issue Number: close #xxx
Further comments
If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...