Repository navigation
[SPARK-60018] Add ApplicationStateSummary.Suspended and ApplicationStatus#resume - #946
dongjoon-hyun wants to merge 2 commits into
Conversation
|
Could you review this PR when you have some time, @viirya ? |
There was a problem hiding this comment.
The API changes look reasonable to me. Appending Suspended preserves the existing enum ordinals, and the revised isStopping() preserves the classification of all existing states. Extracting startNextAttempt() also keeps the existing restart history handling intact.
The distinction between resuming and restarting is clear: resuming advances the attempt ID without consuming the restart budget, while resetting the consecutive scheduling-failure counter. The tests cover both history modes and useful interactions with subsequent failures.
No blocking concerns found in this API-only change. For the separate PR that wires up running-application suspension, please cover reconciliation of persisted Suspended states (the current reconciler routes them to the unknown-state handler) and decide whether explicit resumption should bypass restart backoff (AppInitStep currently applies it whenever previousAttemptSummary is non-null, including after resume()). These are follow-up integration notes, not requests to expand this PR.
| } | ||
|
|
||
| /** | ||
| * Starts a new attempt of a suspended application which is resumed. Like a restart, the new |
There was a problem hiding this comment.
Non-blocking documentation suggestion: could we explicitly state that this method only constructs the next status and that callers must complete the required cleanup of the suspended attempt before starting the next one? This would make the caller's responsibility clear, similar to the resource-release precondition documented on terminateOrRestart().
There was a problem hiding this comment.
Thank you, @viirya. I added a paragraph saying that it only creates the status of the new attempt, and that the resources of the suspended attempt are expected to be released already, like terminateOrRestart.
|
Thank you for the review and approval, @viirya. Both notes are covered by the follow-up PR.
|
|
Thank you @dongjoon-hyun ! |
|
Thank you always, @viirya ! |
|
Merged to main |
What changes were proposed in this pull request?
This PR aims to add
ApplicationStateSummary.SuspendedandApplicationStatus#resume.SuspendedtoApplicationStateSummary, and makeisStopping()exclude it. Otherwise,AppCleanUpStepwould end a suspended application. The result for the existing states is unchanged.ApplicationStatus#resume, which starts the next attempt of aSuspendedapplication fromSubmitted. Unlike a restart, it does not increase the restart counters, and it resets onlyschedulingFailureRestartCounter.Suspendedto theSparkApplicationCRD ofspark.apache.org/v1, and updatedocs/migration_guide.md.Why are the changes needed?
To support
spec.suspendfor a runningSparkApplicationin a follow-up PR. Currently, it takes effect only before the driver is requested.Does this PR introduce any user-facing change?
Yes, in the API only. Unlike 1.0.0 (2026-07-23), the
SparkApplicationCRD accepts theSuspendedstate, an exhaustiveswitchoverApplicationStateSummaryneeds a case forSuspended, andApplicationStatushas the new public methodresume. There is no behavior change, since nothing uses them until the follow-up PR.How was this patch tested?
Pass the CIs with the newly added test cases.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 5.5