You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by kayousterhout <gi...@git.apache.org> on 2014/10/21 03:51:01 UTC
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
GitHub user kayousterhout opened a pull request:
https://github.com/apache/spark/pull/2867
[SPARK-4016] Allow user to show/hide UI metrics.
This commit adds a set of checkboxes to the stage detail
page that the user can use to show additional task metrics,
including the GC time, result serialization time, result fetch
time, and scheduler delay. All of these metrics are now
hidden by default. This allows advanced users to look at more
detailed metrics, without distracting the average user.
This change also cleans up the stage detail page so that metrics
are shown in the same order in the summary table as in the task table,
and updates the metrics in both tables such that they contain the same
set of metrics.
The ability to remember a user's preferences for which metrics
should be shown has been filed as SPARK-4024.
Here's what the stage detail page looks like by default:
and with a few additional metrics shown:
cc @shivaram @andrewor14
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kayousterhout/spark-1 SPARK-checkboxes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/2867.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2867
----
commit e989560562b473624159e4e3554ec9898884a247
Author: Kay Ousterhout <ka...@gmail.com>
Date: 2014-10-21T01:30:38Z
[SPARK-4016] Allow user to show/hide UI metrics.
This commit adds a set of checkboxes to the stage detail
page that the user can use to show additional task metrics,
including the GC time, result serialization time, result fetch
time, and scheduler delay. All of these metrics are now
hidden by default. This allows advanced users to look at more
detailed metrics, without distracting the average user.
This change also cleans up the stage detail page so that metrics
are shown in the same order in the summary table as in the task table,
and updates the metrics in both tables such that they contain the same
set of metrics.
The ability to remember a user's preferences for which metrics
should be shown has been filed as SPARK-4024.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60170250
Do we have tooltips or a short description of what each of those metrics are before we check the boxes? Or do we have to hover over the column name itself to learn that?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61191735
[Test build #22567 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22567/consoleFull) for PR 2867 at commit [`6015913`](https://github.com/apache/spark/commit/6015913b1594f3f0868d261bbf7c84e414cd8b93).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class WriteAheadLogBackedBlockRDDPartition(`
* `class WriteAheadLogBackedBlockRDD[T: ClassTag](`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-62073914
Hey by the way here's a usability nit that grew adversely on me over time. Every time I unchecked everything, it feels very natural to close the menu, but when I open the menu again to see what I unchecked, it automatically checks everything. I would think that the little triangle doesn't actually change my settings but only collapses/expands the boxes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/2867
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60155916
Ok I updated this, as well as the photos in the description at the top. As per @andrewor14's suggestion, there's now an expandable menu of options. Based on @ash211's suggestion, when you click the menu to expand it, all of the options get checked by default (and then the user can uncheck options she isn't interested in, if necessary). Let me know what your thoughts are!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60496109
[Test build #22220 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22220/consoleFull) for PR 2867 at commit [`c5a8b99`](https://github.com/apache/spark/commit/c5a8b99ed8f11f80c3bcb4d7ef2153abae6794bb).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61191743
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22567/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/2867#discussion_r19574596
--- Diff: core/src/main/scala/org/apache/spark/ui/UIUtils.scala ---
@@ -247,22 +250,32 @@ private[spark] object UIUtils extends Logging {
}
val colWidth = 100.toDouble / headers.size
val colWidthAttr = if (fixedWidth) colWidth + "%" else ""
- val headerRow: Seq[Node] = {
- // if none of the headers have "\n" in them
- if (headers.forall(!_.contains("\n"))) {
- // represent header as simple text
- headers.map(h => <th width={colWidthAttr}>{h}</th>)
+
+ def getClass(index: Int): String = {
+ if (index < headerClasses.size) {
+ headerClasses(index)
} else {
- // represent header text as list while respecting "\n"
- headers.map { case h =>
- <th width={colWidthAttr}>
- <ul class ="unstyled">
- { h.split("\n").map { case t => <li> {t} </li> } }
- </ul>
- </th>
- }
+ ""
+ }
+ }
+
+ val newlinesInHeader = headers.exists(_.contains("\n"))
--- End diff --
Oh I see, don't worry about it then. I thought it's just a coding style thing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60493993
[Test build #22220 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22220/consoleFull) for PR 2867 at commit [`c5a8b99`](https://github.com/apache/spark/commit/c5a8b99ed8f11f80c3bcb4d7ef2153abae6794bb).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60020570
Hm, I think from the UX perspective this requires the cursor to move further if they want to toggle multiple options at once, which could be annoying. Also, if we align them horizontally we'll eventually run out of space as we add more options.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60442759
I've decided to hold off on introducing my table-builder abstraction, since it's kind of leaky because there are corner-cases that it doesn't handle cleanly. Therefore, this shouldn't block on that. It would be great to try to get this merged so that it can unblock your other metrics patch.
Regarding tooptips: I agree with Andrew that inline descriptions of the metrics might look a little cluttered, so hover tooltips would be a nice addition. This is a net improvement over what we have now, though, so I suppose we could wait until later to add those tooltips.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60811266
Hey @kayousterhout this LGTM other than a few very minor comments, though I haven't looked at the JS / CSS code in detail. Do we plan to make more of the existing columns "additional columns"? Is this just a starting point?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60653883
@JoshRosen @andrewor14 does one of you have time to look at this today? I added the tool tips to each additional metric. Would be great to get this in so that I can update my other metrics pull request that depends on this one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60305233
Yeah I'm not sure. I kinda like the way it looks right now. It might look a little cluttered if it's inlined.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/2867#discussion_r19494377
--- Diff: core/src/main/scala/org/apache/spark/ui/UIUtils.scala ---
@@ -247,22 +250,32 @@ private[spark] object UIUtils extends Logging {
}
val colWidth = 100.toDouble / headers.size
val colWidthAttr = if (fixedWidth) colWidth + "%" else ""
- val headerRow: Seq[Node] = {
- // if none of the headers have "\n" in them
- if (headers.forall(!_.contains("\n"))) {
- // represent header as simple text
- headers.map(h => <th width={colWidthAttr}>{h}</th>)
+
+ def getClass(index: Int): String = {
+ if (index < headerClasses.size) {
+ headerClasses(index)
} else {
- // represent header text as list while respecting "\n"
- headers.map { case h =>
- <th width={colWidthAttr}>
- <ul class ="unstyled">
- { h.split("\n").map { case t => <li> {t} </li> } }
- </ul>
- </th>
- }
+ ""
+ }
+ }
+
+ val newlinesInHeader = headers.exists(_.contains("\n"))
+ def getHeaderContent(header: String): Seq[Node] = {
+ if (newlinesInHeader) {
+ <ul class="unstyled">
+ { header.split("\n").map { case t => <li> {t} </li> } }
+ </ul>
+ } else {
+ Text(header)
+ }
+ }
+
+ val headerRow: Seq[Node] = {
+ headers.view.zipWithIndex.map{ x =>
--- End diff --
space before {
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61302657
Thanks Josh, and thanks to you and Andrew both for the thorough reviews!!
This is much more usable now than my original version.
On Fri, Oct 31, 2014 at 10:30 AM, asfgit <no...@github.com> wrote:
> Closed #2867 <https://github.com/apache/spark/pull/2867> via adb6415
> <https://github.com/apache/spark/commit/adb6415c1d65d466a10c50e8dc6cb3bf2805ebdf>
> .
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/spark/pull/2867#event-186698666>.
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/2867#discussion_r19572899
--- Diff: core/src/main/scala/org/apache/spark/ui/UIUtils.scala ---
@@ -247,22 +250,32 @@ private[spark] object UIUtils extends Logging {
}
val colWidth = 100.toDouble / headers.size
val colWidthAttr = if (fixedWidth) colWidth + "%" else ""
- val headerRow: Seq[Node] = {
- // if none of the headers have "\n" in them
- if (headers.forall(!_.contains("\n"))) {
- // represent header as simple text
- headers.map(h => <th width={colWidthAttr}>{h}</th>)
+
+ def getClass(index: Int): String = {
+ if (index < headerClasses.size) {
+ headerClasses(index)
} else {
- // represent header text as list while respecting "\n"
- headers.map { case h =>
- <th width={colWidthAttr}>
- <ul class ="unstyled">
- { h.split("\n").map { case t => <li> {t} </li> } }
- </ul>
- </th>
- }
+ ""
+ }
+ }
+
+ val newlinesInHeader = headers.exists(_.contains("\n"))
--- End diff --
I could but then it becomes a n^2 computation on the headers, because it has to scan through all of the headers each time it adds new header content. n is not huge so maybe a non-issue; would you still prefer inlining it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61007239
@andrewor14 I filed a JIRA for the accumulators thing, which I think we should actually handle by just hiding that column when no accumulators are defined (similar to what we do for shuffle read, shuffle write, etc.): https://issues.apache.org/jira/browse/SPARK-4141
Re:(de)select all, currently, everything is deselected by default. If you click the "show additional metrics" expandy header, it then shows you all the options *and* selects all of them -- which I thought might most closely match what most people will want (based on my use of this and @ash211's comment). Can we wait to add more additional select / deselect-all boxes until we see how people end up using this and what's actually helpful?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60170174
Looks awesome from a visual point of view, though I haven't looked at the code in detail yet.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60011996
Haha @andrewor14 that's exactly how I had it originally but @pwendell wanted it to look like this. I see arguments for both versions so happy with whatever the two of you agree on!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60443288
I can add the tooltips in an hour or so -- was waiting to modify this to
hear if there were more review comments, but it sounds like there are not!
Will post an updated version in a bit. Thanks for the update Josh!
On Fri, Oct 24, 2014 at 1:14 PM, Josh Rosen <no...@github.com>
wrote:
> I've decided to hold off on introducing my table-builder abstraction,
> since it's kind of leaky because there are corner-cases that it doesn't
> handle cleanly. Therefore, this shouldn't block on that. It would be great
> to try to get this merged so that it can unblock your other metrics patch.
>
> Regarding tooptips: I agree with Andrew that inline descriptions of the
> metrics might look a little cluttered, so hover tooltips would be a nice
> addition. This is a net improvement over what we have now, though, so I
> suppose we could wait until later to add those tooltips.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/spark/pull/2867#issuecomment-60442759>.
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-59872492
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21952/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/2867#discussion_r19494602
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -65,55 +65,100 @@ private[ui] class StagePage(parent: JobProgressTab) extends WebUIPage("stage") {
<strong>Total task time across all tasks: </strong>
{UIUtils.formatDuration(stageData.executorRunTime)}
</li>
- {if (hasInput)
+ {if (hasInput) {
<li>
<strong>Input: </strong>
{Utils.bytesToString(stageData.inputBytes)}
</li>
- }
- {if (hasShuffleRead)
+ }}
+ {if (hasShuffleRead) {
<li>
<strong>Shuffle read: </strong>
{Utils.bytesToString(stageData.shuffleReadBytes)}
</li>
- }
- {if (hasShuffleWrite)
+ }}
+ {if (hasShuffleWrite) {
<li>
<strong>Shuffle write: </strong>
{Utils.bytesToString(stageData.shuffleWriteBytes)}
</li>
- }
- {if (hasBytesSpilled)
- <li>
- <strong>Shuffle spill (memory): </strong>
- {Utils.bytesToString(stageData.memoryBytesSpilled)}
- </li>
- <li>
- <strong>Shuffle spill (disk): </strong>
- {Utils.bytesToString(stageData.diskBytesSpilled)}
- </li>
- }
+ }}
+ {if (hasBytesSpilled) {
+ <li>
+ <strong>Shuffle spill (memory): </strong>
+ {Utils.bytesToString(stageData.memoryBytesSpilled)}
+ </li>
+ <li>
+ <strong>Shuffle spill (disk): </strong>
+ {Utils.bytesToString(stageData.diskBytesSpilled)}
+ </li>
+ }}
</ul>
</div>
- // scalastyle:on
+
+ val showAdditionalMetrics =
+ <div>
+ <span class="expand-additional-metrics-arrow arrow-closed"></span>
+ <span class="expand-additional-metrics"><strong>Show additional metrics</strong></span>
+ <div class="additional-metrics collapsed">
+ <ul style="list-style-type:none">
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.SCHEDULER_DELAY} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.SCHEDULER_DELAY}/>
+ Scheduler Delay
+ </span>
+ </li>
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.GC_TIME} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.GC_TIME}/>
+ GC Time
+ </span>
+ </li>
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.RESULT_SERIALIZATION_TIME} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.RESULT_SERIALIZATION_TIME}/>
+ Result Serialization Time
+ </span>
+ </li>
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.GETTING_RESULT_TIME} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.GETTING_RESULT_TIME}/>
+ Getting Result Time
+ </span>
+ </li>
+ </ul>
+ </div>
+ </div>
+
val accumulableHeaders: Seq[String] = Seq("Accumulable", "Value")
def accumulableRow(acc: AccumulableInfo) = <tr><td>{acc.name}</td><td>{acc.value}</td></tr>
val accumulableTable = UIUtils.listingTable(accumulableHeaders, accumulableRow,
accumulables.values.toSeq)
- val taskHeaders: Seq[String] =
+ val taskHeadersAndClasses: Seq[(String, String)] =
Seq(
- "Index", "ID", "Attempt", "Status", "Locality Level", "Executor ID / Host",
- "Launch Time", "Duration", "GC Time", "Accumulators") ++
- {if (hasInput) Seq("Input") else Nil} ++
- {if (hasShuffleRead) Seq("Shuffle Read") else Nil} ++
- {if (hasShuffleWrite) Seq("Write Time", "Shuffle Write") else Nil} ++
- {if (hasBytesSpilled) Seq("Shuffle Spill (Memory)", "Shuffle Spill (Disk)") else Nil} ++
- Seq("Errors")
+ ("Index", ""), ("ID", ""), ("Attempt", ""), ("Status", ""), ("Locality Level", ""),
+ ("Executor ID / Host", ""), ("Launch Time", ""), ("Duration", ""), ("Accumulators", ""),
+ ("Scheduler Delay", TaskDetailsClassNames.SCHEDULER_DELAY),
+ ("GC Time", TaskDetailsClassNames.GC_TIME),
+ ("Result Serialization Time", TaskDetailsClassNames.RESULT_SERIALIZATION_TIME),
+ ("Getting Result Time", TaskDetailsClassNames.GETTING_RESULT_TIME)) ++
+ {if (hasInput) Seq(("Input", "")) else Nil} ++
+ {if (hasShuffleRead) Seq(("Shuffle Read", "")) else Nil} ++
+ {if (hasShuffleWrite) Seq(("Write Time", ""), ("Shuffle Write", "")) else Nil} ++
+ {if (hasBytesSpilled) Seq(("Shuffle Spill (Memory)", ""), ("Shuffle Spill (Disk)", ""))
+ else Nil} ++
+ Seq(("Errors", ""))
+
+ val unzipped = taskHeadersAndClasses.unzip
val taskTable = UIUtils.listingTable(
- taskHeaders, taskRow(hasInput, hasShuffleRead, hasShuffleWrite, hasBytesSpilled), tasks)
-
+ unzipped._1, taskRow(hasInput, hasShuffleRead, hasShuffleWrite, hasBytesSpilled), tasks,
+ headerClasses=unzipped._2)
--- End diff --
I think you need spaces before and after `=`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-59872654
@kayousterhout This might integrate nicely with my #2852, which introduces some new abstractions to simplify the web UI's table rendering code. With my framework, I think you might be able to automatically generate the ids used to show / hide columns rather than having to have a class that holds a bunch of strings.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/2867#discussion_r19494574
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -65,55 +65,100 @@ private[ui] class StagePage(parent: JobProgressTab) extends WebUIPage("stage") {
<strong>Total task time across all tasks: </strong>
{UIUtils.formatDuration(stageData.executorRunTime)}
</li>
- {if (hasInput)
+ {if (hasInput) {
<li>
<strong>Input: </strong>
{Utils.bytesToString(stageData.inputBytes)}
</li>
- }
- {if (hasShuffleRead)
+ }}
+ {if (hasShuffleRead) {
<li>
<strong>Shuffle read: </strong>
{Utils.bytesToString(stageData.shuffleReadBytes)}
</li>
- }
- {if (hasShuffleWrite)
+ }}
+ {if (hasShuffleWrite) {
<li>
<strong>Shuffle write: </strong>
{Utils.bytesToString(stageData.shuffleWriteBytes)}
</li>
- }
- {if (hasBytesSpilled)
- <li>
- <strong>Shuffle spill (memory): </strong>
- {Utils.bytesToString(stageData.memoryBytesSpilled)}
- </li>
- <li>
- <strong>Shuffle spill (disk): </strong>
- {Utils.bytesToString(stageData.diskBytesSpilled)}
- </li>
- }
+ }}
+ {if (hasBytesSpilled) {
+ <li>
+ <strong>Shuffle spill (memory): </strong>
+ {Utils.bytesToString(stageData.memoryBytesSpilled)}
+ </li>
+ <li>
+ <strong>Shuffle spill (disk): </strong>
+ {Utils.bytesToString(stageData.diskBytesSpilled)}
+ </li>
+ }}
</ul>
</div>
- // scalastyle:on
+
+ val showAdditionalMetrics =
+ <div>
+ <span class="expand-additional-metrics-arrow arrow-closed"></span>
+ <span class="expand-additional-metrics"><strong>Show additional metrics</strong></span>
+ <div class="additional-metrics collapsed">
+ <ul style="list-style-type:none">
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.SCHEDULER_DELAY} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.SCHEDULER_DELAY}/>
+ Scheduler Delay
+ </span>
+ </li>
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.GC_TIME} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.GC_TIME}/>
+ GC Time
+ </span>
+ </li>
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.RESULT_SERIALIZATION_TIME} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.RESULT_SERIALIZATION_TIME}/>
+ Result Serialization Time
+ </span>
+ </li>
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.GETTING_RESULT_TIME} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.GETTING_RESULT_TIME}/>
+ Getting Result Time
+ </span>
+ </li>
+ </ul>
+ </div>
+ </div>
+
val accumulableHeaders: Seq[String] = Seq("Accumulable", "Value")
def accumulableRow(acc: AccumulableInfo) = <tr><td>{acc.name}</td><td>{acc.value}</td></tr>
val accumulableTable = UIUtils.listingTable(accumulableHeaders, accumulableRow,
accumulables.values.toSeq)
- val taskHeaders: Seq[String] =
+ val taskHeadersAndClasses: Seq[(String, String)] =
--- End diff --
Can you add a // comment here to explain what "classes" means?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/2867#discussion_r19494293
--- Diff: core/src/main/scala/org/apache/spark/ui/ToolTips.scala ---
@@ -31,4 +31,16 @@ private[spark] object ToolTips {
val SHUFFLE_READ =
"""Bytes read from remote executors. Typically less than shuffle write bytes
because this does not include shuffle data read locally."""
+
+ val GETTING_RESULT_TIME =
+ """Time that the driver spends fetching task results from workers. If this is large, consider
+ decreasing the amount of data returned from each task."""
+
+ val RESULT_SERIALIZATION_TIME =
+ """Time spent serializing the task result on the executor before sending it back to the
+ driver."""
+
+ val GC_TIME =
+ """Time that the executor spent paused for Java garbage collection while the task was
--- End diff --
spends? Elsewhere we use present tense
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60166300
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22043/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/2867#discussion_r19494160
--- Diff: core/src/main/resources/org/apache/spark/ui/static/table.js ---
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* Adds background colors to stripe table rows. This is necessary (instead of using css or the
+ * table striping provided by bootstrap) to appropriately stripe tables with hidden rows. */
+function stripeTables(){
--- End diff --
space before {, though I don't think we run style checks on JS
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60000885
I haven't done a close review yet, but I think it looks nicer if the boxes are listed vertically and the whole "Show additional metrics" section is collapsible.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61183923
Also, @JoshRosen, thanks for trying this out!!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60166292
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22043/consoleFull) for PR 2867 at commit [`1e3504d`](https://github.com/apache/spark/commit/1e3504d6a34aa508453b102afccd02b9de1dfcd8).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61014358
I downloaded this and tried it out. It looks good to me, except for a few minor usability things:
- I should be able to click on either the checkbox _or the checkbox label_ to expand / hide a metric. I'm pretty sure that there's some standard HTML way to group labels and input elements together so that clicking the label focuses or toggles the field. The larger click target makes this easier to use / more accessible.
- I'd place the arrow next to "Show additional metrics" inside of the link / header so that it's also clickable.
These are really minor nitpicks, though, although the first one could make a big usability difference for users who have trouble clicking small things.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61182104
@JoshRosen thanks for pointing that out -- I did indeed fail to push! I rebased and pushed a new version that also incorporates your two usability comments. Unfortunately the simple label fix doesn't work because of the javascript associated with clicking, but I figured out a pretty simple way to do this with a modest amount of javascript (let me know if you see a better way!).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60811631
Thanks Andrew! I'm happy to make more "additional" but all the rest seemed
like things people would want by default? Did you have things in mind?
On Tue, Oct 28, 2014 at 11:55 AM, andrewor14 <no...@github.com>
wrote:
> Hey @kayousterhout <https://github.com/kayousterhout> this LGTM other
> than a few very minor comments, though I haven't looked at the JS / CSS
> code in detail. Do we plan to make more of the existing columns "additional
> columns"? Is this just a starting point?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/spark/pull/2867#issuecomment-60811266>.
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61296742
Thanks for updating this. This looks good, so I'm going to merge it. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/2867#discussion_r19572994
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -65,55 +65,100 @@ private[ui] class StagePage(parent: JobProgressTab) extends WebUIPage("stage") {
<strong>Total task time across all tasks: </strong>
{UIUtils.formatDuration(stageData.executorRunTime)}
</li>
- {if (hasInput)
+ {if (hasInput) {
<li>
<strong>Input: </strong>
{Utils.bytesToString(stageData.inputBytes)}
</li>
- }
- {if (hasShuffleRead)
+ }}
+ {if (hasShuffleRead) {
<li>
<strong>Shuffle read: </strong>
{Utils.bytesToString(stageData.shuffleReadBytes)}
</li>
- }
- {if (hasShuffleWrite)
+ }}
+ {if (hasShuffleWrite) {
<li>
<strong>Shuffle write: </strong>
{Utils.bytesToString(stageData.shuffleWriteBytes)}
</li>
- }
- {if (hasBytesSpilled)
- <li>
- <strong>Shuffle spill (memory): </strong>
- {Utils.bytesToString(stageData.memoryBytesSpilled)}
- </li>
- <li>
- <strong>Shuffle spill (disk): </strong>
- {Utils.bytesToString(stageData.diskBytesSpilled)}
- </li>
- }
+ }}
+ {if (hasBytesSpilled) {
+ <li>
+ <strong>Shuffle spill (memory): </strong>
+ {Utils.bytesToString(stageData.memoryBytesSpilled)}
+ </li>
+ <li>
+ <strong>Shuffle spill (disk): </strong>
+ {Utils.bytesToString(stageData.diskBytesSpilled)}
+ </li>
+ }}
</ul>
</div>
- // scalastyle:on
+
+ val showAdditionalMetrics =
+ <div>
+ <span class="expand-additional-metrics-arrow arrow-closed"></span>
+ <span class="expand-additional-metrics"><strong>Show additional metrics</strong></span>
+ <div class="additional-metrics collapsed">
+ <ul style="list-style-type:none">
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.SCHEDULER_DELAY} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.SCHEDULER_DELAY}/>
+ Scheduler Delay
+ </span>
+ </li>
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.GC_TIME} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.GC_TIME}/>
+ GC Time
+ </span>
+ </li>
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.RESULT_SERIALIZATION_TIME} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.RESULT_SERIALIZATION_TIME}/>
+ Result Serialization Time
+ </span>
+ </li>
+ <li>
+ <span data-toggle="tooltip"
+ title={ToolTips.GETTING_RESULT_TIME} data-placement="right">
+ <input type="checkbox" name={TaskDetailsClassNames.GETTING_RESULT_TIME}/>
+ Getting Result Time
+ </span>
+ </li>
+ </ul>
+ </div>
+ </div>
+
val accumulableHeaders: Seq[String] = Seq("Accumulable", "Value")
def accumulableRow(acc: AccumulableInfo) = <tr><td>{acc.name}</td><td>{acc.value}</td></tr>
val accumulableTable = UIUtils.listingTable(accumulableHeaders, accumulableRow,
accumulables.values.toSeq)
- val taskHeaders: Seq[String] =
+ val taskHeadersAndClasses: Seq[(String, String)] =
--- End diff --
Fixed this to just have a more descriptive variable name (taskHeadersAndCssClasses)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by ash211 <gi...@git.apache.org>.
Github user ash211 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60028659
The first thing I can imagine doing if I start looking at these is to just check them all. Thoughts to making that operation easier, with say a check-all button?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60156671
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22043/consoleFull) for PR 2867 at commit [`1e3504d`](https://github.com/apache/spark/commit/1e3504d6a34aa508453b102afccd02b9de1dfcd8).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61151205
@kayousterhout You mentioned upthread that you fixed some of Andrew's style comments, but I don't see those changes; did you forget to push your commit?
You shouldn't block on my latest UX comments; I can fix them myself in a subsequent UI cleanup PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60496111
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22220/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61182588
[Test build #22567 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22567/consoleFull) for PR 2867 at commit [`6015913`](https://github.com/apache/spark/commit/6015913b1594f3f0868d261bbf7c84e414cd8b93).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-61010503
Yeah makes sense. Anything to add @JoshRosen?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout closed the pull request at:
https://github.com/apache/spark/pull/2867
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/2867#discussion_r19494527
--- Diff: core/src/main/scala/org/apache/spark/ui/UIUtils.scala ---
@@ -247,22 +250,32 @@ private[spark] object UIUtils extends Logging {
}
val colWidth = 100.toDouble / headers.size
val colWidthAttr = if (fixedWidth) colWidth + "%" else ""
- val headerRow: Seq[Node] = {
- // if none of the headers have "\n" in them
- if (headers.forall(!_.contains("\n"))) {
- // represent header as simple text
- headers.map(h => <th width={colWidthAttr}>{h}</th>)
+
+ def getClass(index: Int): String = {
+ if (index < headerClasses.size) {
+ headerClasses(index)
} else {
- // represent header text as list while respecting "\n"
- headers.map { case h =>
- <th width={colWidthAttr}>
- <ul class ="unstyled">
- { h.split("\n").map { case t => <li> {t} </li> } }
- </ul>
- </th>
- }
+ ""
+ }
+ }
+
+ val newlinesInHeader = headers.exists(_.contains("\n"))
--- End diff --
Can you just inline this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60700291
I'll take a look tonight if not early tomorrow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-59872487
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21952/consoleFull) for PR 2867 at commit [`e989560`](https://github.com/apache/spark/commit/e989560562b473624159e4e3554ec9898884a247).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60301065
+1 to some description. I still don't know what scheduler delay actually means.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60296814
@andrewor14 I can add tooltips to each metric name, or just list it in line (e.g., Scheduler Delay: A metric that explains ....). Which do you think is better?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
GitHub user kayousterhout reopened a pull request:
https://github.com/apache/spark/pull/2867
[SPARK-4016] Allow user to show/hide UI metrics.
This commit adds a set of checkboxes to the stage detail
page that the user can use to show additional task metrics,
including the GC time, result serialization time, result fetch
time, and scheduler delay. All of these metrics are now
hidden by default. This allows advanced users to look at more
detailed metrics, without distracting the average user.
This change also cleans up the stage detail page so that metrics
are shown in the same order in the summary table as in the task table,
and updates the metrics in both tables such that they contain the same
set of metrics.
The ability to remember a user's preferences for which metrics
should be shown has been filed as SPARK-4024.
Here's what the stage detail page looks like by default:
and once a user clicks "Show additional metrics" (note that all the metrics get checked by default):
cc @shivaram @andrewor14
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kayousterhout/spark-1 SPARK-checkboxes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/2867.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2867
----
commit e989560562b473624159e4e3554ec9898884a247
Author: Kay Ousterhout <ka...@gmail.com>
Date: 2014-10-21T01:30:38Z
[SPARK-4016] Allow user to show/hide UI metrics.
This commit adds a set of checkboxes to the stage detail
page that the user can use to show additional task metrics,
including the GC time, result serialization time, result fetch
time, and scheduler delay. All of these metrics are now
hidden by default. This allows advanced users to look at more
detailed metrics, without distracting the average user.
This change also cleans up the stage detail page so that metrics
are shown in the same order in the summary table as in the task table,
and updates the metrics in both tables such that they contain the same
set of metrics.
The ability to remember a user's preferences for which metrics
should be shown has been filed as SPARK-4024.
commit 1e3504d6a34aa508453b102afccd02b9de1dfcd8
Author: Kay Ousterhout <ka...@gmail.com>
Date: 2014-10-22T20:54:41Z
Made list of add'l metrics collapsible.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-59868203
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21952/consoleFull) for PR 2867 at commit [`e989560`](https://github.com/apache/spark/commit/e989560562b473624159e4e3554ec9898884a247).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60301451
@aarondav I wonder if I should change scheduler delay to "task launch overhead" or similar. It's (time when schedule receives task completion - time from when scheduler launches task) - executor run time -- so basically the extra overhead introduced by the scheduler. I'm no
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60811924
Actually one other thing we might want is a `De/select all` checkbox that I think @ash211 suggested. It would be useful if the user just doesn't care about any of these metrics and want their UIs to look more compact.
> I'm happy to make more "additional" but all the rest seemed
like things people would want by default? Did you have things in mind?
I was thinking accumulators. It seems to be empty most of the time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-60041059
@ash211 that's a great idea, I'll add that and incorporate @andrewor14's suggestion about a drop-down menu. Thanks all!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4016] Allow user to show/hide UI metric...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/2867#issuecomment-62094500
That's a good point and something I've found too. What do you think is the most usable way to change this?
One idea is for the "Show additional metrics" / little arrow to only expand and collapse the options, and then have a separate "select all" box at the top to choose all of the metrics. The issue with that is that it still requires two clicks for someone to enable everything.
A second idea, which seems perhaps preferable to me, is to have it say "Show additional metrics (show all)", with the show all in grey. And then if you click the "(show all)" part, it expands the options and selects all of them. Is that too confusing? What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org