Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/17166#discussion_r107740806
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
@@ -467,7 +474,7 @@ private[spark] class TaskSchedulerImpl
private[scheduler](
taskState: TaskState,
reason: TaskFailedReason): Unit = synchronized {
taskSetManager.handleFailedTask(tid, taskState, reason)
- if (!taskSetManager.isZombie && taskState != TaskState.KILLED) {
+ if (!taskSetManager.isZombie) {
--- End diff --
@mridulm for (a), what evidence do you have that reviveOffers is costly and
the bottleneck in scheduling a large job? I agree that we're adding many
reviveOffers calls in the case of large jobs -- but for the other jobs I've
benchmarked in the past, I haven't seen this be a bottleneck (and when all of a
job's tasks have started running, reviveOffers should be very fast).
Re: (b), I did an extensive review of the associated scheduler code and
tasks *are* re-attempted (as was verified by Eric's test). As I mentioned
above, the only reason they're not re-attempted in the current uses of
TaskKilled is because either the stage has been killed (so the task set is
marked as a zombie) or because the task attempt has already succeeded elsewhere
(as for speculative tasks). Also the Mesos scheduler code uses TaskKilled in
the same way as this PR (where Mesos may kill a task that should be
re-scheduled elsewhere). If you don't think that killed tasks that haven't
succeeded elsewhere will be re-run, can you point to the specific code?
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
