Skip to content

Commit 9f7af65

Browse files
committed
[SPARK-59635] Requeue immediately when AppRunningStep changes the running state
### What changes were proposed in this pull request? This PR aims to make `AppRunningStep` requeue immediately (`completeAndImmediateRequeue()`) instead of using the default interval (`completeAndDefaultRequeue()`) when it appends a new running state (e.g. `DriverReady -> RunningHealthy`). The next reconcile then takes the `proposedStateSummary == prevStateSummary` branch and observes the driver with `AppDriverRunningObserver`. This is the same way `AppReconcileStep.observeDriver` already works when it changes the state. ### Why are the changes needed? When `AppRunningStep` changes the state, it completes the reconcile loop, so the same reconcile doesn't observe driver termination. The following `AppResourceObserveStep(AppDriverRunningObserver)` doesn't run either. If no more pod events arrive, the termination is only detected by the periodic reconcile (`spark.kubernetes.operator.reconciler.intervalSeconds`). This happens easily with the per-resource reconcile rate limiter (`refreshPeriodSeconds=15`, `maxLoopForPeriod=5` by default). A short job's driver and executor pod events are postponed to the end of the rate-limit period. The postponed reconcile then sees `DriverReady` while the driver pod has already succeeded. For example, `tests/e2e/state-transition` failed locally because `Succeeded` was recorded 120 seconds after `RunningHealthy`, which is longer than the 60s assert timeout: ``` 14:17:50.09 Submitted 14:17:50.39 DriverRequested 14:17:50.86 DriverStarted / DriverReady 14:18:05.09 RunningHealthy 14:20:05.11 Succeeded ``` With this PR, `Succeeded` is recorded right after `RunningHealthy`: ``` 14:38:14.650 RunningHealthy 14:38:14.660 Succeeded 14:38:14.762 ResourceReleased ``` The cost is one extra reconcile for each running state transition. ### Does this PR introduce _any_ user-facing change? Yes. The operator now detects driver termination right after it reports a running state, instead of waiting for the next periodic reconcile. ### How was this patch tested? Pass the CIs with the newly added test case in `AppRunningStepTest`. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 5 Closes #845 from dongjoon-hyun/SPARK-59635. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
1 parent f264275 commit 9f7af65

2 files changed

Lines changed: 21 additions & 2 deletions

File tree

‎spark-operator/src/main/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps/AppRunningStep.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
package org.apache.spark.k8s.operator.reconciler.reconcilesteps;
2121

2222
import static org.apache.spark.k8s.operator.Constants.*;
23-
import static org.apache.spark.k8s.operator.reconciler.ReconcileProgress.completeAndDefaultRequeue;
23+
import static org.apache.spark.k8s.operator.reconciler.ReconcileProgress.completeAndImmediateRequeue;
2424

2525
import java.util.List;
2626
import java.util.Set;
@@ -107,7 +107,7 @@ public ReconcileProgress reconcile(
107107
.getStatus()
108108
.appendNewState(new ApplicationState(proposedStateSummary, stateMessage));
109109
return attemptStatusUpdate(
110-
context, statusRecorder, updatedStatus, completeAndDefaultRequeue());
110+
context, statusRecorder, updatedStatus, completeAndImmediateRequeue());
111111
}
112112
}
113113

‎spark-operator/src/test/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps/AppRunningStepTest.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737

3838
import org.apache.spark.k8s.operator.SparkApplication;
3939
import org.apache.spark.k8s.operator.context.SparkAppContext;
40+
import org.apache.spark.k8s.operator.reconciler.ReconcileProgress;
4041
import org.apache.spark.k8s.operator.spec.ApplicationSpec;
4142
import org.apache.spark.k8s.operator.spec.ApplicationTolerations;
4243
import org.apache.spark.k8s.operator.spec.ExecutorInstanceConfig;
@@ -164,6 +165,24 @@ void runningHealthyDynamicAllocationEnabled() {
164165
assertEquals(ApplicationStateSummary.RunningHealthy, currentState);
165166
}
166167

168+
@Test
169+
void immediateRequeueWhenRunningStateChanges() {
170+
// Requeue immediately so that the next reconcile observes the driver in the new state
171+
appSpec.setApplicationTolerations(ApplicationTolerations.builder().build());
172+
173+
appStatus =
174+
appStatus.appendNewState(
175+
new ApplicationState(ApplicationStateSummary.DriverReady, "Driver ready"));
176+
app.setStatus(appStatus);
177+
178+
ReconcileProgress progress = appRunningStep.reconcile(mockContext, mockRecorder);
179+
180+
assertEquals(
181+
ApplicationStateSummary.RunningHealthy,
182+
app.getStatus().getCurrentState().getCurrentStateSummary());
183+
assertEquals(ReconcileProgress.completeAndImmediateRequeue(), progress);
184+
}
185+
167186
@Test
168187
void runningHealthyExecutorsAtMaxCapacity() {
169188
// Dynamic allocation disabled, executors at max capacity

0 commit comments

Comments
 (0)