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

Allow server to directly return the final aggregation result #9304

Merged
merged 1 commit into from Sep 7, 2022

Conversation

Jackie-Jiang
Copy link
Contributor

When there is only one server queried, we can calculate the final aggregation result on the server side, instead of serializing back the intermediate result (which can be huge for distinct_count, percentile etc.).

This PR adds a new query option serverReturnFinalResult to let the server directly return the final aggregation result. As a follow up, we may add a flag to the broker config to let the broker automatically add this query option when it is querying only one server.

@Jackie-Jiang Jackie-Jiang added enhancement release-notes Referenced by PRs that need attention when compiling the next release notes labels Aug 31, 2022
@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2022

Codecov Report

Merging #9304 (a2a36a4) into master (784f995) will decrease coverage by 0.04%.
The diff coverage is 65.72%.

@@             Coverage Diff              @@
##             master    #9304      +/-   ##
============================================
- Coverage     69.77%   69.73%   -0.05%     
- Complexity     4692     4771      +79     
============================================
  Files          1873     1874       +1     
  Lines         99570    99706     +136     
  Branches      15160    15185      +25     
============================================
+ Hits          69477    69526      +49     
- Misses        25156    25247      +91     
+ Partials       4937     4933       -4     
Flag Coverage Δ
integration1 26.21% <30.04%> (+0.05%) ⬆️
integration2 24.89% <56.80%> (+0.07%) ⬆️
unittests1 66.94% <33.33%> (-0.10%) ⬇️
unittests2 15.28% <0.00%> (-0.03%) ⬇️

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

Impacted Files Coverage Δ
.../columnminmaxvalue/ColumnMinMaxValueGenerator.java 73.68% <0.00%> (ø)
...a/org/apache/pinot/segment/spi/ColumnMetadata.java 80.00% <0.00%> (-20.00%) ⬇️
...java/org/apache/pinot/segment/spi/V1Constants.java 12.50% <ø> (ø)
...va/org/apache/pinot/spi/utils/CommonConstants.java 27.69% <ø> (ø)
...aggregation/function/AggregationFunctionUtils.java 71.11% <33.33%> (-18.89%) ⬇️
...erator/blocks/results/AggregationResultsBlock.java 72.15% <48.78%> (-23.60%) ⬇️
...e/operator/blocks/results/GroupByResultsBlock.java 84.11% <50.00%> (-1.61%) ⬇️
...core/query/reduce/AggregationDataTableReducer.java 78.57% <62.50%> (-2.39%) ⬇️
...not/core/query/reduce/GroupByDataTableReducer.java 82.87% <67.90%> (-9.26%) ⬇️
.../org/apache/pinot/core/util/QueryOptionsUtils.java 68.00% <84.61%> (+1.33%) ⬆️
... and 36 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement release-notes Referenced by PRs that need attention when compiling the next release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants