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

Add a non-blocking groupBy implementation #14698

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xiangfu0
Copy link
Contributor

instead of all threads upsert to single IndexedTable, this approach will try to leverage each thread to generate local groupby result then merge and set.

@xiangfu0 xiangfu0 marked this pull request as draft December 22, 2024 09:35
@xiangfu0 xiangfu0 force-pushed the groupby-optimization branch 2 times, most recently from 7fe387a to 0dd717f Compare December 22, 2024 09:50
@codecov-commenter
Copy link

codecov-commenter commented Dec 22, 2024

Codecov Report

Attention: Patch coverage is 40.62500% with 114 lines in your changes missing coverage. Please review.

Project coverage is 63.81%. Comparing base (59551e4) to head (758d7fe).
Report is 1509 commits behind head on master.

Files with missing lines Patch % Lines
...tor/combine/PartitionedGroupByCombineOperator.java 0.00% 81 Missing ⚠️
...tor/combine/NonblockingGroupByCombineOperator.java 78.78% 7 Missing and 7 partials ⚠️
...org/apache/pinot/core/data/table/IndexedTable.java 20.00% 8 Missing ⚠️
...va/org/apache/pinot/core/plan/CombinePlanNode.java 63.63% 2 Missing and 2 partials ⚠️
...alcite/rel/rules/PinotAggregateToSemiJoinRule.java 0.00% 3 Missing ⚠️
...pinot/core/plan/maker/InstancePlanMakerImplV2.java 75.00% 1 Missing and 1 partial ⚠️
...core/operator/filter/predicate/PredicateUtils.java 0.00% 1 Missing ⚠️
...pinot/core/query/request/context/QueryContext.java 87.50% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #14698      +/-   ##
============================================
+ Coverage     61.75%   63.81%   +2.05%     
- Complexity      207     1607    +1400     
============================================
  Files          2436     2717     +281     
  Lines        133233   149944   +16711     
  Branches      20636    22962    +2326     
============================================
+ Hits          82274    95682   +13408     
- Misses        44911    47233    +2322     
- Partials       6048     7029     +981     
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 63.77% <40.62%> (+2.06%) ⬆️
java-21 63.70% <40.62%> (+2.08%) ⬆️
skip-bytebuffers-false 63.80% <40.62%> (+2.06%) ⬆️
skip-bytebuffers-true 56.00% <40.62%> (+28.27%) ⬆️
temurin 63.81% <40.62%> (+2.05%) ⬆️
unittests 63.80% <40.62%> (+2.06%) ⬆️
unittests1 56.17% <40.62%> (+9.28%) ⬆️
unittests2 34.34% <1.04%> (+6.60%) ⬆️

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.

@xiangfu0 xiangfu0 force-pushed the groupby-optimization branch 3 times, most recently from caa59d4 to 5eee7a3 Compare December 22, 2024 11:55
@ankitsultana
Copy link
Contributor

@xiangfu0 : are you folks also planning to run some benchmarks? And any other ideas you are already trying out?

@xiangfu0 xiangfu0 force-pushed the groupby-optimization branch 15 times, most recently from 3a1a06c to 530b877 Compare December 25, 2024 01:28
@xiangfu0 xiangfu0 force-pushed the groupby-optimization branch from 530b877 to 758d7fe Compare December 25, 2024 12:16
@wirybeaver
Copy link
Contributor

@xiangfu0 Glad to see the master is working on the partitioned groupBy algorithm. Do we plan to support disk spilled mode? #12080

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants