You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by zhzhan <gi...@git.apache.org> on 2015/02/18 20:36:16 UTC
[GitHub] spark pull request: [Spark 5889] Remove pid file after stopping se...
GitHub user zhzhan opened a pull request:
https://github.com/apache/spark/pull/4676
[Spark 5889] Remove pid file after stopping service.
Currently the pid file is not deleted, and potentially may cause some problem after service is stopped. The fix remove the pid file after service stopped.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zhzhan/spark spark-5889
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4676.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 #4676
----
commit e63bfa5dcd7f35e0101a8aa6631a1ae29b81a399
Author: Zhan Zhang <zh...@gmail.com>
Date: 2014-08-08T17:47:18Z
test
commit c0c7d2ae0dcd4d8921513910985e10f1f58e8ab4
Author: Zhan Zhang <zh...@gmail.com>
Date: 2015-01-07T21:01:45Z
squash all commits
commit bfabd91d350fbb48c103896a585b362c7c823c2d
Author: Zhan Zhang <zh...@gmail.com>
Date: 2015-02-18T19:22:23Z
spark-5889: remove pid file after stopping service
----
---
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-5889] Remove pid file after stopping se...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-74946981
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27685/
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-5889] Remove pid file after stopping se...
Posted by zhzhan <gi...@git.apache.org>.
Github user zhzhan commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75158902
@srowen No problem. Thanks for the explanation, and that's a valid one.
---
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-5889] Remove pid file after stopping se...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75168263
[Test build #27753 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27753/consoleFull) for PR 4676 at commit [`eb01be1`](https://github.com/apache/spark/commit/eb01be1afc4456248cc26f38948d9eff9ef4ea2d).
* 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-5889] Remove pid file after stopping se...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/4676#discussion_r25009839
--- Diff: sbin/spark-daemon.sh ---
@@ -169,6 +169,7 @@ case $option in
if [[ $(ps -p "$TARGET_ID" -o args=) =~ $command ]]; then
echo "stopping $command"
kill "$TARGET_ID"
--- End diff --
a more concise way to do this is `kill ... && rm -rf $pid` I think
---
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-5889] Remove pid file after stopping se...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75169780
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27747/
Test FAILed.
---
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-5889] Remove pid file after stopping se...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4676#discussion_r24978988
--- Diff: sbin/spark-daemon.sh ---
@@ -169,6 +169,7 @@ case $option in
if [[ $(ps -p "$TARGET_ID" -o args=) =~ $command ]]; then
echo "stopping $command"
kill "$TARGET_ID"
--- End diff --
Concretely, can there be an `if` statement here to check whether `kill` succeeded, and only `rm` if so?
---
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-5889] Remove pid file after stopping se...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4676#discussion_r25011859
--- Diff: sbin/spark-daemon.sh ---
@@ -169,6 +169,7 @@ case $option in
if [[ $(ps -p "$TARGET_ID" -o args=) =~ $command ]]; then
echo "stopping $command"
kill "$TARGET_ID"
--- End diff --
Yeah, it occurs to me that this doesn't even guarantee the process was killed, but that the signal was sent. Well, I kind of like `kill ... && rm -f $pid` and would vote for merging that.
---
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-5889] Remove pid file after stopping se...
Posted by zhzhan <gi...@git.apache.org>.
Github user zhzhan commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75154178
@srowen @andrewor14 Changed to kill "$TARGET_ID" && rm -f $pid. Thanks.
---
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-5889] Remove pid file after stopping se...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75169775
**[Test build #27747 timed out](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27747/consoleFull)** for PR 4676 at commit [`b4c009e`](https://github.com/apache/spark/commit/b4c009e78ab0ccc6215fb08929408ba8e848ebdf) after a configured wait of `120m`.
---
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-5889] Remove pid file after stopping se...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75156596
(Test failure looks like a Jenkins network problem; ignore it)
---
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-5889] Remove pid file after stopping se...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-74946963
[Test build #27685 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27685/consoleFull) for PR 4676 at commit [`bfabd91`](https://github.com/apache/spark/commit/bfabd91d350fbb48c103896a585b362c7c823c2d).
* 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-5889] Remove pid file after stopping se...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75156191
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27746/
Test FAILed.
---
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-5889] Remove pid file after stopping se...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75154672
[Test build #27747 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27747/consoleFull) for PR 4676 at commit [`b4c009e`](https://github.com/apache/spark/commit/b4c009e78ab0ccc6215fb08929408ba8e848ebdf).
* 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-5889] Remove pid file after stopping se...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/4676
---
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-5889] Remove pid file after stopping se...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-74932367
[Test build #27685 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27685/consoleFull) for PR 4676 at commit [`bfabd91`](https://github.com/apache/spark/commit/bfabd91d350fbb48c103896a585b362c7c823c2d).
* 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-5889] Remove pid file after stopping se...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75168268
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27753/
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-5889] Remove pid file after stopping se...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75155705
@zhzhan I apologize for the extra round trip on this simple change, but it occurs to me that we should quote `"$pid"`. We should probably do it elsewhere in this an other scripts as @nchammas reminds us, but I imagine it's especially important as the argument to `rm`. Just in case there's... I don't know, a `/opt/very/important/Project` file and for some reason the PID path is `/opt/very/important/Project Data/pid`!
---
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-5889] Remove pid file after stopping se...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4676#issuecomment-75159159
[Test build #27753 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27753/consoleFull) for PR 4676 at commit [`eb01be1`](https://github.com/apache/spark/commit/eb01be1afc4456248cc26f38948d9eff9ef4ea2d).
* 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