miaoever commented on code in PR #59229:
URL: https://github.com/apache/spark/pull/59229#discussion_r4176343526
##########
core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala:
##########
@@ -950,12 +968,10 @@ private[spark] class ExecutorAllocationManager(
// (SPARK-8366)
allocationManager.onSchedulerBacklogged()
}
- if (!taskEnd.taskInfo.speculative) {
- // If a non-speculative task is intentionally killed, it means
the speculative task
- // has succeeded, and no further task of this task index will be
resubmitted. In this
- // case, the task index is completed and we shouldn't remove it
from
- // stageAttemptToTaskIndices. Otherwise, we will have a pending
non-speculative task
- // for the task index (SPARK-30511)
+ if (!taskEnd.taskInfo.speculative &&
Review Comment:
@HyukjinKwon Added a regression test covering a non-killed regular-task
failure after its speculative copy succeeds. The test confirms the task index
remains completed with zero pending demand. Also verified that the test fails
when the successful-speculative guard is removed and passes with the guard
restored.
([ExecutorAllocationManagerSuite.scala](https://github.com/tmp/agentcloud-100000578760927-workspace/6be51178-a1c4-4043-9980-828e45c11858/spark/core/src/test/scala/org/apache/spark/ExecutorAllocationManagerSuite.scala:965),
line 965)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
