You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/03/07 05:17:54 UTC
[GitHub] [spark] caican00 opened a new pull request #35744: 3.2.0 print sql parsing time
caican00 opened a new pull request #35744:
URL: https://github.com/apache/spark/pull/35744
What changes were proposed in this pull request?
The PR is proposed to:
print out the timeSpent for each phase of a SQL and the total timeSpent of the SQL for parsing
related issue: https://issues.apache.org/jira/browse/SPARK-37383
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 print out the SQL parsing time.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Existing tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 commented on a change in pull request #35744: [SPARK-37383][SQL]Print the parsing time for each phase of a SQL
Posted by GitBox <gi...@apache.org>.
caican00 commented on a change in pull request #35744:
URL: https://github.com/apache/spark/pull/35744#discussion_r823273706
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala
##########
@@ -90,7 +91,7 @@ object QueryPlanningTracker {
}
-class QueryPlanningTracker {
+class QueryPlanningTracker extends Logging {
Review comment:
It seems like a good idea, i'll try to fix it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #35744: [SPARK-37383][SQL]Print the parsing time for each phase of a SQL
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35744:
URL: https://github.com/apache/spark/pull/35744#discussion_r820981669
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala
##########
@@ -120,6 +120,24 @@ class QueryPlanningTracker {
ret
}
+ /**
+ * print out the timeSpent for each phase of a SQL
+ */
+ def logTimeSpent(): Unit = {
+ var totalTimeSpent = 0L
+ val timeSpentSummary: StringBuffer = new StringBuffer()
Review comment:
Maybe `StringBuilder ` is ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 commented on pull request #35744: [SPARK-37383][SQL][WEBUI]Show the parsing time for each phase of a SQL on spark ui
Posted by GitBox <gi...@apache.org>.
caican00 commented on pull request #35744:
URL: https://github.com/apache/spark/pull/35744#issuecomment-1064833328
```
@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 Thank you for your suggestion so much. I have updated it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 commented on pull request #35744: [SPARK-37383][SQL]Print the parsing time for each phase of a SQL
Posted by GitBox <gi...@apache.org>.
caican00 commented on pull request #35744:
URL: https://github.com/apache/spark/pull/35744#issuecomment-1061367789
cc @rxin
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #35744: [SPARK-37383][SQL]Print the parsing time for each phase of a SQL
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35744:
URL: https://github.com/apache/spark/pull/35744#discussion_r821705409
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala
##########
@@ -90,7 +91,7 @@ object QueryPlanningTracker {
}
-class QueryPlanningTracker {
+class QueryPlanningTracker extends Logging {
Review comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 commented on a change in pull request #35744: [SPARK-37383][SQL]Print the parsing time for each phase of a SQL
Posted by GitBox <gi...@apache.org>.
caican00 commented on a change in pull request #35744:
URL: https://github.com/apache/spark/pull/35744#discussion_r821269514
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala
##########
@@ -120,6 +120,24 @@ class QueryPlanningTracker {
ret
}
+ /**
+ * print out the timeSpent for each phase of a SQL
+ */
+ def logTimeSpent(): Unit = {
+ var totalTimeSpent = 0L
+ val timeSpentSummary: StringBuffer = new StringBuffer()
Review comment:
It seems so. thanks, updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 commented on pull request #35744: [SPARK-37383][SQL][WEBUI]Show the parsing time for each phase of a SQL on spark ui
Posted by GitBox <gi...@apache.org>.
caican00 commented on pull request #35744:
URL: https://github.com/apache/spark/pull/35744#issuecomment-1067617905
> @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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #35744: [SPARK-37383][SQL]Print the parsing time for each phase of a SQL
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35744:
URL: https://github.com/apache/spark/pull/35744#discussion_r820983423
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala
##########
@@ -19,9 +19,9 @@ package org.apache.spark.sql.catalyst
import scala.collection.JavaConverters._
+import org.apache.spark.internal.Logging
import org.apache.spark.util.BoundedPriorityQueue
-
Review comment:
no need to remove this line
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yliou commented on pull request #35744: [SPARK-37383][SQL][WEBUI]Show the parsing time for each phase of a SQL on spark ui
Posted by GitBox <gi...@apache.org>.
yliou commented on pull request #35744:
URL: https://github.com/apache/spark/pull/35744#issuecomment-1075492766
Hi @caican00, I was working on the same idea but have a different approach at https://github.com/apache/spark/pull/35939 which also has a REST endpoint and Spark rule timing information.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 commented on pull request #35744: [SPARK-37383][SQL]Print the parsing time for each phase of a SQL
Posted by GitBox <gi...@apache.org>.
caican00 commented on pull request #35744:
URL: https://github.com/apache/spark/pull/35744#issuecomment-1060294872
@cloud-fan Could you please help review the code? thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #35744: [SPARK-37383][SQL]Show the parsing time for each phase of a SQL on spark ui
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35744:
URL: https://github.com/apache/spark/pull/35744#issuecomment-1064780860
@caican00 You should add a screenshot to the pr description and highlight the specific changes because this pr will cause `WEBUI` changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 commented on a change in pull request #35744: [SPARK-37383][SQL]Print the parsing time for each phase of a SQL
Posted by GitBox <gi...@apache.org>.
caican00 commented on a change in pull request #35744:
URL: https://github.com/apache/spark/pull/35744#discussion_r821269673
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala
##########
@@ -19,9 +19,9 @@ package org.apache.spark.sql.catalyst
import scala.collection.JavaConverters._
+import org.apache.spark.internal.Logging
import org.apache.spark.util.BoundedPriorityQueue
-
Review comment:
thanks, updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang edited a comment on pull request #35744: [SPARK-37383][SQL]Show the parsing time for each phase of a SQL on spark ui
Posted by GitBox <gi...@apache.org>.
LuciferYang edited a comment on pull request #35744:
URL: https://github.com/apache/spark/pull/35744#issuecomment-1064780860
@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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 commented on a change in pull request #35744: [SPARK-37383][SQL][WEBUI]Show the parsing time for each phase of a SQL on spark ui
Posted by GitBox <gi...@apache.org>.
caican00 commented on a change in pull request #35744:
URL: https://github.com/apache/spark/pull/35744#discussion_r825729245
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala
##########
@@ -90,7 +91,7 @@ object QueryPlanningTracker {
}
-class QueryPlanningTracker {
+class QueryPlanningTracker extends Logging {
Review comment:
@cloud-fan Hi, could you help to review this patch again? thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 commented on a change in pull request #35744: [SPARK-37383][SQL][WEBUI]Show the parsing time for each phase of a SQL on spark ui
Posted by GitBox <gi...@apache.org>.
caican00 commented on a change in pull request #35744:
URL: https://github.com/apache/spark/pull/35744#discussion_r825729245
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala
##########
@@ -90,7 +91,7 @@ object QueryPlanningTracker {
}
-class QueryPlanningTracker {
+class QueryPlanningTracker extends Logging {
Review comment:
@cloud-fan Hi, could you review this patch again? thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 commented on a change in pull request #35744: [SPARK-37383][SQL]Print the parsing time for each phase of a SQL
Posted by GitBox <gi...@apache.org>.
caican00 commented on a change in pull request #35744:
URL: https://github.com/apache/spark/pull/35744#discussion_r821268713
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/QueryPlanningTracker.scala
##########
@@ -19,9 +19,9 @@ package org.apache.spark.sql.catalyst
import scala.collection.JavaConverters._
+import org.apache.spark.internal.Logging
import org.apache.spark.util.BoundedPriorityQueue
-
Review comment:
thanks! updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #35744: [SPARK-37383][SQL]Print the parsing time for each phase of a SQL
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35744:
URL: https://github.com/apache/spark/pull/35744#issuecomment-1061250405
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 removed a comment on pull request #35744: [SPARK-37383][SQL][WEBUI]Show the parsing time for each phase of a SQL on spark ui
Posted by GitBox <gi...@apache.org>.
caican00 removed a comment on pull request #35744:
URL: https://github.com/apache/spark/pull/35744#issuecomment-1064833328
```
@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 Thank you for your suggestion so much. I have updated it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] caican00 edited a comment on pull request #35744: [SPARK-37383][SQL][WEBUI]Show the parsing time for each phase of a SQL on spark ui
Posted by GitBox <gi...@apache.org>.
caican00 edited a comment on pull request #35744:
URL: https://github.com/apache/spark/pull/35744#issuecomment-1061367789
@rxin Hi, could you help to review this patch? thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org