You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by chenghao-intel <gi...@git.apache.org> on 2015/02/15 08:11:25 UTC
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
GitHub user chenghao-intel opened a pull request:
https://github.com/apache/spark/pull/4611
[SPARK-5825] [Spark Submit] Remove the double checking when killing process
`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).
This PR removes the double check, as the cooking the PID filename should be same for the confirmation.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/chenghao-intel/spark stopping_service
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4611.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #4611
----
commit f1bef6622caad140bfbd68ce87b2871bde4c3aed
Author: Cheng Hao <ha...@intel.com>
Date: 2015-02-15T06:53:01Z
remove the double checking when killing process
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/4611
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74414416
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74453783
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74595309
@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`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74799888
[Test build #27670 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27670/consoleFull) for PR 4611 at commit [`a0051f6`](https://github.com/apache/spark/commit/a0051f61e38e6f5e48fd7485546a2a1e3356b40f).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74596000
@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?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74965737
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74453780
[Test build #27537 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27537/consoleFull) for PR 4611 at commit [`0b9ea52`](https://github.com/apache/spark/commit/0b9ea528e6186ff8ad85a305abc693657e4d7536).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74650236
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`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74538795
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74800315
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74449523
[Test build #27537 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27537/consoleFull) for PR 4611 at commit [`0b9ea52`](https://github.com/apache/spark/commit/0b9ea528e6186ff8ad85a305abc693657e4d7536).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74408762
[Test build #27510 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27510/consoleFull) for PR 4611 at commit [`f1bef66`](https://github.com/apache/spark/commit/f1bef6622caad140bfbd68ce87b2871bde4c3aed).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74806525
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74406895
[Test build #27510 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27510/consoleFull) for PR 4611 at commit [`f1bef66`](https://github.com/apache/spark/commit/f1bef6622caad140bfbd68ce87b2871bde4c3aed).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-75128516
@andrewor14 Ah! right of course. I looked right past that. Yes that's a good change then.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-75125804
@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 and 1.3 thanks @chenghao-intel
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74449462
Thank you @srowen for the explanation, I've updated the scripts, can you review it again?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74408764
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74842751
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74806519
[Test build #27670 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27670/consoleFull) for PR 4611 at commit [`a0051f6`](https://github.com/apache/spark/commit/a0051f61e38e6f5e48fd7485546a2a1e3356b40f).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74617241
@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?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5825] [Spark Submit] Remove the double ...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/4611#issuecomment-74406965
#4382 failed due to the `SparkSubmit`(HiveThriftServer) cannot be killed in unit testing. Probably we need to manually kill those processes, or restart the jenkins servers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org