sunchao commented on code in PR #58380:
URL: https://github.com/apache/spark/pull/58380#discussion_r3882815448


##########
core/src/test/scala/org/apache/spark/ExecutorAllocationManagerSuite.scala:
##########
@@ -2065,6 +2065,31 @@ class ExecutorAllocationManagerSuite extends 
SparkFunSuite {
   private def executorsDecommissioning(manager: ExecutorAllocationManager): 
Set[String] = {
     manager.executorMonitor.executorsDecommissioning()
   }
+
+  test("SPARK-49485: request additional executor when speculative tasks equal 
maxNeeded") {
+    val manager = createManager(createConf(1, 5, 2))
+    assert(maxNumExecutorsNeededPerResourceProfile(manager, defaultProfile) 
=== 0)
+
+    // Submit stage with 2 tasks
+    post(SparkListenerStageSubmitted(createStageInfo(0, 2)))
+    post(SparkListenerExecutorAdded(0, "executor-1", new ExecutorInfo("host1", 
1, Map.empty)))
+    post(SparkListenerExecutorAdded(0, "executor-2", new ExecutorInfo("host1", 
1, Map.empty)))
+
+    // Task 0 and Task 1 start on executor-1 and executor-2
+    val taskInfo0 = createTaskInfo(0, 0, "executor-1")
+    val taskInfo1 = createTaskInfo(1, 1, "executor-2")
+    post(SparkListenerTaskStart(0, 0, taskInfo0))
+    post(SparkListenerTaskStart(0, 0, taskInfo1))
+
+    // 2 tasks running, 2 active executors. maxNeeded = 2
+    assert(maxNumExecutorsNeededPerResourceProfile(manager, defaultProfile) 
=== 2)
+
+    // Task 0 is submitted as speculatable
+    post(SparkListenerSpeculativeTaskSubmitted(0, 0))
+
+    // With pendingSpeculative > 0 and maxNeeded == activeExecutors (2), 
offset allocates 1 more
+    assert(maxNumExecutorsNeededPerResourceProfile(manager, defaultProfile) 
=== 3)

Review Comment:
   **[P2] Exercise the new allocation condition in the regression test**
   
   `createConf(1, 5, 2)` sets the initial executor count, leaving one task slot 
per executor. After speculation is submitted, `maxNeeded` is already `ceil((2 + 
1) / 1) = 3`, while the active count is 2. The new branch never executes, so 
this assertion cannot detect removal of the fix.
   
   For example, configure two two-core executors with three running tasks and 
one pending speculative task: base then returns 2 and head returns 3.



-- 
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]

Reply via email to