Skip to content

[multistage] add rel trait rule for individual hep optimization #11831

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.

Sign up for GitHub

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

Jump to bottom
Closed
walterddr wants to merge 5 commits into from
Closed

[multistage] add rel trait rule for individual hep optimization #11831

walterddr wants to merge 5 commits into from

Conversation

@walterddr
Copy link
Contributor

@walterddr walterddr commented Oct 18, 2023 •
edited
Loading

Allow propagation of partition distribution trait info across the tree to be used during Physical Planning phase.
This can be used to

  • unnecessary shuffling can be changed into direct (if the list of servers are identical)
  • multi-exchanges can be collapsed
  • allow more leaf-stage optimization rules to be run.

Follow up

  • This PR only allows LEFT-side trait propagation for JOIN nodes;
  • This PR does not support trait propagation for SET-UNION / WINDOW nodes.

@walterddr walterddr marked this pull request as draft October 18, 2023 19:51
walterddr
walterddr commented Oct 18, 2023
View reviewed changes
...-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotRelDistributionTraitRule.java Outdated
Copy link
Contributor Author

@walterddr walterddr Oct 18, 2023

Choose a reason for hiding this comment

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

  1. this rule should ideally propagate all RelDistribution trait to all RelNodes.
  2. when doing stage and worker/mailbox assignment, one can be sure to use the input of an LogicalExchange & the input to the LogicalExchange to determine whether the exchange can be a passthrough (fixing issues in [draft][multistage] add exchange removal rule #11794)

CC @Jackie-Jiang @ankitsultana

@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2023 •
edited
Loading

Codecov Report

Merging #11831 (13872cb) into master (8d49b11) will increase coverage by 0.02%.
The diff coverage is 82.72%.

@@             Coverage Diff              @@
##             master   #11831      +/-   ##
============================================
+ Coverage     61.40%   61.43%   +0.02%     
- Complexity     1145     1169      +24     
============================================
  Files          2378     2380       +2     
  Lines        128851   128941      +90     
  Branches      19926    19942      +16     
============================================
+ Hits          79127    79215      +88     
+ Misses        44002    43992      -10     
- Partials       5722     5734      +12
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration <0.01% <0.00%> (-0.01%) ⬇️
integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration2 0.00% <0.00%> (ø)
java-11 61.39% <82.72%> (+26.65%) ⬆️
java-21 61.31% <82.72%> (+0.01%) ⬆️
skip-bytebuffers-false 61.41% <82.72%> (+0.01%) ⬆️
skip-bytebuffers-true 61.28% <82.72%> (+0.03%) ⬆️
temurin 61.43% <82.72%> (+0.02%) ⬆️
unittests 61.43% <82.72%> (+0.02%) ⬆️
unittests1 46.65% <82.72%> (+0.03%) ⬆️
unittests2 27.63% <0.00%> (-0.01%) ⬇️

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

Files Coverage Δ
...ot/common/request/context/RequestContextUtils.java 68.00% <100.00%> (+0.36%) ⬆️
...n/request/context/predicate/ConstantPredicate.java 100.00% <100.00%> (ø)
...ot/common/request/context/predicate/Predicate.java 100.00% <100.00%> (ø)
...src/main/java/org/apache/pinot/sql/FilterKind.java 100.00% <100.00%> (ø)
...ery/optimizer/filter/NumericalFilterOptimizer.java 81.95% <ø> (ø)
.../java/org/apache/pinot/query/QueryEnvironment.java 91.33% <100.00%> (+0.74%) ⬆️
...org/apache/pinot/query/context/PlannerContext.java 100.00% <100.00%> (ø)
...va/org/apache/pinot/query/runtime/QueryRunner.java 74.22% <100.00%> (-6.48%) ⬇️
...ot/query/runtime/plan/OpChainExecutionContext.java 97.43% <100.00%> (+1.28%) ⬆️
...ime/plan/server/ServerOpChainExecutionContext.java 100.00% <100.00%> (ø)
... and 5 more

... and 12 files with indirect coverage changes

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

@walterddr walterddr force-pushed the poc_rel_distribution_trait_rule branch from c3d92b7 to 50f7ddb Compare October 19, 2023 18:03
@walterddr
Copy link
Contributor Author

walterddr commented Oct 23, 2023

Note to self:
Exchange elimination cannot be done in this rule. it has to be done after the RelDistribution has propagated all the way through. then a top-down (instead of bottom-up) rule needs to apply to eliminate exchange in a

  • (RelNode -> Exchange -> RelNode) fashion for singleRel
  • (RelNode -> left/right Exchange -> left/right RelNode) fashion for joins

otherwise the exchange elimination is done with not enough info and can cause join to not function properly

@Jackie-Jiang Jackie-Jiang added enhancement feature multi-stage Related to the multi-stage query engine labels Oct 25, 2023
This was referenced Oct 26, 2023
Closed
Merged
@walterddr walterddr force-pushed the poc_rel_distribution_trait_rule branch from 50f7ddb to c4c4ce5 Compare October 26, 2023 17:20
add boundary node detector
ca4380b 
connected with QueryRunner

refactor and simplify leaf stage compilation

fix linter and further refactoring

simplified visitor and context class, add javadoc

fix server plan request gen issues
@walterddr walterddr force-pushed the poc_rel_distribution_trait_rule branch 2 times, most recently from cced569 to a6c1e0d Compare November 1, 2023 00:15
@walterddr walterddr force-pushed the poc_rel_distribution_trait_rule branch from a6c1e0d to 38f00e6 Compare November 1, 2023 00:18
Rong Rong added 2 commits October 31, 2023 17:27
fix test
9feecdb
[init] add rel trait rule for individual hep optimization
e9c5d90 
[stash] add other nodes

[stash] adding elimination rules

[stash] fix RelOptUtil usage by utilizing direct method in RelNode

[stash] enable exchange elimination rule as well

[stash] adjust tests

[stash] adding table scan special handling too

[bugfix] change mapping for join to be correct on left only

[stash] fix build

fix test
@walterddr walterddr force-pushed the poc_rel_distribution_trait_rule branch from 38f00e6 to e9c5d90 Compare November 1, 2023 01:33
@walterddr walterddr changed the title [draft][multistage] add rel trait rule for individual hep optimization [multistage] add rel trait rule for individual hep optimization Nov 1, 2023
@walterddr walterddr marked this pull request as ready for review November 1, 2023 14:31
This was referenced Nov 1, 2023
Closed
Closed
@ankitsultana
Copy link
Contributor

ankitsultana commented Nov 1, 2023

@walterddr : Is this ready for review?

@walterddr
Copy link
Contributor Author

walterddr commented Nov 2, 2023

not yet @ankitsultana .we actually made some slight changes so that this PR doesn't depending on the other 2 PRs. marking it back to draft

@walterddr walterddr marked this pull request as draft November 2, 2023 19:30
@walterddr walterddr mentioned this pull request Nov 8, 2023
Merged
@walterddr
Copy link
Contributor Author

walterddr commented Nov 8, 2023

closing in favor of #11976

@walterddr walterddr closed this Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement feature multi-stage Related to the multi-stage query engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@walterddr @codecov-commenter @ankitsultana @Jackie-Jiang