-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-29456][WebUI] Improve tooltip for Session Statistics Table column in JDBC/ODBC Server Tab #26138
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
[SPARK-29456][WebUI] Improve tooltip for Session Statistics Table column in JDBC/ODBC Server Tab #26138
Conversation
| "Session end time, after closing the session." | ||
|
|
||
| val THRIFT_SESSION_DURATION = | ||
| "Active duration of the session." |
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.
Please add tooltips for all columns. You can refer to our web-ui documentation:
Lines 416 to 447 in d334fee
| The second section contains information about active and finished sessions. | |
| * **User** and **IP** of the connection. | |
| * **Session id** link to access to session info. | |
| * **Start time**, **finish time** and **duration** of the session. | |
| * **Total execute** is the number of operations submitted in this session. | |
| <p style="text-align: center;"> | |
| <img src="img/JDBCServer2.png" title="JDBC/ODBC sessions" alt="JDBC/ODBC sessions"> | |
| </p> | |
| The third section has the SQL statistics of the submitted operations. | |
| * **User** that submit the operation. | |
| * **Job id** link to [jobs tab](web-ui.html#jobs-tab). | |
| * **Group id** of the query that group all jobs together. An application can cancel all running jobs using this group id. | |
| * **Start time** of the operation. | |
| * **Finish time** of the execution, before fetching the results. | |
| * **Close time** of the operation after fetching the results. | |
| * **Execution time** is the difference between finish time and start time. | |
| * **Duration time** is the difference between close time and start time. | |
| * **Statement** is the operation being executed. | |
| * **State** of the process. | |
| * _Started_, first state, when the process begins. | |
| * _Compiled_, execution plan generated. | |
| * _Failed_, final state when the execution failed or finished with error. | |
| * _Canceled_, final state when the execution is canceled. | |
| * _Finished_ processing and waiting to fetch results. | |
| * _Closed_, final state when client closed the statement. | |
| * **Detail** of the execution plan with parsed logical plan, analyzed logical plan, optimized logical plan and physical plan or errors in the the SQL statement. | |
| <p style="text-align: center;"> | |
| <img src="img/JDBCServer3.png" title="JDBC/ODBC SQL Statistics" alt="JDBC/ODBC SQL Statistics"> | |
| </p> |
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.
@wangyum in the same page statistics table does not have tooltip for all the column. Should we address that too?
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.
@wangyum i have added tooltip for all the columns in session table. Could you review?
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.
I don't think we need tooltips for every single column. "IP of the session" does not add anything over the column name. We should only add tooltips where they add something meaningful, like, clarifying for duration fields what the start and end time are exactly.
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.
Duration tooltips can basically specify one useful thing: what is the start and end time? this isn't telling the user anything more right now
|
ok to test |
|
Test build #112588 has finished for PR 26138 at commit
|
|
Test build #112597 has finished for PR 26138 at commit
|
35e6918 to
0028ccd
Compare
|
Test build #113208 has finished for PR 26138 at commit
|
srowen
left a comment
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.
Same here, these are basically repeating the title and I don't think we need to clutter the UI with that kind of popup tooltip
|
@srowen do you suggest that we have tooltips only for columns like |
| </a> | ||
| </th> | ||
| if (tooltip.nonEmpty) { | ||
| <th width={colWidthAttr}> |
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.
Can we avoid duplicating the th and a elements in both branches?
| </th> | ||
| if (tooltip.nonEmpty) { | ||
| <th width={colWidthAttr}> | ||
| <a href={headerLink}> |
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.
Same
| "Number of operations submitted in this session." | ||
|
|
||
| val THRIFT_SESSION_START_TIME = | ||
| "Session Start time,on launching the session." |
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.
Nit: space after ,
| "Session Start time,on launching the session." | ||
|
|
||
| val THRIFT_SESSION_FINISH_TIME = | ||
| "Session end time, after closing the session." |
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.
How about "Time that the session was closed"
| "Number of operations submitted in this session." | ||
|
|
||
| val THRIFT_SESSION_START_TIME = | ||
| "Session Start time,on launching the session." |
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.
"Time that the session was started"?
| "Session end time, after closing the session." | ||
|
|
||
| val THRIFT_SESSION_DURATION = | ||
| "Active duration of the session." |
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.
Duration tooltips can basically specify one useful thing: what is the start and end time? this isn't telling the user anything more right now
|
Test build #113708 has finished for PR 26138 at commit
|
|
Test build #113925 has finished for PR 26138 at commit
|
49df510 to
064ab01
Compare
|
Test build #113934 has finished for PR 26138 at commit
|
|
@srowen i have changed according to the review comments. Could you review? |
|
|
||
| val THRIFT_SESSION_DURATION = | ||
| "Total duration the session has been active, from the time session" + | ||
| " was lauched till it was closed." |
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.
lauched till -> launched until
But should we say 'started' instead anyway, to be consistent with the above?
| <th width={colWidthAttr}> | ||
| <a href={headerLink}> | ||
| { | ||
| if (tooltip.nonEmpty) { |
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 indents are too deep and a little inconsistent - just use 2 space indents
|
@srowen i have reworked on the review comments. Could you review? |
| "Time that the session was closed." | ||
|
|
||
| val THRIFT_SESSION_DURATION = | ||
| "Total duration the session has been active, from the time session" + |
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.
What does it show while it's active, time from start until now?
Maybe finally: Elapsed time since session start, or until closed if the session was closed.
| "Number of operations submitted in this session." | ||
|
|
||
| val THRIFT_SESSION_START_TIME = | ||
| "Session Start time, on launching the session." |
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.
"Time the session was started." But I'd just delete this and the next tooltip. It doesn't add much.
| {header} | ||
| </span> | ||
| } else { | ||
| {header} |
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.
Nit: unindent
|
Test build #113938 has finished for PR 26138 at commit
|
839dd12 to
09ee0d4
Compare
|
@srowen I have reworked. kindly verify the changes. |
|
Test build #113957 has finished for PR 26138 at commit
|
|
Merged to master |
What changes were proposed in this pull request?
Some of the columns of JDBC/ODBC tab Session info in Web UI are hard to understand.
Add tool tip for Start time, finish time , Duration and Total Execution
Why are the changes needed?
To improve the understanding of the WebUI
Does this PR introduce any user-facing change?
No
How was this patch tested?
manual test