You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jbonofre <gi...@git.apache.org> on 2015/11/05 06:57:14 UTC
[GitHub] spark pull request: SPARK-2533 - Add locality levels on stage summ...
GitHub user jbonofre opened a pull request:
https://github.com/apache/spark/pull/9487
SPARK-2533 - Add locality levels on stage summary view
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jbonofre/spark SPARK-2533-2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/9487.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 #9487
----
commit f7cf9689fc4219ac9ab59df2d3cbbd875bcd69be
Author: Jean-Baptiste Onofré <jb...@apache.org>
Date: 2015-11-05T05:56:37Z
SPARK-2533 - Add locality levels on stage summary view
----
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44224075
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -177,6 +192,10 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
<div class="additional-metrics collapsed">
<ul>
<li>
+ <strong>Locality Level Summary: </strong>
--- End diff --
What do you think about a new table "Locality Levels Summary" displayed by a checkbox in additional metric ?
---
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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44224792
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -177,6 +192,10 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
<div class="additional-metrics collapsed">
<ul>
<li>
+ <strong>Locality Level Summary: </strong>
--- End diff --
Yea, it didn't register to me that this was adding to the list of additional metrics enabled by checkboxes. If it goes here, I think it would have to be treated the same way. If a table displays well, that's reasonable to me. It might be harder to make render though; could be overkill at this stage. @kayousterhout has better sensibilities here.
---
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-2533 - Add locality levels on stage summ...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44064427
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -28,7 +28,7 @@ import org.apache.commons.lang3.StringEscapeUtils
import org.apache.spark.{InternalAccumulator, SparkConf}
import org.apache.spark.executor.TaskMetrics
-import org.apache.spark.scheduler.{AccumulableInfo, TaskInfo}
+import org.apache.spark.scheduler.{TaskLocality, AccumulableInfo, TaskInfo}
--- End diff --
alphabetize imports (so import org.apache.spark.scheduler.{AccumulableInfo, TaskInfo, TaskLocality}
---
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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44003668
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -70,6 +71,17 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
private val displayPeakExecutionMemory = parent.conf.getBoolean("spark.sql.unsafe.enabled", true)
+ private def getLocalitySummaryString(stageData: StageUIData): String = {
+ val localityCounts = new HashMap[String, Long]
+ stageData.taskData.values.foreach( taskUIData => {
--- End diff --
And while picking nits, how about mapping to (key,value) tuples and calling `toMap` at the end instead of making a `Map` Java-style?
---
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-2533 - Add locality levels on stage summ...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155670350
I'd prefer your original approach to this, because in either case, it takes up one line of text in the default view, but with this, it takes up one line of text *and* you have to click on that line to see the metrics.
@andrewor14 @srowen what about putting the summary ("Locality summary: process local: 3 tasks; dode local: 6 tasks; ..." under the "Summary metrics" heading and above the table (and always displayed)?
@jbonofre might be a good idea to hold off on changing the code until there's consensus on the right approach
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44032740
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -70,6 +71,33 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
private val displayPeakExecutionMemory = parent.conf.getBoolean("spark.sql.unsafe.enabled", true)
+ private def getLocalitySummaryString(stageData: StageUIData): String = {
+ val localityCounts = new HashMap[String, Long]
+ localityCounts.put("Process local", 0L);
+ localityCounts.put("Node local", 0L);
+ localityCounts.put("Rack local", 0L);
+ localityCounts.put("Any", 0L);
+ stageData.taskData.values.foreach( taskUIData => {
+ taskUIData.taskInfo.taskLocality match {
+ case TaskLocality.PROCESS_LOCAL => {
+ localityCounts.put("Process local", localityCounts.getOrElse("Process local", 0L) + 1)
+ }
+ case TaskLocality.NODE_LOCAL => {
+ localityCounts.put("Node local", localityCounts.getOrElse("Node local", 0L) + 1)
+ }
+ case TaskLocality.RACK_LOCAL => {
+ localityCounts.put("Rack local", localityCounts.getOrElse("Rack local", 0L) + 1)
+ }
+ case TaskLocality.ANY => {
+ localityCounts.put("Any", localityCounts.getOrElse("Any", 0L) + 1)
+ }
+ }
+ })
+ return localityCounts.map { _ match {
+ case (localityLevel, count) => s"$localityLevel: $count task(s)"
+ }}.mkString("; ")
+ }
+
--- End diff --
Fair enough, let me try. Thanks again Sean, I appreciate all your help, support and guidance !
---
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-2533 - Add locality levels on stage summ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156187649
Merged build started.
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-154188536
PR rebased and updated with alphabetize import thanks to @kayousterhout suggestion (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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-154327787
---
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-2533 - Add locality levels on stage summ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156190193
**[Test build #45752 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45752/consoleFull)** for PR 9487 at commit [`bd7de49`](https://github.com/apache/spark/commit/bd7de491171cdea9a585c68168f9743c41cc4b3f).
---
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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44223623
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -177,6 +192,10 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
<div class="additional-metrics collapsed">
<ul>
<li>
+ <strong>Locality Level Summary: </strong>
--- End diff --
Hm, actually on final thought, is this the right place to show this info? it's dumped into a list of metrics, but all the others are controlled by arrows and checkboxes.
---
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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44016799
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -70,6 +71,33 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
private val displayPeakExecutionMemory = parent.conf.getBoolean("spark.sql.unsafe.enabled", true)
+ private def getLocalitySummaryString(stageData: StageUIData): String = {
+ val localityCounts = new HashMap[String, Long]
+ localityCounts.put("Process local", 0L);
+ localityCounts.put("Node local", 0L);
+ localityCounts.put("Rack local", 0L);
+ localityCounts.put("Any", 0L);
+ stageData.taskData.values.foreach( taskUIData => {
+ taskUIData.taskInfo.taskLocality match {
+ case TaskLocality.PROCESS_LOCAL => {
+ localityCounts.put("Process local", localityCounts.getOrElse("Process local", 0L) + 1)
+ }
+ case TaskLocality.NODE_LOCAL => {
+ localityCounts.put("Node local", localityCounts.getOrElse("Node local", 0L) + 1)
+ }
+ case TaskLocality.RACK_LOCAL => {
+ localityCounts.put("Rack local", localityCounts.getOrElse("Rack local", 0L) + 1)
+ }
+ case TaskLocality.ANY => {
+ localityCounts.put("Any", localityCounts.getOrElse("Any", 0L) + 1)
+ }
+ }
+ })
+ return localityCounts.map { _ match {
+ case (localityLevel, count) => s"$localityLevel: $count task(s)"
+ }}.mkString("; ")
+ }
+
--- End diff --
This might result in inconsistent order of output and always shows 0 counts, which may or may not be desirable. How about this which avoids some of the repetition:
```
val localities = stageData.taskData.values.map(_.taskInfo.taskLocality)
val localityCounts = localities.groupBy(identity).mapValues(_.size)
val localityNamesAndCounts = localityCounts.toSeq.map { case (locality, count) =>
val localityName = locality match {
case TaskLocality.PROCESS_LOCAL => "Process local"
case TaskLocality.NODE_LOCAL => "Node local"
case TaskLocality.RACK_LOCAL => "Rack local"
case TaskLocality.ANY => "Any"
}
(localityName, count)
}
localityNamesAndCounts.sorted.mkString("; ")
```
---
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-2533 - Add locality levels on stage summ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-154003441
**[Test build #1985 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/1985/consoleFull)** for PR 9487 at commit [`f7cf968`](https://github.com/apache/spark/commit/f7cf9689fc4219ac9ab59df2d3cbbd875bcd69be).
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155799886
You are right: it's more related to tasks. And yes, it's valid for both completed and active tasks.
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44003767
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -70,6 +71,17 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
private val displayPeakExecutionMemory = parent.conf.getBoolean("spark.sql.unsafe.enabled", true)
+ private def getLocalitySummaryString(stageData: StageUIData): String = {
+ val localityCounts = new HashMap[String, Long]
+ stageData.taskData.values.foreach( taskUIData => {
+ val locality = taskUIData.taskInfo.taskLocality.toString
+ localityCounts.put(locality, localityCounts.getOrElse(locality, 0L) + 1)
+ })
+ return localityCounts.map { _ match {
--- End diff --
Correct Sean. Thanks ! Actually, I'm implementing second Kay's suggestion (using "Advanced Metrics" checkbox). I will update the 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-2533 - Add locality levels on stage summ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156272972
LGTM merging into master 1.6, 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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-153965376
@kayousterhout suggestions:
1) can we change the format of this to say "Process local: 2 tasks; Rack local: 4 tasks; Any: 15 tasks"? I think that's easier to read and more clear what it is.
2) I wonder if we should put this under the "Show additional metrics" drop down. Right now, it's one of the first things someone will see when they open the UI, and it's not clear to me that that's the right prioritization.
I'm updating there.
---
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-2533 - Add locality levels on stage summ...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44075195
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -70,6 +70,21 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
private val displayPeakExecutionMemory = parent.conf.getBoolean("spark.sql.unsafe.enabled", true)
+ private def getLocalitySummaryString(stageData: StageUIData): String = {
+ val localities = stageData.taskData.values.map(_.taskInfo.taskLocality)
+ val localityCounts = localities.groupBy(identity).mapValues(_.size)
+ val localityNamesAndCounts = localityCounts.toSeq.map { case (locality, count) =>
+ val localityName = locality match {
+ case TaskLocality.PROCESS_LOCAL => "Process local"
+ case TaskLocality.NODE_LOCAL => "Node local"
+ case TaskLocality.RACK_LOCAL => "Rack local"
+ case TaskLocality.ANY => "Any"
+ }
+ (localityName, count)
--- End diff --
It looks like this prints as "(Process local, 3); (Rack local, 5)"? Am I understanding that correctly? If so, can you replace line 83 with s"$localityName: $count" to get a nicer looking format?
---
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-2533 - Add locality levels on stage summ...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155855935
Good point re: the summary metrics being only for completed tasks. I think it's a little weird to put the summary metric under the "tasks" table though; what about the original proposal of putting it near the top, under "total 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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155666778
PR rebased and updated with a collapsed div. Please, see the following screenshots:
Default stage view:
When user expands the "locality level summary" div:
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155545104
Correct Kay, it's the way that I did at the beginning.
I'm proposing:
- display under the total time as I did at the beginning
- add a "Locality Levels Summary" expanded div (like "Additional Metric"), collapsed by default
Thoughts ?
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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-154331487
LGTM
---
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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156154306
OK that seems pretty good.
---
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-2533 - Add locality levels on stage summ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-153965272
Can one of the admins verify this patch?
---
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-2533 - Add locality levels on stage summ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155859656
I agree, we should just put it under total time. The KV pairs at the top of the page are for summary info aggregated across tasks and that's exactly what this is. It looks clunky as a table and doesn't seem to be of equal importance to things like the event timeline.
---
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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44003608
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -70,6 +71,17 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
private val displayPeakExecutionMemory = parent.conf.getBoolean("spark.sql.unsafe.enabled", true)
+ private def getLocalitySummaryString(stageData: StageUIData): String = {
+ val localityCounts = new HashMap[String, Long]
+ stageData.taskData.values.foreach( taskUIData => {
+ val locality = taskUIData.taskInfo.taskLocality.toString
+ localityCounts.put(locality, localityCounts.getOrElse(locality, 0L) + 1)
+ })
+ return localityCounts.map { _ match {
--- End diff --
I think you can omit `return` and the `_ match` since you have just one `case`?
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-153984694
Reference to the "old" PR: https://github.com/apache/spark/pull/9117
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44106408
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -70,6 +70,21 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
private val displayPeakExecutionMemory = parent.conf.getBoolean("spark.sql.unsafe.enabled", true)
+ private def getLocalitySummaryString(stageData: StageUIData): String = {
+ val localities = stageData.taskData.values.map(_.taskInfo.taskLocality)
+ val localityCounts = localities.groupBy(identity).mapValues(_.size)
+ val localityNamesAndCounts = localityCounts.toSeq.map { case (locality, count) =>
+ val localityName = locality match {
+ case TaskLocality.PROCESS_LOCAL => "Process local"
+ case TaskLocality.NODE_LOCAL => "Node local"
+ case TaskLocality.RACK_LOCAL => "Rack local"
+ case TaskLocality.ANY => "Any"
+ }
+ (localityName, count)
--- End diff --
Good point, let me do 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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44225825
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -177,6 +192,10 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
<div class="additional-metrics collapsed">
<ul>
<li>
+ <strong>Locality Level Summary: </strong>
--- End diff --
Catcha. Let me try something around this for you.
---
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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44063765
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -21,6 +21,7 @@ import java.net.URLEncoder
import java.util.Date
import javax.servlet.http.HttpServletRequest
+import scala.collection.mutable.HashMap
--- End diff --
Looking good. Finally, I think this import is no longer needed.
---
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-2533 - Add locality levels on stage summ...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-154206133
Can you post the updated screenshot?
---
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-2533 - Add locality levels on stage summ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/9487
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-154070111
Updated PR implementing Kay's suggestions: formatting the locality level summary string, and moving under the "Show Additional Metrics" drop down.
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-154178581
PR rebased and updated according to Sean's suggestion
---
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-2533 - Add locality levels on stage summ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156221405
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/45752/
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156327610
Thanks ! Let me prepare new 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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44003621
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -70,6 +71,17 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
private val displayPeakExecutionMemory = parent.conf.getBoolean("spark.sql.unsafe.enabled", true)
+ private def getLocalitySummaryString(stageData: StageUIData): String = {
+ val localityCounts = new HashMap[String, Long]
+ stageData.taskData.values.foreach( taskUIData => {
--- End diff --
You can make the open ( a { and omit the other {
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155856959
+1 with @kayousterhout
---
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-2533 - Add locality levels on stage summ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156221235
**[Test build #45752 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45752/consoleFull)** for PR 9487 at commit [`bd7de49`](https://github.com/apache/spark/commit/bd7de491171cdea9a585c68168f9743c41cc4b3f).
* 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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on a diff in the pull request:
https://github.com/apache/spark/pull/9487#discussion_r44064054
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -21,6 +21,7 @@ import java.net.URLEncoder
import java.util.Date
import javax.servlet.http.HttpServletRequest
+import scala.collection.mutable.HashMap
--- End diff --
True !!! Let me fix 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-2533 - Add locality levels on stage summ...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155532078
@andrewor14 I think that was how @jbonofre originally did it, but the complaint with that approach was that it clutters the UI with information that may not be useful to many people. I'm torn here though because it is pretty small; I'm fine with just putting it under the total time if @srowen is OK with 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-2533 - Add locality levels on stage summ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156221402
Merged build finished. 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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155799204
Thanks, the picture helps me think about where this should go.
This seems most related to the "Tasks" section which also reports the locality of each task. Is it too random to stick this in as a line of text before or after that table?
Summary Metrics: could go here too. This is only for completed tasks though. The metrics are for active tasks too right?
---
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-2533 - Add locality levels on stage summ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-154038912
**[Test build #1985 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/1985/consoleFull)** for PR 9487 at commit [`f7cf968`](https://github.com/apache/spark/commit/f7cf9689fc4219ac9ab59df2d3cbbd875bcd69be).
* 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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155670928
@kayousterhout ok, let's wait feedback from the others. I will revert my commit, back on a single string always displayed.
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155868888
Let me revert the unecessary commits.
---
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-2533 - Add locality levels on stage summ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156187252
retest this please
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-154328189
PR rebased and implement Kay's suggestion (about the locality level summary string format)
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-154303891
Sure, let me prepare 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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155549560
I'm updating the PR with the collapsed table. I will attach a screenshot.
---
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-2533 - Add locality levels on stage summ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156187615
Merged build triggered.
---
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-2533 - Add locality levels on stage summ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155858987
Ha OK, full circle there. At least we've considered every possible position on the page. I'm OK with that. Maybe later there's a big redesign that re-sorts all the info here but for now it's no bad place to stick this info, logically or aesthetically.
---
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-2533 - Add locality levels on stage summ...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-156086317
PR rebased, locality levels summary on stage just under the time (as first proposal).
---
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-2533 - Add locality levels on stage summ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/9487#issuecomment-155531017
Hm, I find it a little weird that we put it under additional metrics. It's not really a metric and it's different from the rest of the things under that drop down. Should we just put it under `Total time across all tasks`?
---
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