dongjoon-hyun opened a new pull request, #845:
URL: https://github.com/apache/spark-kubernetes-operator/pull/845

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


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