linzebing commented on a change in pull request #27223:
[SPARK-30511][SPARK-28403][CORE] Don't treat failed/killed speculative tasks as
pending in Spark scheduler
URL: https://github.com/apache/spark/pull/27223#discussion_r370537529
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -614,18 +614,24 @@ private[spark] class ExecutorAllocationManager(
stageAttemptToNumRunningTask -= stageAttempt
}
}
- // If the task failed, we expect it to be resubmitted later. To ensure
we have
- // enough resources to run the resubmitted task, we need to mark the
scheduler
- // as backlogged again if it's not already marked as such (SPARK-8366)
- if (taskEnd.reason != Success) {
- if (totalPendingTasks() == 0) {
- allocationManager.onSchedulerBacklogged()
- }
- if (taskEnd.taskInfo.speculative) {
- stageAttemptToSpeculativeTaskIndices.get(stageAttempt).foreach
{_.remove(taskIndex)}
- } else {
- stageAttemptToTaskIndices.get(stageAttempt).foreach
{_.remove(taskIndex)}
- }
+
+ if (taskEnd.taskInfo.speculative) {
+ stageAttemptToSpeculativeTaskIndices.get(stageAttempt).foreach
{_.remove{taskIndex}}
+ stageAttemptToNumSpeculativeTasks(stageAttempt) -= 1
Review comment:
When a speculative task fails while the original task is running, it will
not be resubmitted (because we have one copy running, see
https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala#L289-L293),
but a new speculative task will be launched later in a future speculation
check cycle. The code works correctly in this case, as we update both
`stageAttemptToSpeculativeTaskIndices` and `stageAttemptToNumSpeculativeTasks`,
so this failed speculative task will just behave as it never existed. I can add
this to my test case.
@tgravescs, @jiangxb1987: Note that the current design (with or without this
PR) will launch more-than-needed executors when **the original task fails when
the speculative task is running**. In this case, the original task will not be
resubmitted (same as above because we have one copy running). Speculation check
will later launch one more speculative task on the already running speculative
task. In this case, **we have 2 speculative tasks for the same task index**. In
this case, we will still launch more than needed executors.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
