Skip to content

[SPARK-37383][SQL][WEBUI]Show the parsing time for each phase of a SQL on spark ui #35744

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
caican00 wants to merge 16 commits into from
Closed

[SPARK-37383][SQL][WEBUI]Show the parsing time for each phase of a SQL on spark ui #35744

caican00 wants to merge 16 commits into from

Conversation

@caican00
Copy link
Contributor

@caican00 caican00 commented Mar 7, 2022 •
edited
Loading

What changes were proposed in this pull request?

The PR is proposed to:
show the timeSpent for each phase of a SQL on spark ui.
image

image

The implications of the following indicators

parsing:   parser phase
analysis:   analysis phase
optimization:   optimization phase
planning:   physical plan phase

Why are the changes needed?

the time spent for each parsing phase of a SQL is counted and recorded in QueryPlanningTracker , but it is not yet shown anywhere.
when SQL parsing is suspected to be slow, we cannot confirm which phase is slow，therefore, it is necessary to show the SQL parsing time.

Does this PR introduce any user-facing change?

No

How was this patch tested?

modified tests.

@github-actions github-actions bot added the SQL label Mar 7, 2022
@caican00 caican00 changed the title 3.2.0 print sql parsing time [SPARK-37383][SQL]Print the parsing time for each phase of a SQL Mar 7, 2022
@caican00
Copy link
Contributor Author

caican00 commented Mar 7, 2022

@cloud-fan Could you please help review the code? thanks

LuciferYang
LuciferYang reviewed Mar 7, 2022
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala Outdated
*/
def logTimeSpent(): Unit = {
var totalTimeSpent = 0L
val timeSpentSummary: StringBuffer = new StringBuffer()
Copy link
Contributor

@LuciferYang LuciferYang Mar 7, 2022

Choose a reason for hiding this comment

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

Maybe StringBuilder is ok

Copy link
Contributor Author

@caican00 caican00 Mar 8, 2022

Choose a reason for hiding this comment

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

It seems so. thanks, updated

LuciferYang
LuciferYang reviewed Mar 7, 2022
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala
import org.apache.spark.internal.Logging
import org.apache.spark.util.BoundedPriorityQueue


Copy link
Contributor

@LuciferYang LuciferYang Mar 7, 2022

Choose a reason for hiding this comment

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

no need to remove this line

Copy link
Contributor Author

@caican00 caican00 Mar 8, 2022

Choose a reason for hiding this comment

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

thanks, updated

@AmplabJenkins
Copy link

AmplabJenkins commented Mar 7, 2022

Can one of the admins verify this patch?

@caican00
Copy link
Contributor Author

caican00 commented Mar 8, 2022 •
edited
Loading

@rxin Hi, could you help to review this patch? thanks

cloud-fan
cloud-fan reviewed Mar 8, 2022
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala Outdated


class QueryPlanningTracker {
class QueryPlanningTracker extends Logging {
Copy link
Contributor

@cloud-fan cloud-fan Mar 8, 2022

Choose a reason for hiding this comment

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

This is for third-party tools to track the query status... If we want Spark to show these metrics out of the box, I don't think log is the right place. Web UI is probably better but we need some design.

Copy link
Contributor Author

@caican00 caican00 Mar 10, 2022

Choose a reason for hiding this comment

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

It seems like a good idea, i'll try to fix it

Copy link
Contributor Author

@caican00 caican00 Mar 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@cloud-fan Hi, could you help to review this patch again? thanks

@caican00 caican00 changed the title [SPARK-37383][SQL]Print the parsing time for each phase of a SQL [SPARK-37383][SQL]Show the parsing time for each phase of a SQL on spark ui Mar 11, 2022
@LuciferYang
Copy link
Contributor

LuciferYang commented Mar 11, 2022 •
edited
Loading

@caican00 You should add a screenshot to the pr description and highlight the specific changes because this pr will cause WEBUI changes.

Need add [WEBUI] to pr title

@caican00 caican00 changed the title [SPARK-37383][SQL]Show the parsing time for each phase of a SQL on spark ui [SPARK-37383][SQL][WEBUI]Show the parsing time for each phase of a SQL on spark ui Mar 11, 2022
@caican00
Copy link
Contributor Author

caican00 commented Mar 15, 2022

@caican00 You should add a screenshot to the pr description and highlight the specific changes because this pr will cause WEBUI changes.

Need add [WEBUI] to pr title

@LuciferYang Hi, i have update it. Could you help to review this patch again? thanks

@yliou
Copy link
Contributor

yliou commented Mar 22, 2022

Hi @caican00, I was working on the same idea but have a different approach at #35939 which also has a REST endpoint and Spark rule timing information.

@github-actions
Copy link

github-actions bot commented Sep 27, 2022

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Sep 27, 2022
@github-actions github-actions bot closed this Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LuciferYang LuciferYang LuciferYang left review comments

@cloud-fan cloud-fan cloud-fan left review comments

Assignees

No one assigned

Labels

SQL Stale WEB UI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@caican00 @AmplabJenkins @LuciferYang @yliou @cloud-fan