j1wonpark commented on PR #57651:
URL: https://github.com/apache/spark/pull/57651#issuecomment-5486575786
@Ngone51 Sure. The negative value comes from `getPendingTaskSum`:
`stageAttemptToNumTasks.getOrElse(attempt, 0) -
stageAttemptToTaskIndices(attempt).size`. A late `TaskStart` can recreate
`stageAttemptToTaskIndices` for a stage attempt whose `stageAttemptToNumTasks`
entry was already removed, which makes the subtraction go below zero. Step by
step, with stage attempt S of 2 tasks (this is exactly the first regression
test):
1. `TaskStart` for task 0: `stageAttemptToNumRunningTask(S) = 1`,
`stageAttemptToTaskIndices(S) = {0}`.
2. Task 1 is launched right around stage completion, so its `TaskStart`
reaches the listener after `StageCompleted`. This is the same reordering that
the "do NOT remove stageAttempt from stageAttemptToNumRunningTask" comment in
`onStageCompleted` already accounts for on the `TaskEnd` side.
3. `StageCompleted(S)` removes `stageAttemptToNumTasks(S)` and
`stageAttemptToTaskIndices(S)`. S stays in `resourceProfileIdToStageAttempt`
because task 0 is still running.
4. The late `TaskStart` for task 1 (before this PR) unconditionally runs
`stageAttemptToTaskIndices.getOrElseUpdate(S, ...) += 1`, recreating the index
set for the completed attempt. From this point `getPendingTaskSum(S) = 0 - 1 =
-1`.
5. Both tasks then end with `Success`. The `Success` path never removes
entries from `stageAttemptToTaskIndices` (a finished index still counts as
started, which is what keeps `numTasks - indices.size` correct for live
stages), and when `stageAttemptToNumRunningTask(S)` drops to 0,
`removeStageFromResourceProfileIfUnused(S)` refuses to drop S because
`stageAttemptToTaskIndices.contains(S)` is now true. So the phantom entry, and
the -1, stay forever.
6. When the next stage with 1 pending task is submitted,
`pendingTasksPerResourceProfile` sums -1 + 1 = 0, so
`maxNumExecutorsNeededPerResourceProfile` returns 0. With `minExecutors=0` the
target never leaves 0 and no executors are requested, which is the production
hang we hit.
So the corruption is introduced by the late `TaskStart`, and the matching
`TaskEnd` is what makes it permanent by blocking the stage attempt cleanup. On
master the first regression test fails right at the
`pendingRegularTasksForDefaultProfile(manager) === 0` assertion, with -1 as the
actual value.
--
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]
