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

Limit GroupByCombineOperator to use 2 * numCores threads instead of creating one task per operator #14843

Merged
merged 2 commits into from
Jan 21, 2025

Conversation

yashmayya
Copy link
Collaborator

  • For most leaf stage combine operators (in SSQE), we calculate the number of tasks spawned as Math.min(numSegments, maxExecutionThreads) where maxExecutionThreads is calculated as Math.max(1, Math.min(10, numCores / 2)) (unless it is explicitly specified in the query options using maxExecutionThreads).
  • Each of these tasks processes a subset of the segments. However, for the GroupByCombineOperator specifically, this maxExecutionThreads is currently overridden to be equal to the number of segments being processed (see here by default for some reason.
  • This is not so problematic for the single-stage query engine where these tasks are submitted to a fixed thread pool executor service with the number of threads being 2 * numCores by default (this one).
  • In fact, some of the combine operators even have comments stating that while the numTasks are the number of async tasks submitted to the executor service, the actual number of threads used by the server would be limited to the fixed number of threads available in the executor service.
  • However, in the multi-stage engine's leaf stages, the executor service used here is the same one from QueryRunner - i.e., this cached thread pool executor that can spawn infinite threads. This means that for leaf stage operators that involve a group by, the number of threads spawned for a single multi-stage query on each server can be equal to the number of segments being processed on that server (could be thousands of threads which is super problematic).
  • This patch updates the override to cap the number of tasks spawned by the GroupByCombineOperator by default to be the default number of query worker threads (which is 2 * numCores). This shouldn't impact SSQE performance while also avoiding the creation of a huge number of threads in MSQE.
  • Note that this is only a bandaid solution and we need to more carefully think through all the implications of the threading model for the MSQE (cc @gortiz who has some interesting ideas here).

@yashmayya yashmayya added query performance multi-stage Related to the multi-stage query engine labels Jan 20, 2025
@yashmayya yashmayya marked this pull request as ready for review January 20, 2025 04:20
@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.75%. Comparing base (59551e4) to head (3dbe014).
Report is 1598 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #14843      +/-   ##
============================================
+ Coverage     61.75%   63.75%   +2.00%     
- Complexity      207     1612    +1405     
============================================
  Files          2436     2708     +272     
  Lines        133233   151293   +18060     
  Branches      20636    23357    +2721     
============================================
+ Hits          82274    96464   +14190     
- Misses        44911    47594    +2683     
- Partials       6048     7235    +1187     
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 63.73% <100.00%> (+2.02%) ⬆️
java-21 63.63% <100.00%> (+2.01%) ⬆️
skip-bytebuffers-false 63.75% <100.00%> (+2.01%) ⬆️
skip-bytebuffers-true 63.61% <100.00%> (+35.89%) ⬆️
temurin 63.75% <100.00%> (+2.00%) ⬆️
unittests 63.75% <100.00%> (+2.00%) ⬆️
unittests1 56.32% <100.00%> (+9.43%) ⬆️
unittests2 34.04% <0.00%> (+6.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gortiz
Copy link
Contributor

gortiz commented Jan 20, 2025

This shouldn't impact SSQE performance while ...

Can we verify that empirically with a test?

@yashmayya
Copy link
Collaborator Author

yashmayya commented Jan 21, 2025

Can we verify that empirically with a test?

I updated the existing BenchmarkQueries to also support a larger number of segments and here's the results for the group by queries in that benchmark.


Old (without changes from this PR)

Benchmark               (_numRows)  (_numSegments)                                                                                                                                                                                          (_query)  (_scenario)  Mode  Cnt     Score    Error  Units
BenchmarkQueries.query     1500000               1                                                                                                                     SELECT RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5    36.764 ±  1.189  ms/op
BenchmarkQueries.query     1500000               1                                                                                       SELECT RAW_STRING_COL,RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_STRING_COL,RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5   134.669 ±  2.014  ms/op
BenchmarkQueries.query     1500000               1                                                           SELECT NO_INDEX_STRING_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY NO_INDEX_STRING_COL,INT_COL ORDER BY NO_INDEX_STRING_COL, INT_COL ASC     EXP(0.5)  avgt    5     5.147 ±  0.334  ms/op
BenchmarkQueries.query     1500000               1  SELECT NO_INDEX_STRING_COL,LOW_CARDINALITY_STRING_COL,COUNT(*) FROM MyTable GROUP BY LOW_CARDINALITY_STRING_COL,NO_INDEX_STRING_COL ORDER BY LOW_CARDINALITY_STRING_COL, NO_INDEX_STRING_COL ASC     EXP(0.5)  avgt    5     3.568 ±  0.084  ms/op
BenchmarkQueries.query     1500000               1                                                                                                               select count(*), year(INT_COL) as y, month(INT_COL) as m from MyTable group by y, m     EXP(0.5)  avgt    5    35.912 ±  1.409  ms/op
BenchmarkQueries.query     1500000               2                                                                                                                     SELECT RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5    39.488 ±  0.565  ms/op
BenchmarkQueries.query     1500000               2                                                                                       SELECT RAW_STRING_COL,RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_STRING_COL,RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5   157.133 ±  3.754  ms/op
BenchmarkQueries.query     1500000               2                                                           SELECT NO_INDEX_STRING_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY NO_INDEX_STRING_COL,INT_COL ORDER BY NO_INDEX_STRING_COL, INT_COL ASC     EXP(0.5)  avgt    5     5.430 ±  0.158  ms/op
BenchmarkQueries.query     1500000               2  SELECT NO_INDEX_STRING_COL,LOW_CARDINALITY_STRING_COL,COUNT(*) FROM MyTable GROUP BY LOW_CARDINALITY_STRING_COL,NO_INDEX_STRING_COL ORDER BY LOW_CARDINALITY_STRING_COL, NO_INDEX_STRING_COL ASC     EXP(0.5)  avgt    5     3.731 ±  0.163  ms/op
BenchmarkQueries.query     1500000               2                                                                                                               select count(*), year(INT_COL) as y, month(INT_COL) as m from MyTable group by y, m     EXP(0.5)  avgt    5    36.784 ±  1.647  ms/op
BenchmarkQueries.query     1500000              10                                                                                                                     SELECT RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5   166.385 ± 10.191  ms/op
BenchmarkQueries.query     1500000              10                                                                                       SELECT RAW_STRING_COL,RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_STRING_COL,RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5   735.311 ± 20.755  ms/op
BenchmarkQueries.query     1500000              10                                                           SELECT NO_INDEX_STRING_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY NO_INDEX_STRING_COL,INT_COL ORDER BY NO_INDEX_STRING_COL, INT_COL ASC     EXP(0.5)  avgt    5    25.390 ±  0.409  ms/op
BenchmarkQueries.query     1500000              10  SELECT NO_INDEX_STRING_COL,LOW_CARDINALITY_STRING_COL,COUNT(*) FROM MyTable GROUP BY LOW_CARDINALITY_STRING_COL,NO_INDEX_STRING_COL ORDER BY LOW_CARDINALITY_STRING_COL, NO_INDEX_STRING_COL ASC     EXP(0.5)  avgt    5    16.623 ±  0.327  ms/op
BenchmarkQueries.query     1500000              10                                                                                                               select count(*), year(INT_COL) as y, month(INT_COL) as m from MyTable group by y, m     EXP(0.5)  avgt    5   175.212 ±  3.031  ms/op
BenchmarkQueries.query     1500000              50                                                                                                                     SELECT RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5   961.633 ± 17.295  ms/op
BenchmarkQueries.query     1500000              50                                                                                       SELECT RAW_STRING_COL,RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_STRING_COL,RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5  3640.062 ± 21.391  ms/op
BenchmarkQueries.query     1500000              50                                                           SELECT NO_INDEX_STRING_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY NO_INDEX_STRING_COL,INT_COL ORDER BY NO_INDEX_STRING_COL, INT_COL ASC     EXP(0.5)  avgt    5   123.276 ±  1.000  ms/op
BenchmarkQueries.query     1500000              50  SELECT NO_INDEX_STRING_COL,LOW_CARDINALITY_STRING_COL,COUNT(*) FROM MyTable GROUP BY LOW_CARDINALITY_STRING_COL,NO_INDEX_STRING_COL ORDER BY LOW_CARDINALITY_STRING_COL, NO_INDEX_STRING_COL ASC     EXP(0.5)  avgt    5    81.729 ±  0.784  ms/op
BenchmarkQueries.query     1500000              50                                                                                                               select count(*), year(INT_COL) as y, month(INT_COL) as m from MyTable group by y, m     EXP(0.5)  avgt    5   862.859 ± 49.178  ms/op

New (with changes from this PR)

Benchmark               (_numRows)  (_numSegments)                                                                                                                                                                                          (_query)  (_scenario)  Mode  Cnt     Score    Error  Units
BenchmarkQueries.query     1500000               1                                                                                                                     SELECT RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5    37.356 ±  0.371  ms/op
BenchmarkQueries.query     1500000               1                                                                                       SELECT RAW_STRING_COL,RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_STRING_COL,RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5   133.085 ±  1.483  ms/op
BenchmarkQueries.query     1500000               1                                                           SELECT NO_INDEX_STRING_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY NO_INDEX_STRING_COL,INT_COL ORDER BY NO_INDEX_STRING_COL, INT_COL ASC     EXP(0.5)  avgt    5     5.282 ±  0.309  ms/op
BenchmarkQueries.query     1500000               1  SELECT NO_INDEX_STRING_COL,LOW_CARDINALITY_STRING_COL,COUNT(*) FROM MyTable GROUP BY LOW_CARDINALITY_STRING_COL,NO_INDEX_STRING_COL ORDER BY LOW_CARDINALITY_STRING_COL, NO_INDEX_STRING_COL ASC     EXP(0.5)  avgt    5     3.648 ±  0.147  ms/op
BenchmarkQueries.query     1500000               1                                                                                                               select count(*), year(INT_COL) as y, month(INT_COL) as m from MyTable group by y, m     EXP(0.5)  avgt    5    34.617 ±  0.604  ms/op
BenchmarkQueries.query     1500000               2                                                                                                                     SELECT RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5    37.743 ±  0.558  ms/op
BenchmarkQueries.query     1500000               2                                                                                       SELECT RAW_STRING_COL,RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_STRING_COL,RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5   157.974 ±  1.498  ms/op
BenchmarkQueries.query     1500000               2                                                           SELECT NO_INDEX_STRING_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY NO_INDEX_STRING_COL,INT_COL ORDER BY NO_INDEX_STRING_COL, INT_COL ASC     EXP(0.5)  avgt    5     5.401 ±  0.134  ms/op
BenchmarkQueries.query     1500000               2  SELECT NO_INDEX_STRING_COL,LOW_CARDINALITY_STRING_COL,COUNT(*) FROM MyTable GROUP BY LOW_CARDINALITY_STRING_COL,NO_INDEX_STRING_COL ORDER BY LOW_CARDINALITY_STRING_COL, NO_INDEX_STRING_COL ASC     EXP(0.5)  avgt    5     3.633 ±  0.145  ms/op
BenchmarkQueries.query     1500000               2                                                                                                               select count(*), year(INT_COL) as y, month(INT_COL) as m from MyTable group by y, m     EXP(0.5)  avgt    5    34.755 ±  1.062  ms/op
BenchmarkQueries.query     1500000              10                                                                                                                     SELECT RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5   191.722 ±  4.703  ms/op
BenchmarkQueries.query     1500000              10                                                                                       SELECT RAW_STRING_COL,RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_STRING_COL,RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5   762.582 ±  5.516  ms/op
BenchmarkQueries.query     1500000              10                                                           SELECT NO_INDEX_STRING_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY NO_INDEX_STRING_COL,INT_COL ORDER BY NO_INDEX_STRING_COL, INT_COL ASC     EXP(0.5)  avgt    5    24.967 ±  0.320  ms/op
BenchmarkQueries.query     1500000              10  SELECT NO_INDEX_STRING_COL,LOW_CARDINALITY_STRING_COL,COUNT(*) FROM MyTable GROUP BY LOW_CARDINALITY_STRING_COL,NO_INDEX_STRING_COL ORDER BY LOW_CARDINALITY_STRING_COL, NO_INDEX_STRING_COL ASC     EXP(0.5)  avgt    5    16.709 ±  0.314  ms/op
BenchmarkQueries.query     1500000              10                                                                                                               select count(*), year(INT_COL) as y, month(INT_COL) as m from MyTable group by y, m     EXP(0.5)  avgt    5   175.788 ± 11.362  ms/op
BenchmarkQueries.query     1500000              50                                                                                                                     SELECT RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5   917.493 ± 17.065  ms/op
BenchmarkQueries.query     1500000              50                                                                                       SELECT RAW_STRING_COL,RAW_INT_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY RAW_STRING_COL,RAW_INT_COL,INT_COL     EXP(0.5)  avgt    5  3623.057 ± 87.547  ms/op
BenchmarkQueries.query     1500000              50                                                           SELECT NO_INDEX_STRING_COL,INT_COL,COUNT(*) FROM MyTable GROUP BY NO_INDEX_STRING_COL,INT_COL ORDER BY NO_INDEX_STRING_COL, INT_COL ASC     EXP(0.5)  avgt    5   122.368 ±  1.879  ms/op
BenchmarkQueries.query     1500000              50  SELECT NO_INDEX_STRING_COL,LOW_CARDINALITY_STRING_COL,COUNT(*) FROM MyTable GROUP BY LOW_CARDINALITY_STRING_COL,NO_INDEX_STRING_COL ORDER BY LOW_CARDINALITY_STRING_COL, NO_INDEX_STRING_COL ASC     EXP(0.5)  avgt    5    82.259 ±  2.439  ms/op
BenchmarkQueries.query     1500000              50                                                                                                               select count(*), year(INT_COL) as y, month(INT_COL) as m from MyTable group by y, m     EXP(0.5)  avgt    5   781.819 ± 13.464  ms/op

Copy link
Contributor

@gortiz gortiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Thanks Yash! I know running these tests is very time consuming, but given it wasn't possible limit the change to only MSE, I wanted to be sure.

@Jackie-Jiang Jackie-Jiang merged commit d0d8b66 into apache:master Jan 21, 2025
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multi-stage Related to the multi-stage query engine performance query
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants