[SPARK-54774][CORE] Submit failed should keep same exit code with app exit code #53539
Conversation
|
ping @dongjoon-hyun @cloud-fan |
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Do you think we can have a test coverage for this, @AngersZhuuuu ?
Added, could you take a look? |
|
gentle ping @dongjoon-hyun @cloud-fan |
| assertResult(3)(runSparkSubmit(args, expectFailure = true)) | ||
| } | ||
|
|
||
| test("SPARK-54774: k8s submit failed should keep same exit code with user code") { |
| ) | ||
| // The test application throws SparkUserAppException with exit code 42, | ||
| // so SparkContext.stop(42) should be called in k8s mode | ||
| assertResult(42)(runSparkSubmit(args, expectFailure = true)) |
There was a problem hiding this comment.
This test case seems to pass without your patch, @AngersZhuuuu . Could you double-check this test case?
$ git diff master --stat
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
$ build/sbt "core/testOnly *.SparkSubmitSuite -- -z SPARK-54774"
[info] SparkSubmitSuite:
[info] - SPARK-54774: k8s submit failed should keep same exit code with user code (2 seconds, 926 milliseconds)
[info] Run completed in 3 seconds, 795 milliseconds.
[info] Total number of tests run: 1
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 1, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 59 s, completed Dec 24, 2025, 6:29:30 AM
There was a problem hiding this comment.
How about current?
|
BTW, it would be great if we can generalize the PR title by removing |
AngersZhuuuu
changed the title
Done |
| val listStatus = fileSystem.listStatus(testDirPath) | ||
| val logData = EventLogFileReader.openEventLog(listStatus.last.getPath, fileSystem) | ||
| Source.fromInputStream(logData)(Codec.UTF8).getLines().filter { line => | ||
| line.contains("SparkListenerApplicationEnd") |
There was a problem hiding this comment.
a little bit hacky, possible to use a custom listener to achieve this?
BTW, will getLines close the resource correctly?
There was a problem hiding this comment.
a little bit hacky, possible to use a custom listener to achieve this?
BTW, will
getLinesclose the resource correctly?
Looks like previous UT also use this, follow other UT
|
ping @dongjoon-hyun |
This change is specific to k8s, right ? It is within a |
4 participants
What changes were proposed in this pull request?
When submit failed, k8s case will stop SparkContext.
Here we already know the exit code, so here we stop SC should also pass the exitcode too.
Why are the changes needed?
keep same exit code for SC and whole app, help when checking the log
Does this PR introduce any user-facing change?
No
How was this patch tested?
Added UT
Was this patch authored or co-authored using generative AI tooling?
No