[SPARK-31271][UI] fix web ui for driver side SQL metrics by cloud-fan · Pull Request #28037 · apache/spark

cloud-fan · 2020-03-26T13:48:36Z

What changes were proposed in this pull request?

In #23551, we changed the metrics type of driver-side SQL metrics to size/time etc. which comes with max/min/median info.

This doesn't make sense for driver side SQL metrics as they have only one value. It makes the web UI hard to read:

This PR updates the SQL metrics UI to only display max/min/median if there are more than one metrics values:

Why are the changes needed?

Makes the UI easier to read

Does this PR introduce any user-facing change?

no

How was this patch tested?

manual test

cloud-fan · 2020-03-26T13:49:28Z

cc @gengliangwang @sarutak

SparkQA · 2020-03-26T15:29:28Z

Test build #120424 has finished for PR 28037 at commit 9e84f21.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

sql/core/src/main/scala/org/apache/spark/sql/execution/metric/SQLMetrics.scala

sarutak · 2020-03-26T18:52:58Z

@cloud-fan
Basically LGTM.
But as you suggested, the metrics format for stageId and attemptId can be changed so should we wait for #28039 to be merged.

dongjoon-hyun · 2020-03-26T21:03:32Z

sql/core/src/main/scala/org/apache/spark/sql/execution/metric/SQLMetrics.scala

Could you fix the following in SQLMetricsTestUtils.scala? That causes UT failures.

val totalNumBytesMetric = executedNode.metrics.find( _.name == "written output total (min, med, max (stageId (attemptId): taskId))").get

SparkQA · 2020-03-27T11:06:49Z

Test build #120477 has finished for PR 28037 at commit f91c51c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-03-27T12:46:58Z

Test build #120476 has finished for PR 28037 at commit acbaa45.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-03-27T16:33:00Z

Test build #120488 has finished for PR 28037 at commit 5838142.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2020-03-27T19:47:44Z

sql/core/src/main/scala/org/apache/spark/sql/execution/metric/SQLMetrics.scala

-        metric
+        s"total $METRICS_NAME_SUFFIX\n$sum ($min, $med, $max $taskInfo)"
      }
-      s"\n$sum ($min, $med, $max)"


Just a question: In this case, it seems that we didn't have $taskInfo previously.

We have, see https://github.com/apache/spark/pull/28037/files#diff-803f475b01acfae1c5c96807c2ea9ddcL215

dongjoon-hyun

+1, LGTM. I also verified this locally via web UI. Thank you, all.
Merged to master/3.0.

### What changes were proposed in this pull request? In #23551, we changed the metrics type of driver-side SQL metrics to size/time etc. which comes with max/min/median info. This doesn't make sense for driver side SQL metrics as they have only one value. It makes the web UI hard to read: ![image](https://user-images.githubusercontent.com/3182036/77653892-42db9900-6fab-11ea-8e7f-92f763fa32ff.png) This PR updates the SQL metrics UI to only display max/min/median if there are more than one metrics values: ![image](https://user-images.githubusercontent.com/3182036/77653975-5f77d100-6fab-11ea-849e-64c935377c8e.png) ### Why are the changes needed? Makes the UI easier to read ### Does this PR introduce any user-facing change? no ### How was this patch tested? manual test Closes #28037 from cloud-fan/ui. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit c4e98c0) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

### What changes were proposed in this pull request? In apache#23551, we changed the metrics type of driver-side SQL metrics to size/time etc. which comes with max/min/median info. This doesn't make sense for driver side SQL metrics as they have only one value. It makes the web UI hard to read: ![image](https://user-images.githubusercontent.com/3182036/77653892-42db9900-6fab-11ea-8e7f-92f763fa32ff.png) This PR updates the SQL metrics UI to only display max/min/median if there are more than one metrics values: ![image](https://user-images.githubusercontent.com/3182036/77653975-5f77d100-6fab-11ea-849e-64c935377c8e.png) ### Why are the changes needed? Makes the UI easier to read ### Does this PR introduce any user-facing change? no ### How was this patch tested? manual test Closes apache#28037 from cloud-fan/ui. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

sarutak reviewed Mar 26, 2020

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/metric/SQLMetrics.scala Show resolved Hide resolved

sql/core/src/main/scala/org/apache/spark/sql/execution/metric/SQLMetrics.scala Show resolved Hide resolved

dongjoon-hyun reviewed Mar 26, 2020

View reviewed changes

dongjoon-hyun added the WEB UI label Mar 26, 2020

cloud-fan mentioned this pull request Mar 27, 2020

[SPARK-31275][WEBUI] Improve the metrics format in ExecutionPage for StageId. #28039

Closed

cloud-fan force-pushed the ui branch 2 times, most recently from acbaa45 to f91c51c Compare March 27, 2020 08:57

fix web ui for driver side metrics

5838142

cloud-fan force-pushed the ui branch from f91c51c to 5838142 Compare March 27, 2020 11:40

dongjoon-hyun reviewed Mar 27, 2020

View reviewed changes

dongjoon-hyun approved these changes Mar 27, 2020

View reviewed changes

dongjoon-hyun closed this in c4e98c0 Mar 27, 2020

gengliangwang mentioned this pull request May 12, 2020

[SPARK-31440][SQL] Improve SQL Rest API #28208

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-31271][UI] fix web ui for driver side SQL metrics#28037

[SPARK-31271][UI] fix web ui for driver side SQL metrics#28037
cloud-fan wants to merge 1 commit intoapache:masterfrom
cloud-fan:ui

cloud-fan commented Mar 26, 2020

Uh oh!

cloud-fan commented Mar 26, 2020

Uh oh!

SparkQA commented Mar 26, 2020

Uh oh!

Uh oh!

Uh oh!

sarutak commented Mar 26, 2020

Uh oh!

dongjoon-hyun Mar 26, 2020 •

edited

Loading

Uh oh!

SparkQA commented Mar 27, 2020

Uh oh!

SparkQA commented Mar 27, 2020

Uh oh!

SparkQA commented Mar 27, 2020

Uh oh!

dongjoon-hyun Mar 27, 2020 •

edited

Loading

Uh oh!

cloud-fan Mar 30, 2020

Uh oh!

dongjoon-hyun left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

Conversation

cloud-fan commented Mar 26, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

cloud-fan commented Mar 26, 2020

Uh oh!

SparkQA commented Mar 26, 2020

Uh oh!

Uh oh!

Uh oh!

sarutak commented Mar 26, 2020

Uh oh!

dongjoon-hyun Mar 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 27, 2020

Uh oh!

SparkQA commented Mar 27, 2020

Uh oh!

SparkQA commented Mar 27, 2020

Uh oh!

dongjoon-hyun Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan Mar 30, 2020

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

dongjoon-hyun Mar 26, 2020 •

edited

Loading

dongjoon-hyun Mar 27, 2020 •

edited

Loading