sunchao commented on PR #58380: URL: https://github.com/apache/spark/pull/58380#issuecomment-5482352298
The previously reported StatefulSet registration issue is addressed in reviewed commit `1ac32efce4cb93c6ea31d6955af4d54f09ceede6`. Two P2 issues remain. **[P2] Count running speculative attempts, not distinct task indices** The new [getRunningSpeculativeTaskSum](https://github.com/apache/spark/blob/1ac32efce4cb93c6ea31d6955af4d54f09ceede6/core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala#L1073-L1074) returns the size of a set keyed by task index. After a primary attempt fails, its surviving speculative attempt S1 can itself be speculated: [TaskSetManager checks every running attempt](https://github.com/apache/spark/blob/1ac32efce4cb93c6ea31d6955af4d54f09ceede6/core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala#L1311-L1315), and the dequeue guard permits another speculative attempt when `copiesRunning(index) == 1`. S1 and S2 both have `speculative=true`, but their starts produce only one set entry. Ending either attempt also removes that entry while the other remains running. A concrete allocation consequence uses three task slots per executor and allocation ratio 1. Let a two-task result stage have one completed fast task and a slow primary P; two unrelated regular tasks from another stage use the same resource profile. Executor B runs those two regular tasks plus S1, the speculative copy of P. P's executor A fails, leaving one regular task pending, and replacement executor C can later launch S2 after S1 becomes speculatable again. While S2 is pending, the estimate is 2. Once S2 starts, there are still five running-or-pending attempts: one regular pending and four running, of which two are speculative. The accessor reports only one running speculative attempt, so the regular baseline becomes `ceil((1 + 4 - 1) / 3) = 2`. Since raw `maxNeeded` is also 2, the new branch raises the target to 3. Counting both speculative attempts gives a regular baseline of 1 and keeps the target at 2, as do the base and previous revision after launch. The regular pending entry keeps the backlog timer active, so this can request an unnecessary executor even though B and C already run all four attempts. Please track live speculative attempt IDs or equivalent multiplicities, and cover re-speculation after a primary failure. This finding is based on source inspection, not a local runtime reproduction. **[P2] Update the existing regression expectations for retained speculation** The existing `SPARK-41192` assertions at [lines 758-759](https://github.com/apache/spark/blob/1ac32efce4cb93c6ea31d6955af4d54f09ceede6/core/src/test/scala/org/apache/spark/ExecutorAllocationManagerSuite.scala#L758-L759) and `SPARK-30511` assertions at [lines 865-866](https://github.com/apache/spark/blob/1ac32efce4cb93c6ea31d6955af4d54f09ceede6/core/src/test/scala/org/apache/spark/ExecutorAllocationManagerSuite.scala#L865-L866) still expect 1 executor while speculative attempts are running. The revised retention behavior intentionally returns 2 in both cases, and the [current core CI job](https://github.com/zahed1994/spark/actions/runs/33369053539/job/99420145042) fails both tests with `2 did not equal 1`. The later assertions at [lines 886-887](https://github.com/apache/spark/blob/1ac32efce4cb93c6ea31d6955af4d54f09ceede6/core/src/test/scala/org/apache/spark/ExecutorAllocationManagerSuite.scala#L886-L887) also need the corresponding update; that mismatch is source-established because the earlier failure prevents CI from reaching it. These failures call for updating the old expectations, not reverting the intended retention behavior. Validation: the exact-head CI artifact reports 47 passes and 2 failures out of 49 allocation tests. Both new tests pass, and Scala lint passes. -- 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]
