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/10/14 17:56:12 UTC
[GitHub] spark pull request: SPARK-2533 - Add locality levels (with tasks c...
GitHub user jbonofre opened a pull request:
https://github.com/apache/spark/pull/9117
SPARK-2533 - Add locality levels (with tasks count) on job stages summary
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jbonofre/spark SPARK-2533
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/9117.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 #9117
----
commit 805c7a218d0b1501499164d694ad86bb7ab4408b
Author: Jean-Baptiste Onofré <jb...@apache.org>
Date: 2015-10-14T15:54:52Z
SPARK-2533 - Add locality levels (with tasks count) on job stages summary
----
---
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 (with tasks c...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-149127284
Yeah this was my thought - could we provide a summary in the stage page rather than in the stage index. I do see how an aggregated summary is significantly more useful than just mentally aggregating based on the task table. Doing this on the stage page would avoid adding more columns to the index view, and this column could get complicated if stages have many locality levels in play. It might be nice to see an alternative patch that does 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 (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/9117#discussion_r43668404
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -122,6 +123,24 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
val externalAccumulables = allAccumulables.values.filter { acc => !acc.internal }
val hasAccumulators = externalAccumulables.size > 0
+ val tasksLocality = new mutable.HashMap[String, Long]
+ if (stageData.taskData.size > 0) {
+ for ((key, value) <- stageData.taskData) {
+ val locality = value.taskInfo.taskLocality.toString;
--- End diff --
you can re-write the next few lines as:
tasksLocality.put(locality, tasksLocality.getOrElse(locality, 0) + 1)
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153962539
Crap I did a mistake, let me clean it up.
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-148125649
Sure, give me 2 minutes ;)
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-149254765
Updated PR by moving locality level summary on stage details.
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153970081
Unfortunately, github says the branch has been recreated or forced-pushed :/
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre closed the pull request at:
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 (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/9117#discussion_r43668168
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -122,6 +123,24 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
val externalAccumulables = allAccumulables.values.filter { acc => !acc.internal }
val hasAccumulators = externalAccumulables.size > 0
+ val tasksLocality = new mutable.HashMap[String, Long]
+ if (stageData.taskData.size > 0) {
+ for ((key, value) <- stageData.taskData) {
--- End diff --
Can you change this to stageData.taskData.values.foreach { taskUiData => .... }
---
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 (with tasks c...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-148097686
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153970579
@kayousterhout I'm very sorry about my mistake (I worked on another project in the same time and mixed the git actions :(, probably too early on the morning for me ;)).
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153969839
OK no problem. I thought you would prefer a new clean one. My bad and sorry about 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 (with tasks c...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-148297180
cc @pwendell @kayousterhout thoughts on 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-2533 - Add locality levels (with tasks c...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-149125876
ping @kayousterhout to me it's the normal issue - sure it's useful for some cases, but is it worth putting in the index page? It could be better to just put a locality summary on the stage page itself, if one doesn't already exist.
---
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 (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/9117#discussion_r43667755
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -122,6 +123,24 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
val externalAccumulables = allAccumulables.values.filter { acc => !acc.internal }
val hasAccumulators = externalAccumulables.size > 0
+ val tasksLocality = new mutable.HashMap[String, Long]
--- End diff --
Can you move all of this into a "getLocalitySummaryString" function or similar?
---
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 (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/9117#discussion_r43667980
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -122,6 +123,24 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
val externalAccumulables = allAccumulables.values.filter { acc => !acc.internal }
val hasAccumulators = externalAccumulables.size > 0
+ val tasksLocality = new mutable.HashMap[String, Long]
+ if (stageData.taskData.size > 0) {
--- End diff --
Is this outer if-statement necessary?
---
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 (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/9117#discussion_r43667618
--- 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
--- End diff --
Can you directly import mutable.HashMap here? That's more consistent with style elsewhere
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153371909
I rebased and updated the PR to improve the code according to Kay's comments.
Now, I'm starting to implement the two Kay's suggestions.
@kayousterhout thanks again for your support !
---
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 (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/9117#discussion_r43668782
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala ---
@@ -19,6 +19,7 @@ package org.apache.spark.ui.jobs
import java.util.Date
+import scala.collection.mutable.{HashMap}
--- End diff --
remove 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-2533 - Add locality levels (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153127544
Two 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.
---
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 (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/9117#discussion_r43668261
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -122,6 +123,24 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
val externalAccumulables = allAccumulables.values.filter { acc => !acc.internal }
val hasAccumulators = externalAccumulables.size > 0
+ val tasksLocality = new mutable.HashMap[String, Long]
--- End diff --
rename to localityCounts or localityLevelToNumTasks?
---
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 (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153966311
Would you mind re-opening this and closing the new one? You can change the code here to be equivalent to the code in your new branch by doing "git push --force origin SPARK-2533-2:SPARK-2533". It's easier for us to track things if we keep the history in one pull request.
As an aside, when you're working on pull requests, it's helpful if you implement changes as new commits on top of your original one. That way, reviewers can review the commits in the history to look at just what's changed since the last review. When we merge things into Spark, we squash it all into one commit, so using a bunch of commits in a pull request won't pollute the git history later 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-2533 - Add locality levels (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-148126672
Note the "new" Locality Levels column in the stage summary view. The format is:
locality_level(number_of_tasks)
For instance NODE_LOCAL(1), means one task in this stage with NODE_LOCAL locality level.
---
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 (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153973135
No problem -- at this point, just do whatever's easiest to get a pull request that's in an ok state!
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153964256
Let me close this PR and create a new one (easier to read).
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-149128826
+1, let me update this PR according to @pwendell comment. Give me 10mn ;) Thanks Patrick !
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-152997918
Gently reminder ... 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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-148126436
---
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 (with tasks c...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-148116353
Can you post a screenshot for the change?
---
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 (with tasks c...
Posted by kayousterhout <gi...@git.apache.org>.
Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/9117#discussion_r43668740
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -122,6 +123,24 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
val externalAccumulables = allAccumulables.values.filter { acc => !acc.internal }
val hasAccumulators = externalAccumulables.size > 0
+ val tasksLocality = new mutable.HashMap[String, Long]
+ if (stageData.taskData.size > 0) {
+ for ((key, value) <- stageData.taskData) {
+ val locality = value.taskInfo.taskLocality.toString;
+ if (tasksLocality.contains(locality)) {
+ val count: Long = tasksLocality.get(locality).get + 1;
+ tasksLocality.put(locality, count);
+ } else {
+ tasksLocality.put(locality, 1);
+ }
+ }
+ }
+ var localityLevels: String = "";
--- End diff --
How about re-writing this as: (all of the concat statements are tough to read)
tasksLocality.map { _ match {
case (localityLevel, count) =>
s"$localityLevel ($count)"
}}.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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153983273
Thanks @kayousterhout. I promise it's my last mistake ;)
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-149126639
@pwendell the locality summary is on the stages page. I can also do a locality summary on the view of each stage (we have it in the tasks table, but a summary with the tasks count for each locality is helpful IMHO).
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153970157
And yes, I did a mistake (merge instead of rebase).
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-149254629
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on a diff in the pull request:
https://github.com/apache/spark/pull/9117#discussion_r43668918
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -122,6 +123,24 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
val externalAccumulables = allAccumulables.values.filter { acc => !acc.internal }
val hasAccumulators = externalAccumulables.size > 0
+ val tasksLocality = new mutable.HashMap[String, Long]
+ if (stageData.taskData.size > 0) {
+ for ((key, value) <- stageData.taskData) {
+ val locality = value.taskInfo.taskLocality.toString;
--- End diff --
@kayousterhout Thanks so much for all your feedbacks !!! Much appreciated. Let me update the PR accordingly.
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153962461
Sorry for the noise
---
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 (with tasks c...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/9117#issuecomment-153962443
Rebase
---
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