Add ability to run AQE with optimizations end-to-end#19563
Merged
rschlussel merged 8 commits intoprestodb:masterfrom Jun 12, 2023
Merged
Add ability to run AQE with optimizations end-to-end#19563rschlussel merged 8 commits intoprestodb:masterfrom
rschlussel merged 8 commits intoprestodb:masterfrom
Conversation
rschlussel
force-pushed
the
aqe-stats-2
branch
6 times, most recently
from
0ae884e to
9e86cf1
Compare
pranjalssh
reviewed
May 10, 2023
Contributor
pranjalssh
left a comment
pranjalssh
left a comment
There was a problem hiding this comment.
Reviewed till commit 4
...t/java/com/facebook/presto/sql/planner/iterative/rule/TestDetermineJoinDistributionType.java
Outdated
Show resolved
Hide resolved
...st/java/com/facebook/presto/spark/adaptive/execution/TestPrestoSparkAdaptiveJoinQueries.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/JoinSwappingUtils.java
Outdated
Show resolved
Hide resolved
rschlussel
commented
May 12, 2023
Contributor
Author
There was a problem hiding this comment.
the test has crazy union alls now because PrestoSparkRowOutputOperator has a minimum row batch target size of about 1mb, which is also about the size of a hash table for a single orders table, so I needed to make that side even bigger to be able to validate the memory used by the join operator. Additionally, with optimize_hash_generation disabled, the row batch ends up having a retained size a bit bigger than 5mb (haven't gotten to the bottom of exactly why), so I needed to make the other side of the join big enough that it would fail on the build side with a 6mb memory limit.
rschlussel
force-pushed
the
aqe-stats-2
branch
2 times, most recently
from
1e8977b to
b8eb408
Compare
Contributor
Author
|
Gah... PrestoSparkRowOutputOperator memory usage is so temperamental and so close to the memory needed for the bigger table on the build side. it's making this pair of tests impossible to write. |
pranjalssh
reviewed
May 22, 2023
Contributor
There was a problem hiding this comment.
Have you checked whether hbo stats come up correctly for a remotesourcenode? It may not have a statsequivalentplannode attached
Contributor
Author
There was a problem hiding this comment.
great catch. it wasn't getting populated correctly. Didn't notice since i added it explicitly in my test.
Contributor
Author
There was a problem hiding this comment.
I put the fix for this earlier in the stack, so commit 6 is now commit 7
Contributor
There was a problem hiding this comment.
Will a test like this work:
SELECT * FROM nation n JOIN (SELECT * FROM orders cross join unnest sequence(1, 50)) o ON n.nationkey = o.orderkey
Contributor
Author
There was a problem hiding this comment.
no, wouldn't help.
The PrestoSparkStatsCalculator uses runtime stats if they differ from historical stats.
Allow using stats from RemoteSourceNodes for size-based join. This is necessary for runtime optimizations.
Join build and probe sides have different requirements for the local distribution they need. However, at runtime local exchanges have already been added, so when we flip join sides, we also need to adjust local exchanges on the left and right accordingly. Extract the code that handles this from RuntimeReorderJoinSides to a common utility class so we can reuse it for this optimizer. Without this change, queries will have wrong results. The test for this is in the commit that adds the reoptimization step for AQE.
Add re-optimization step for Presto on Spark adaptive execution
Move methods shared between multiple join swapping optimizers to JoinSwappingUtils.
28 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test plan - added unit tests. TODO: do a verifier run
This PR adds the missing features to complete the work to incorporate the PickJoinSides optimization into AQE