Skip to content

Try improve performance of aggregation in order of sorting key#19401

Merged
akuzm merged 7 commits intoClickHouse:masterfrom
CurtizJ:aggregating-in-order
Mar 9, 2021
Merged

Try improve performance of aggregation in order of sorting key#19401
akuzm merged 7 commits intoClickHouse:masterfrom
CurtizJ:aggregating-in-order

Conversation

@CurtizJ
Copy link
Copy Markdown
Member

@CurtizJ CurtizJ commented Jan 22, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Improve performance of aggregation in order of sorting key (with enabled setting optimize_aggregation_in_order)

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Jan 22, 2021
@CurtizJ CurtizJ force-pushed the aggregating-in-order branch from fe8f9b0 to 666aab6 Compare January 26, 2021 18:45
@CurtizJ CurtizJ marked this pull request as ready for review January 26, 2021 18:45
@azat azat mentioned this pull request Jan 26, 2021
Open
@CurtizJ
Copy link
Copy Markdown
Member Author

CurtizJ commented Jan 27, 2021 •
edited
Loading

Fuzzer report is related to changes.

@alexey-milovidov alexey-milovidov mentioned this pull request Jan 31, 2021
Closed
@akuzm
Copy link
Copy Markdown
Contributor

akuzm commented Mar 5, 2021

The performance improvement in read_in_order_many_parts is nice. For aggregation_in_order, the improvement is only noticeable for the query with GROUP BY CounterID, EventDate, intHash32(UserID), and not for other queries. Is this expected?

https://clickhouse-test-reports.s3.yandex.net/19401/2e8b45afc17d4bce41f6d5517a49842a15d69720/performance_comparison/all-queries.html#all-query-times.aggregation_in_order.0

@akuzm akuzm self-assigned this Mar 5, 2021
@CurtizJ
Copy link
Copy Markdown
Member Author

CurtizJ commented Mar 5, 2021

But in the test read_in_order_many_parts query SELECT sum(val2) FROM mt_100_parts GROUP BY val1 FORMAT Null has speeded up. For aggregation_in_order it's expected, that there is almost no improvement, because hits table contains only one or few parts and merge stage takes non-significant amount of time.

@akuzm
Copy link
Copy Markdown
Contributor

akuzm commented Mar 9, 2021

Integration and perf test failures are unrelated.

@akuzm akuzm merged commit 0de89e2 into ClickHouse:master Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@akuzm akuzm akuzm left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@akuzm akuzm

Labels

pr-improvement Pull request with some product improvements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@CurtizJ @akuzm @robot-clickhouse