[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/9154 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-150049165 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44093/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-150049161 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-150048853 **[Test build #44093 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44093/consoleFull)** for PR 9154 at commit [`4394f00`](https://github.com/apache/spark/commit/4394f003f8950900eec1e27972083a2cf03f838b). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-150025058 **[Test build #44093 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44093/consoleFull)** for PR 9154 at commit [`4394f00`](https://github.com/apache/spark/commit/4394f003f8950900eec1e27972083a2cf03f838b). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-150024356 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-150024378 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user kayousterhout commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-150024132 Jenkins, 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-150009626 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44079/ Test FAILed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-150009623 Merged build finished. Test FAILed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-150009550 **[Test build #44079 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44079/consoleFull)** for PR 9154 at commit [`4394f00`](https://github.com/apache/spark/commit/4394f003f8950900eec1e27972083a2cf03f838b). * This patch **fails Spark unit 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-149984317 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-149985382 **[Test build #44079 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44079/consoleFull)** for PR 9154 at commit [`4394f00`](https://github.com/apache/spark/commit/4394f003f8950900eec1e27972083a2cf03f838b). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-149984278 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user markhamstra commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-149669055 This 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user markhamstra commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-149667908 > what's the point of keeping a pending list for a dead executor? I haven't done enough code spelunking to know, but I'm wondering whether the pending list for a dead executor may become useful for "I'm not dead" executors. [An extended Monty Python quote is very tempting, and surprisingly relevant, at this point.] It's entirely possible for executors to miss heartbeats (usually because they have ingested some bad user code that is now consuming most of their CPU resources) and appear dead to the master, only to arise from the dead a short time later (and potentially promptly miss another heartbeat.) If the before-you-were-dead pending task lists are or should be reattached to the Lazurus executors, then there potentially is a point to keeping them around. On the other hand, the "Ah, thank you very much" approach of clubbing the "not dead" is often what ends up needing to be done manually for these executors that refuse to die, so trying to complete their resurrection and make them useful again may be a fool's errand regardless. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user markhamstra commented on a diff in the pull request:
https://github.com/apache/spark/pull/9154#discussion_r42534219
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala ---
@@ -783,18 +783,6 @@ private[spark] class TaskSetManager(
/** Called by TaskScheduler when an executor is lost so we can
re-enqueue our tasks */
override def executorLost(execId: String, host: String, reason:
ExecutorLossReason) {
-logInfo("Re-queueing tasks for " + execId + " from TaskSet " +
taskSet.id)
-
-// Re-enqueue pending tasks for this host based on the status of the
cluster. Note
-// that it's okay if we add a task to the same queue twice (if it had
multiple preferred
-// locations), because dequeueTaskFromList will skip already-running
tasks.
-for (index <- getPendingTasksForExecutor(execId)) {
- addPendingTask(index, readding = true)
-}
-for (index <- getPendingTasksForHost(host)) {
- addPendingTask(index, readding = true)
-}
--- End diff --
Ah, now I see that @CodingCat had the same 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user markhamstra commented on a diff in the pull request:
https://github.com/apache/spark/pull/9154#discussion_r42530729
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala ---
@@ -783,18 +783,6 @@ private[spark] class TaskSetManager(
/** Called by TaskScheduler when an executor is lost so we can
re-enqueue our tasks */
override def executorLost(execId: String, host: String, reason:
ExecutorLossReason) {
-logInfo("Re-queueing tasks for " + execId + " from TaskSet " +
taskSet.id)
-
-// Re-enqueue pending tasks for this host based on the status of the
cluster. Note
-// that it's okay if we add a task to the same queue twice (if it had
multiple preferred
-// locations), because dequeueTaskFromList will skip already-running
tasks.
-for (index <- getPendingTasksForExecutor(execId)) {
- addPendingTask(index, readding = true)
-}
-for (index <- getPendingTasksForHost(host)) {
- addPendingTask(index, readding = true)
-}
--- End diff --
After this diff, nothing is using the misspelled `readding` parameter
anymore, so we may as well drop that from `addPendingTask`.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user kayousterhout commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-149293450 Yeah you're right that there doesn't seem to be any point in keeping entries for dead executors. I'm guessing the assumption is that all this state is cleaned up after the stage finishes anyway, so it's not a big deal to have some extra state hanging around. But in any case, that's a separate bug. @markhamstra if you have time to look at this, it would be helpful to have one more set of eyes! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-149292119 Ok, I see. I think I was under the impression that the code was actually removing tasks from old pending lists, but that doesn't seem to be the case. That also looks like a bug in itself - what's the point of keeping a pending list for a dead executor? I can see keeping a pending list for empty hosts because of dynamic allocation, but even that sounds fishy (the per-host pending list can be re-created if an executor is ever started again on that host). So you're right, after reading the code again a few more times it does seem addPendingTask is redundant. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user kayousterhout commented on the pull request:
https://github.com/apache/spark/pull/9154#issuecomment-149289335
To give a specific example, suppose task t1 has preferred locations on
executor e1 (on host h1), e2 (also on host h1) and e3 (on host h2).
The data structures will look like:
pendingTasksForExecutor: {e1: t1, e2: t1, e3: t1}
pendingTasksForHost: {h1: t1, h2: t1}
We've agreed that the "addPendingTask" call is irrelevant for tasks that
are currently running (because addPendingTask is called for any running tasks
in handleFailedTask), so let's say t1 hasn't been run yet.
Now suppose executor e2 dies. We never remove any entries from
pendingTasksForExecutor or pendingTasksForHost (not in addPendingTask, nor
anyplace else, as far as I can tell; we still won't schedule things on the died
executor, because the TaskSetManager will never get a resource offer for it).
addPendingTask will "readd" entries for each of t1's preferred locations
(including the lost executor -- we don't check whether the executor is alive
when updating the map entries). However, all of these locations were already
added above, so this call has no effect.
Which part of this reasoning do you think is incorrect?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user kayousterhout commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-149287156 What do you mean by "fixed"? As I said in my earlier comment, I don't see how addPendingTasks fixes that, since addPendingTask doesn't remove anything from any mappings. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-149284732 > What do you mean about tasks being in the wrong list? Let's say you have a task whose locality preference includes "executor 1". Now "executor 1" dies. That task would be in the `pendingTasksForExecutor` list for that executor, and now that's wrong and it should be fixed. My understanding is that `addPendingTask` is achieving that 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user kayousterhout commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-149106963 @vanzin my understanding is that addPendingTask looks through each preferred location for that task, and adds the task to the lists for (1) the list of executors corresponding to that location, (2) the list of hosts corresponding to that location and (3) the list of racks corresponding to that location. For tasks that are not yet running, it seems like this calling this in executorLost should have no effect: the only difference from the previous time that addPendingTask was called (before the executor was lost) is that, before the executor was lost, the task would have been added to one additional list for the lost executor. What do you mean about tasks being in the wrong list? I see that there will be an entry (with a list of pending tasks) in pendingTaskForExecutor corresponding to an executor that is dead, but calling "addPendingTasks" never removes anything from any mappings, so that call doesn't fix that problem. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148881593 the code block starts from https://github.com/apache/spark/pull/9154/files#diff-bad3987c83bd22d46416d3dd9d208e76R790 seems fishy to me are we updating 'copiesRunning' for twice? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148881300 errrare we distinguish running and pending in TaskInfo?I really need to update my knowledge about scheduler code if no, the `handleFailedTask` is actually called within https://github.com/apache/spark/blob/8ac71d62d976bbfd0159cac6816dd8fa580ae1cb/core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala#L816 what it does is we `re-add` all tasks whose executorId equals to the ID of this just failed executor --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148881135 @CodingCat but `handleFailedTask` only calls `addPendingTask` for, as the name suggests, failed tasks. This code is re-scheduling *pending* tasks that haven't even begun execution yet. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148880182 @vanzin I think the key point is at https://github.com/apache/spark/blob/8ac71d62d976bbfd0159cac6816dd8fa580ae1cb/core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala#L821 and in handleFailedTask, we have called addPendingTask https://github.com/apache/spark/blob/8ac71d62d976bbfd0159cac6816dd8fa580ae1cb/core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala#L652 My suggestion is that maybe we shall remove `readding` parameter in addToList, as the removed lines in this patch are the only places where we are not using the default values... --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148878074 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148878028 [Test build #43868 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43868/console) for PR 9154 at commit [`26aa899`](https://github.com/apache/spark/commit/26aa8999a62eba632c670ba3dc57065f0d5ea4df). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148878075 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43868/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148870844 >they're still in all of the correct pending lists, so calling addPendingTask has no effect That's not really accurate, is it? `TaskSetManager.executorLost` is called by `TaskSchedulerImpl.executorLost` after it has updated the list of available executors and which hosts they live on; so if there are pending tasks for the executor and / or the host, they're currently in the wrong list, and the `addPendingTask` calls 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user kayousterhout commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148867654 @CodingCat @cmccabe you both also modified this code recently...does this change look reasonable to 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148866726 [Test build #43868 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43868/consoleFull) for PR 9154 at commit [`26aa899`](https://github.com/apache/spark/commit/26aa8999a62eba632c670ba3dc57065f0d5ea4df). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148865777 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9154#issuecomment-148865770 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11163] Remove unnecessary addPendingTas...
GitHub user kayousterhout opened a pull request: https://github.com/apache/spark/pull/9154 [SPARK-11163] Remove unnecessary addPendingTask calls. This commit removes unnecessary calls to addPendingTask in TaskSetManager.executorLost. These calls are unnecessary: for tasks that are still pending and haven't been launched, they're still in all of the correct pending lists, so calling addPendingTask has no effect. For tasks that are currently running (which may still be in the pending lists, depending on how they were scheduled), we call addPendingTask in handleFailedTask, so the calls at the beginning of executorLost are redundant. I think these calls are left over from when we re-computed the locality levels in addPendingTask; now that we call recomputeLocality separately, I don't think these are necessary. @markhamstra can you take a look at this? cc @vanzin You can merge this pull request into a Git repository by running: $ git pull https://github.com/kayousterhout/spark-1 SPARK-11163 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/9154.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 #9154 commit 26aa8999a62eba632c670ba3dc57065f0d5ea4df Author: Kay Ousterhout Date: 2015-10-16T23:32:00Z [SPARK-11163] Remove unnecessary addPendingTask calls. This commit removes unnecessary calls to addPendingTask in TaskSetManager.executorLost. These calls are unnecessary: for tasks that are still pending and haven't been launched, they're still in all of the correct pending lists, so calling addPendingTask has no effect. For tasks that are currently running (which may still be in the pending lists, depending on how they were scheduled), we call addPendingTask in handleFailedTask, so the calls at the beginning of executorLost are redundant. I think these calls are left over from when we re-computed the locality levels in addPendingTask; now that we call recomputeLocality separately, I don't think these are 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
