[SPARK-29019][WebUI] Improve tooltip JDBC/ODBC Server tab by planga82 · Pull Request #25723 · apache/spark

planga82 · 2019-09-08T14:51:39Z

What changes were proposed in this pull request?

Some of the columns of JDBC/ODBC server tab in Web UI are hard to understand.
We have documented it at SPARK-28373 but I think it is better to have some tooltips in the SQL statistics table to explain the columns

The columns with new tooltips are finish time, close time, execution time and duration

Improvements in UIUtils can be used in other tables in WebUI to add tooltips

Why are the changes needed?

It is interesting to improve the undestanding of the WebUI

Does this PR introduce any user-facing change?

No

How was this patch tested?

Unit tests are added and manual test.

wangyum · 2019-09-08T14:54:23Z

ok to test

maropu · 2019-09-08T22:39:22Z

hi, @wangyum. You need a permission for the Spark jenkins to say "ok to test". So, you need to ask @shaneknapp for the permission, first.

maropu · 2019-09-08T22:39:51Z

ok to test

SparkQA · 2019-09-08T22:44:47Z

Test build #110318 has finished for PR 25723 at commit c4a209f.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

wangyum · 2019-09-09T02:22:43Z

Thank you @maropu I should have this permission: #25694

maropu · 2019-09-09T02:24:24Z

Oh, I see

SparkQA · 2019-09-09T23:24:23Z

Test build #110370 has finished for PR 25723 at commit 681b534.

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

juliuszsompolski · 2019-09-10T10:05:32Z

+ "Finish time of the execution, before fetching the results"
+
+ val THRIFT_SERVER_CLOSE_TIME =
+ "Close time of the process after fetching the results"


Maybe process -> statement or operation?

juliuszsompolski · 2019-09-10T10:07:12Z

 val table = if (numStatement > 0) {
 val headerRow = Seq("User", "JobID", "GroupID", "Start Time", "Finish Time", "Close Time",
 "Execution Time", "Duration", "Statement", "State", "Detail")
+ val tooltips = Seq(None, None, None, None, Some(ToolTips.THRIFT_SERVER_FINISH_TIME),


Could we add the same to ThriftServerSessionPage?

I thought it was the same code to generate this two pages. I forgot test it. Thanks!!

With the same tooltips of ThriftServerPage

SparkQA · 2019-09-10T21:24:13Z

Test build #110438 has finished for PR 25723 at commit 2d278d0.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

juliuszsompolski · 2019-09-10T21:31:51Z

 """
+
+ val THRIFT_SERVER_FINISH_TIME =
+ "Finish time of the execution, before fetching the results"


I think "Execution finish time" would sound better than "Finish time of the execution"

I'm Sorry, spanish common error when we write in english :-)

juliuszsompolski · 2019-09-10T21:32:03Z

+ "Finish time of the execution, before fetching the results"
+
+ val THRIFT_SERVER_CLOSE_TIME =
+ "Close time of the operation after fetching the results"


Likewise, "Operation close time"

juliuszsompolski · 2019-09-10T22:02:52Z

 "Execution Time", "Duration", "Statement", "State", "Detail")
+ val tooltips = Seq(None, None, None, None, Some(ToolTips.THRIFT_SERVER_FINISH_TIME),
+ Some(ToolTips.THRIFT_SERVER_CLOSE_TIME), Some(ToolTips.THRIFT_SERVER_EXECUTION),
+ Some(ToolTips.THRIFT_SERVER_DURATION), None, None, None)


Or, if not inside UIUtils.listingTable, maybe add assert(headerRow.length == tooltips.length) here and in ThriftServerSessionPage?

I've been thinking about it, yes, it's a good idea, in that way we don't break the homogeneous way of treating the parameters at UIUtils.listingTable and we ensure it is going to be less problems if new headers are added

SparkQA · 2019-09-10T23:53:26Z

Test build #110440 has finished for PR 25723 at commit 793ef92.

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

SparkQA · 2019-09-11T00:39:52Z

Test build #110441 has finished for PR 25723 at commit 344fd6f.

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

juliuszsompolski

LGTM. Thanks.

SparkQA · 2019-09-11T19:12:30Z

Test build #110482 has finished for PR 25723 at commit 6474935.

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

planga82 · 2019-09-11T20:43:17Z

the tests has failed but in two tests that don't have any relation with my last changes. Someone knows if it is normal?

juliuszsompolski · 2019-09-12T08:58:34Z

retest this please

SparkQA · 2019-09-12T11:24:26Z

Test build #110508 has finished for PR 25723 at commit 6474935.

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

juliuszsompolski

LGTM. cc @wangyum @gatorsmile

wangyum · 2019-09-23T10:39:27Z

retest this please

SparkQA · 2019-09-23T12:35:33Z

Test build #111217 has finished for PR 25723 at commit 6474935.

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

gengliangwang · 2019-09-23T12:51:06Z

retest this please

SparkQA · 2019-09-23T16:08:03Z

Test build #111224 has finished for PR 25723 at commit 6474935.

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

wangyum · 2019-09-25T00:34:24Z

 ephemerally when executors are being killed.
 """
+
+ val THRIFT_SERVER_FINISH_TIME =


Could we move these changes and tests to hive-thriftserver module? @srowen @dongjoon-hyun What do you think?

Yeah, if these are only relevant to the thrift server UI, they could go with the other thrift server UI code.

Ok, it make sense to have the tooltips in thriftserver module, i'm going to move it.
To generate the tables for this pages we use UIUtils from spark core module so I needed to improve this utils to add this new functionality.
Also, this improvements could be use in other pages to add tooltips to the tables.

SparkQA · 2019-09-26T21:02:05Z

Test build #111448 has finished for PR 25723 at commit 1bb6196.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-09-27T00:26:11Z

Test build #111451 has finished for PR 25723 at commit 6ab97b8.

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

SparkQA · 2019-09-27T18:44:28Z

Test build #111490 has finished for PR 25723 at commit 281c8ce.

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

SparkQA · 2019-09-27T21:07:12Z

Test build #4887 has finished for PR 25723 at commit 281c8ce.

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

planga82 · 2019-09-27T21:31:06Z

retest this please

srowen · 2019-09-28T19:48:13Z

+ val tooltips = Seq(None, None, None, None, Some(THRIFT_SERVER_FINISH_TIME),
+ Some(THRIFT_SERVER_CLOSE_TIME), Some(THRIFT_SERVER_EXECUTION),
+ Some(THRIFT_SERVER_DURATION), None, None, None)
+ assert(headerRow.length == tooltips.length)


Oh I see, you check it here. Usually I'd say use require but this is a good case for assert instead

wangyum

Also tested in my local:

srowen · 2019-09-29T23:34:44Z

Merged to master

gatorsmile · 2019-10-04T07:26:07Z

It looks great! @planga82 @wangyum @gengliangwang Could we do the similar enhancements for the other UI pages?

planga82 · 2019-10-06T09:30:41Z

@gatorsmile Ok, I can review it

planga82 added 6 commits August 11, 2019 15:46

Merge remote-tracking branch 'upstream/master'
bc0adbc

Merge remote-tracking branch 'upstream/master'
6f213ce

Merge remote-tracking branch 'upstream/master'
cd193a9

Merge remote-tracking branch 'upstream/master'
247d667

Merge remote-tracking branch 'upstream/master'
5956468

[SPARK-29019][WebUI] Improve tooltip JDBC/ODBC Server tab
c4a209f

dongjoon-hyun added the WEB UI label Sep 9, 2019

[SPARK-29019][WebUI] Fix scala style
681b534

juliuszsompolski reviewed Sep 10, 2019
View reviewed changes

gengliangwang reviewed Sep 10, 2019
View reviewed changes

Comment threadcore/src/main/scala/org/apache/spark/ui/UIUtils.scala

planga82 added 2 commits September 10, 2019 22:00

[SPARK-29019][WebUI] Fix typo and text
a95c90b

[SPARK-29019][WebUI] Update ThiftServerSessionPage
2d278d0
With the same tooltips of ThriftServerPage

[SPARK-29019][WebUI] Fix white space scala style
793ef92

juliuszsompolski reviewed Sep 10, 2019
View reviewed changes

planga82 added 2 commits September 10, 2019 23:42

[SPARK-29019][WebUI] Fix tooltip description
7f480f8

[SPARK-29019][WebUI] Improve comments
344fd6f

juliuszsompolski reviewed Sep 10, 2019
View reviewed changes

[SPARK-29019][WebUI] Assert equal lenght parameters
6474935

juliuszsompolski approved these changes Sep 11, 2019
View reviewed changes

juliuszsompolski approved these changes Sep 23, 2019
View reviewed changes

wangyum reviewed Sep 25, 2019
View reviewed changes

[SPARK-29019] Move tooltips to thrift server module
1bb6196

[SPARK-29019] Fix scala style
6ab97b8

wangyum reviewed Sep 27, 2019
View reviewed changes

Comment thread...erver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerSessionPage.scala Outdated

[SPARK-29019] delete blank line
281c8ce

srowen approved these changes Sep 28, 2019
View reviewed changes

wangyum approved these changes Sep 29, 2019
View reviewed changes

srowen closed this in 3ea9d68Sep 29, 2019

shahidki31 mentioned this pull request Oct 23, 2019
[SPARK-29453][WEBUI] Improve tooltips information for SQL tab. #26216
Closed

07ARB mentioned this pull request Nov 9, 2019
[SPARK-29453][WEBUI] Improve tooltips information for SQL tab. #26259
Closed

Uh oh!

Conversation

planga82 commented Sep 8, 2019

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!

wangyum commented Sep 8, 2019

Uh oh!

maropu commented Sep 8, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maropu commented Sep 8, 2019

Uh oh!

SparkQA commented Sep 8, 2019

Uh oh!

wangyum commented Sep 9, 2019

Uh oh!

maropu commented Sep 9, 2019

Uh oh!

SparkQA commented Sep 9, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Sep 10, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 10, 2019

Uh oh!

SparkQA commented Sep 11, 2019

Uh oh!

juliuszsompolski left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 11, 2019

Uh oh!

planga82 commented Sep 11, 2019

Uh oh!

juliuszsompolski commented Sep 12, 2019

Uh oh!

SparkQA commented Sep 12, 2019

Uh oh!

juliuszsompolski left a comment

Choose a reason for hiding this comment

Uh oh!

wangyum commented Sep 23, 2019

Uh oh!

SparkQA commented Sep 23, 2019

Uh oh!

gengliangwang commented Sep 23, 2019

Uh oh!

SparkQA commented Sep 23, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

planga82Sep 26, 2019• 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!

maropu commented Sep 8, 2019•
edited
Loading

planga82Sep 26, 2019•
edited
Loading