[SPARK-5825] [Spark Submit] Remove the double checking instance name when stopping the service

[chenghao-intel]

spark-daemon.sh will confirm the process id by fuzzy matching the class name while stopping the service, however, it will fail if the java process arguments is very long (greater than 4096 characters).
This PR looses the check for the service process.

[SparkQA]

Test build #27510 has started for PR 4611 at commit f1bef66.

• This patch merges cleanly.

[chenghao-intel]

#4382 failed due to the SparkSubmit processes (HiveThriftServer) cannot be killed in unit testing. Probably we need to manually kill those processes, or restart the jenkins servers.

[SparkQA]

Test build #27510 has finished for PR 4611 at commit f1bef66.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27510/
Test PASSed.

[srowen]

I don't think we want to take out this check entirely. This was changed recently to not just test whether the process can be killed (with kill -0) as a way of testing whether it's even a Spark process, but to further examine the command line. That stinks if this fails on long command lines.

What about a weaker check like just grepping for spark in the command line? the point here is to not kill a foreign process that happens to have taken the PID named in a stale Spark PID file.

[chenghao-intel]

Thank you @srowen for the explanation, I've updated the script, can you review it again?

[SparkQA]

Test build #27537 has started for PR 4611 at commit 0b9ea52.

• This patch merges cleanly.

[SparkQA]

Test build #27537 has finished for PR 4611 at commit 0b9ea52.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27537/
Test PASSed.

[srowen]

This uses jps, but that is only included with the JDK. I think it's reasonable to expect many production deployments will only use the JRE. How about simply ps -p 4391 | grep -q $command ? does that have the same problem? I suppose I wasn't clear what part of the current solution doesn't work with long command lines.

[chenghao-intel]

@srowen, I was trying something like ps -p 4391 | grep -q $command, but it doesn't work. The process name is always java, and the parameters are -cp xxxxx, and I believe the command is always cut in the long command line.

jps seems the only way can print out the right process name, which is SparkSubmit.

[srowen]

@chenghao-intel ah, looks like the default behavior on OS X is different from Linux. Try ps -f -p ... on either one and it looks like you get something containing the full command. Does that work?

[chenghao-intel]

@srowen I've tested that both under ubuntu 12.04 and centos 6.5, ps -f -p ... only print the first 4096 characters of its arguments.
By the way, I've also checked the hadoop-daemon.sh of hadoop (hadoop 2.3), seems it doesn't confirm the process name as we did in spark-daemon.sh. Or can we just confirm if it's a java process?

[srowen]

Gotcha. OK. Until someone thinks of a more robust check, I think we can resort to just checking if ps -p $pid -o comm= is java? EDIT: better grep for java too as I find that on OS X, comm= prints the whole absolute path to the binary.

[SparkQA]

Test build #27670 has started for PR 4611 at commit a0051f6.

• This patch merges cleanly.

[chenghao-intel]

Thank you @srowen , I've updated the script, seems much simpler. Appreciated if you can confirm the change under OS X, sorry, I don't have machine with OS X.

[SparkQA]

Test build #27670 has finished for PR 4611 at commit a0051f6.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27670/
Test PASSed.

[srowen]

As I say on OS X you get the whole binary path, not just java:

ps -p ... -o comm=
...
/Library/Java/JavaVirtualMachines/jdk1.8.0_31.jdk/Contents/Home/jre/bin/java

that's why I was thinking if ps -p "$TARGET_ID" -o comm= | grep -q java ; then

• @nchammas for bash syntax thoughts.

[nchammas]

Bash style for this change is fine, though there are other areas in this script with word splitting problems, unfortunately (e.g. some references to $option). Shouldn't block this PR though.

[andrewor14]

@srowen it actually does a =~ which should be equivalent to a grep:

if [[ "abc" =~ "abc" ]]; then echo woohoo; fi # woohoo
if [[ "ffabc" =~ "abc" ]]; then echo woohoo; fi # woohoo
if [[ "ffabcff" =~ "abc" ]]; then echo woohoo; fi # woohoo

This seems like what we want, so I'm going to merge this into master 1.3 and 1.2 thanks @chenghao-intel

[srowen]

@andrewor14 Ah! right of course. I looked right past that. Yes that's a good change then.