Skip to content

Prevent over allocating for group-by reduce#8921

Merged
Jackie-Jiang merged 1 commit intoapache:masterfrom
Jackie-Jiang:prevent_over_allocating
Jun 18, 2022
Merged

Prevent over allocating for group-by reduce#8921
Jackie-Jiang merged 1 commit intoapache:masterfrom
Jackie-Jiang:prevent_over_allocating

Conversation

@Jackie-Jiang
Copy link
Copy Markdown
Contributor

When a group-by query has very large LIMIT, we over-allocate the array even if there are not that many records returned.

This PR:

  • Allocate array based on both the LIMIT and number of records returned
  • Short-circuit the calculation when there is no record returned, or LIMIT is 0

@Jackie-Jiang Jackie-Jiang added enhancement Improvement to existing functionality bugfix labels Jun 18, 2022
@Jackie-Jiang Jackie-Jiang requested a review from xiangfu0 June 18, 2022 02:37
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 18, 2022

Codecov Report

Merging #8921 (a7ceb17) into master (52bf09d) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #8921      +/-   ##
============================================
+ Coverage     69.73%   69.79%   +0.05%     
+ Complexity     4856     4606     -250     
============================================
  Files          1809     1809              
  Lines         94331    94337       +6     
  Branches      14069    14070       +1     
============================================
+ Hits          65784    65841      +57     
+ Misses        23970    23924      -46     
+ Partials       4577     4572       -5     
Flag Coverage Δ
integration1 26.69% <100.00%> (-0.01%) ⬇️
integration2 25.07% <100.00%> (+0.19%) ⬆️
unittests1 66.37% <71.42%> (-0.02%) ⬇️
unittests2 14.88% <0.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
...not/core/query/reduce/GroupByDataTableReducer.java 89.86% <100.00%> (+0.42%) ⬆️
...ore/query/scheduler/resources/ResourceManager.java 84.00% <0.00%> (-12.00%) ⬇️
...ache/pinot/core/operator/docidsets/OrDocIdSet.java 86.36% <0.00%> (-11.37%) ⬇️
.../predicate/NotEqualsPredicateEvaluatorFactory.java 73.07% <0.00%> (-5.77%) ⬇️
.../pinot/core/query/scheduler/PriorityScheduler.java 78.08% <0.00%> (-5.48%) ⬇️
...ntroller/helix/core/minion/TaskMetricsEmitter.java 86.04% <0.00%> (-4.66%) ⬇️
...ion/function/DistinctCountAggregationFunction.java 44.84% <0.00%> (-4.49%) ⬇️
...controller/helix/core/minion/PinotTaskManager.java 67.29% <0.00%> (-1.58%) ⬇️
...e/pinot/segment/local/io/util/PinotDataBitSet.java 95.62% <0.00%> (-1.46%) ⬇️
.../helix/core/realtime/SegmentCompletionManager.java 72.30% <0.00%> (-0.82%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52bf09d...a7ceb17. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants