Repository navigation
Conversation
Assisted-by: Claude Opus 5.5
There was a problem hiding this comment.
🟡 Changes recommended
The regression test can pass with the fix reverted because the immediate application lookup may set appStarted to true, leaving the new branch untested.
1 open issue
What changed in this PR
This PR lets local batches finish successfully when their submit process exits with code 0 before JPS detects the application, addressing intermittent batch-recovery failures.
Changes:
- Accept a missing application ID after successful exit when the application operation does not persist state.
- Preserve existing failure behavior for persistent-state operations and unmatched operations.
- Add a regression test using a long polling interval.
| File | Description |
|---|---|
kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala |
Adds coverage for a local batch finishing before application detection. |
kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala |
Conditions missing-ID failure on application-state persistence support. |
Review effort: Balanced
Comment on lines
+545
to
+548
| // a long check interval lets the process exit before the app is ever polled | ||
| val requestObj = newSparkBatchRequest(Map( | ||
| "spark.master" -> "local", | ||
| BATCH_APPLICATION_CHECK_INTERVAL.key -> "PT1M")) |
Member
Author
There was a problem hiding this comment.
Valid. Fixed in 8d9cd55: the test now hides both the launcher and spark-submit JVMs from jps via -XX:-UsePerfData (kyuubi.engineEnv.SPARK_LAUNCHER_OPTS / SPARK_SUBMIT_OPTS), so the lookup returns NOT_FOUND through process exit, and asserts appId is null and appStartTime is 0 before asserting FINISHED. Fails 3/3 with the fix reverted.
cxzl25
approved these changes
Oct 30, 2026
Assisted-by: Claude Opus 5.5
pan3793
added a commit
that referenced
this pull request
Oct 30, 2026
…ected as finished ### Why are the changes needed? A local (JPS) batch that exits with code 0 can be marked ERROR with `job failed: Some(ApplicationInfo(null,null,NOT_FOUND,...))`. In `submitAndMonitorBatchJob`, the submit process is polled every `kyuubi.batch.application.check.interval` (default 5s). A short client-mode app (e.g. SparkPi on `local`) can start and finish between two polls, so its id is never observed and `appStarted` stays false. JPS does not keep the state of finished apps, so `NOT_FOUND` here is indistinguishable from "never started", and the zero exit code is the only trustworthy signal. This makes `batch sessions recovery` in `BatchesV1ResourceSuite`/`BatchesV2ResourceSuite` flaky, e.g. https://github.com/apache/kyuubi/actions/runs/40693174724/job/122832955449 The fix only fails on a missing app id when the app operation persists app state (YARN, Kubernetes) or no app operation matches, keeping existing behavior for those. ### How was this patch tested? Added `local batch finished before the application is detected` to `BatchesResourceSuite`. It hides the batch JVMs from jps (`-XX:-UsePerfData`) so the app is never detected, then asserts `FINISHED`. It fails without the fix (`"[ERROR]" did not equal "[FINISHED]"`) and passes with it. ``` build/mvn test -pl kyuubi-server -am -Pspark-3.5 -Dtest=none \ -DwildcardSuites=org.apache.kyuubi.server.api.v1.BatchesV1ResourceSuite,org.apache.kyuubi.server.api.v1.BatchesV2ResourceSuite ``` ### Was this patch assisted by generative AI tooling? Assisted-by: Claude Opus 5.5 Closes #7816 from pan3793/kyuubi-flaky-batch-recovery. Closes #7816 8d9cd55 [Cheng Pan] Hide the batch JVMs from jps to make the test deterministic 64b7646 [Cheng Pan] Treat a local batch that exits 0 before its app is detected as finished Authored-by: Cheng Pan <pan3793@gmail.com> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit ab07bdf) Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793
added a commit
that referenced
this pull request
Oct 30, 2026
…ected as finished ### Why are the changes needed? A local (JPS) batch that exits with code 0 can be marked ERROR with `job failed: Some(ApplicationInfo(null,null,NOT_FOUND,...))`. In `submitAndMonitorBatchJob`, the submit process is polled every `kyuubi.batch.application.check.interval` (default 5s). A short client-mode app (e.g. SparkPi on `local`) can start and finish between two polls, so its id is never observed and `appStarted` stays false. JPS does not keep the state of finished apps, so `NOT_FOUND` here is indistinguishable from "never started", and the zero exit code is the only trustworthy signal. This makes `batch sessions recovery` in `BatchesV1ResourceSuite`/`BatchesV2ResourceSuite` flaky, e.g. https://github.com/apache/kyuubi/actions/runs/40693174724/job/122832955449 The fix only fails on a missing app id when the app operation persists app state (YARN, Kubernetes) or no app operation matches, keeping existing behavior for those. ### How was this patch tested? Added `local batch finished before the application is detected` to `BatchesResourceSuite`. It hides the batch JVMs from jps (`-XX:-UsePerfData`) so the app is never detected, then asserts `FINISHED`. It fails without the fix (`"[ERROR]" did not equal "[FINISHED]"`) and passes with it. ``` build/mvn test -pl kyuubi-server -am -Pspark-3.5 -Dtest=none \ -DwildcardSuites=org.apache.kyuubi.server.api.v1.BatchesV1ResourceSuite,org.apache.kyuubi.server.api.v1.BatchesV2ResourceSuite ``` ### Was this patch assisted by generative AI tooling? Assisted-by: Claude Opus 5.5 Closes #7816 from pan3793/kyuubi-flaky-batch-recovery. Closes #7816 8d9cd55 [Cheng Pan] Hide the batch JVMs from jps to make the test deterministic 64b7646 [Cheng Pan] Treat a local batch that exits 0 before its app is detected as finished Authored-by: Cheng Pan <pan3793@gmail.com> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit ab07bdf) Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793
added a commit
that referenced
this pull request
Oct 30, 2026
…ected as finished ### Why are the changes needed? A local (JPS) batch that exits with code 0 can be marked ERROR with `job failed: Some(ApplicationInfo(null,null,NOT_FOUND,...))`. In `submitAndMonitorBatchJob`, the submit process is polled every `kyuubi.batch.application.check.interval` (default 5s). A short client-mode app (e.g. SparkPi on `local`) can start and finish between two polls, so its id is never observed and `appStarted` stays false. JPS does not keep the state of finished apps, so `NOT_FOUND` here is indistinguishable from "never started", and the zero exit code is the only trustworthy signal. This makes `batch sessions recovery` in `BatchesV1ResourceSuite`/`BatchesV2ResourceSuite` flaky, e.g. https://github.com/apache/kyuubi/actions/runs/40693174724/job/122832955449 The fix only fails on a missing app id when the app operation persists app state (YARN, Kubernetes) or no app operation matches, keeping existing behavior for those. ### How was this patch tested? Added `local batch finished before the application is detected` to `BatchesResourceSuite`. It hides the batch JVMs from jps (`-XX:-UsePerfData`) so the app is never detected, then asserts `FINISHED`. It fails without the fix (`"[ERROR]" did not equal "[FINISHED]"`) and passes with it. ``` build/mvn test -pl kyuubi-server -am -Pspark-3.5 -Dtest=none \ -DwildcardSuites=org.apache.kyuubi.server.api.v1.BatchesV1ResourceSuite,org.apache.kyuubi.server.api.v1.BatchesV2ResourceSuite ``` ### Was this patch assisted by generative AI tooling? Assisted-by: Claude Opus 5.5 Closes #7816 from pan3793/kyuubi-flaky-batch-recovery. Closes #7816 8d9cd55 [Cheng Pan] Hide the batch JVMs from jps to make the test deterministic 64b7646 [Cheng Pan] Treat a local batch that exits 0 before its app is detected as finished Authored-by: Cheng Pan <pan3793@gmail.com> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit ab07bdf) Signed-off-by: Cheng Pan <chengpan@apache.org>
Member
|
CI failed on branch-1.10 https://github.com/apache/kyuubi/actions/runs/41022327580/job/123952278548, |
Member
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why are the changes needed?
A local (JPS) batch that exits with code 0 can be marked ERROR with
job failed: Some(ApplicationInfo(null,null,NOT_FOUND,...)).In
submitAndMonitorBatchJob, the submit process is polled everykyuubi.batch.application.check.interval(default 5s). A short client-mode app (e.g. SparkPi onlocal) can start and finish between two polls, so its id is never observed andappStartedstays false. JPS does not keep the state of finished apps, soNOT_FOUNDhere is indistinguishable from "never started", and the zero exit code is the only trustworthy signal.This makes
batch sessions recoveryinBatchesV1ResourceSuite/BatchesV2ResourceSuiteflaky, e.g. https://github.com/apache/kyuubi/actions/runs/40693174724/job/122832955449The fix only fails on a missing app id when the app operation persists app state (YARN, Kubernetes) or no app operation matches, keeping existing behavior for those.
How was this patch tested?
Added
local batch finished before the application is detectedtoBatchesResourceSuite. It hides the batch JVMs from jps (-XX:-UsePerfData) so the app is never detected, then assertsFINISHED. It fails without the fix ("[ERROR]" did not equal "[FINISHED]") and passes with it.Was this patch assisted by generative AI tooling?
Assisted-by: Claude Opus 5.5