[SPARK-15665] [CORE] spark-submit --kill and --status are not working

[devaraj-kavali]

What changes were proposed in this pull request?

--kill and --status were not considered while handling in OptionParser and due to that it was failing. Now handling the --kill and --status options as part of OptionParser.handle.

How was this patch tested?

Added a test org.apache.spark.launcher.SparkSubmitCommandBuilderSuite.testCliKillAndStatus() and also I have verified these manually by running --kill and --status commands.

[srowen]

CC @vanzin

[srowen]

Jenkins test this please

[SparkQA]

Test build #59647 has finished for PR 13407 at commit 62ebc7a.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[vanzin]

LGTM but can you add unit tests so that these aren't inadvertently broken again?

[vanzin]

Actually, looking at it again, I think the fix makes the code a little confusing. Why is appResource being used to store the argument to an option? I know it all aligns in the end, but it looks weird, and it might be possible to break this.

I think it would be better to keep another flag saying whether appResource is required or not, and only checking that (around L159) depending on the value of the flag. Then the existing cases (no args and help) could be changed to set that flag too.

[andrewor14]

or just add a special appResource like PYSPARK_SHELL or SPARK_SHELL

[vanzin]

or just add a special appResource like PYSPARK_SHELL or SPARK_SHELL

If SparkSubmit can still process --kill and --status with those, then that's fine too (just use SparkLauncher.NO_RESOURCE).

[vanzin]

@devaraj-kavali did you have a chance to look at the feedback?

[devaraj-kavali]

Thanks @vanzin and @andrewor14 for looking into this, sorry for the delay.

If SparkSubmit can still process --kill and --status with those, then that's fine too (just use SparkLauncher.NO_RESOURCE).

I tried this but it doesn't work with the below error

[devaraj@server2 spark-master]$ ./bin/spark-submit --kill driver-20160531171222-0000
Error: Cannot load main class from JAR spark-internal with URI null. Please specify a class through --class.
Run with --help for usage help or --verbose for debug output

I have renamed the printInfo flag to isAppResourceReq and used the same for kill and status cases also.

Please review and let me know your feedback.

[vanzin]

nit: the naming sounds generic but this is testing options that take values, not any option... maybe tweak the name a little bit, or pass the option value as an argument (null == no value)?

[vanzin]

A couple of nits, but looks ok pending tests, which I have no idea why they haven't run. retest this please

[SparkQA]

Test build #59862 has finished for PR 13407 at commit 30dfb0f.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[vanzin]

I think these tests are flaky, but... retest this please

[SparkQA]

Test build #59875 has finished for PR 13407 at commit 30dfb0f.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[vanzin]

Merging to master / 2.0. If the names become an issue we can change later.

[devaraj-kavali]

Thanks @vanzin for review and merging.