-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
HIVE-27751 Log Query Compilation summary in an accumulated way #4755
Conversation
It could be a good idea to merge this and #4749 . |
9f361cd
to
0907476
Compare
0907476
to
cd68b07
Compare
cd68b07
to
5974762
Compare
5974762
to
c29e033
Compare
c29e033
to
ef8d2ff
Compare
bbcfa4d
to
ab32885
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good, I think the only remaining element is to have some tests around it so I added some hints of how this could be done with qtests.
Plus a bunch of small nits since we are probably gonna have at least another commit.
ql/src/java/org/apache/hadoop/hive/ql/QueryCompilationSummaryHook.java
Outdated
Show resolved
Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/QueryCompilationSummaryHook.java
Outdated
Show resolved
Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/QueryCompilationSummaryHook.java
Outdated
Show resolved
Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/QueryCompilationSummaryHook.java
Outdated
Show resolved
Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/QueryCompilationSummaryHook.java
Outdated
Show resolved
Hide resolved
ab32885
to
ad41025
Compare
ad41025
to
64655b0
Compare
@zabetak Can you look at the latest patch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the PR @ramesh0201 !
ql/src/java/org/apache/hadoop/hive/ql/QueryCompilationSummaryHook.java
Outdated
Show resolved
Hide resolved
64655b0
to
019582c
Compare
Quality Gate passedIssues Measures |
Thanks @zabetak for the review |
What changes were proposed in this pull request?
We are measuring and accumulating various stages in the Query compilation. The summary will look like below:
Query Compilation Summary
Compile Step - 1 parse 1ms
Compile Step - 2 Generate Resolved ParseTree 6ms
Compile Step - 3 Logical Plan and hive Operator Tree 142ms
Compile Step - 3.1 Generate Logical Plan 88ms
Compile Step - 3.1.1 Calcite: Plan generation 11ms
Compile Step - 3.1.2 MV Rewrite and Field Trimmer 2ms
Compile Step - 3.1.3 Removing SubQuery 1ms
Compile Step - 3.1.4 Decorrelation 0ms
Compile Step - 3.1.5 Validate Query Materialization 0ms
Compile Step - 3.1.6 Calcite: Prejoin ordering transformation 31ms
Compile Step - 3.1.7 MV Rewriting 0ms
Compile Step - 3.1.8 Calcite: Join Reordering 0ms
Compile Step - 3.1.9 Calcite: Postjoin ordering transformation 22ms
Compile Step - 3.1.10 Hive Sort Predicates 11ms
Compile Step - 3.2 Generate Operator Tree 54ms
Compile Step - 4 Deduce ResultsetSchema 0ms
Compile Step - 5 Parse Context generation 0ms
Compile Step - 6 Save and Validate View Creation 0ms
Compile Step - 7 Logical Optimization 7ms
Compile Step - 8 Physical Optimization 10ms
Compile Step - 9 Post Processing 0ms
Total Compilation Time 168ms
Why are the changes needed?
These changes are useful for reading and collecting all the measures of compile time in a single place. It is also useful in debugging a performance issue in the query compilation phase and also to report and compare with various runs
Does this PR introduce any user-facing change?
No
Is the change a dependency upgrade?
No
How was this patch tested?
Tested in the local for any q file with the config added