You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by WangTaoTheTonic <gi...@git.apache.org> on 2014/12/12 08:31:15 UTC
[GitHub] spark pull request: [Deploy]some other processes might take the pi...
GitHub user WangTaoTheTonic opened a pull request:
https://github.com/apache/spark/pull/3683
[Deploy]some other processes might take the pid
Some other processes might use the pid saved in pid file. In that case we should ignore it and launch daemons.
JIRA is down for maintenance. I will file one once it return.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/WangTaoTheTonic/spark otherproc
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3683.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 #3683
----
commit f36cfb42c01ee525878e3baac8962128c66d2093
Author: WangTaoTheTonic <ba...@aliyun.com>
Date: 2014-12-12T07:14:13Z
some other processes might take the 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-4832][Deploy]some other processes might...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on a diff in the pull request:
https://github.com/apache/spark/pull/3683#discussion_r24640684
--- Diff: sbin/spark-daemon.sh ---
@@ -164,9 +165,10 @@ case $option in
(stop)
if [ -f $pid ]; then
- if kill -0 `cat $pid` > /dev/null 2>&1; then
+ TARGET_ID=`cat $pid`
+ if [[ `ps -p $TARGET_ID -o args=` =~ $command ]]; then
echo stopping $command
- kill `cat $pid`
+ kill $TARGET_ID
--- End diff --
Quote the variable here please.
---
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-4832][Deploy]some other processes might...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-74214438
[Test build #27422 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27422/consoleFull) for PR 3683 at commit [`daa86a1`](https://github.com/apache/spark/commit/daa86a1c24a1cee41b41f32e7c378a60c94ec471).
* 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-4832][Deploy]some other processes might...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on a diff in the pull request:
https://github.com/apache/spark/pull/3683#discussion_r24640678
--- Diff: sbin/spark-daemon.sh ---
@@ -129,8 +129,9 @@ case $option in
mkdir -p "$SPARK_PID_DIR"
if [ -f $pid ]; then
- if kill -0 `cat $pid` > /dev/null 2>&1; then
- echo $command running as process `cat $pid`. Stop it first.
+ TARGET_ID=`cat $pid`
+ if [[ `ps -p $TARGET_ID -o args=` =~ $command ]]; then
+ echo $command running as process $TARGET_ID. Stop it first.
--- End diff --
Quote the whole output string to protect against Bash word splitting. If `$command` contains a semicolon, for example, this will turn into 2 commands.
---
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-4832][Deploy]some other processes might...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on a diff in the pull request:
https://github.com/apache/spark/pull/3683#discussion_r24640723
--- Diff: sbin/spark-daemon.sh ---
@@ -164,9 +165,10 @@ case $option in
(stop)
if [ -f $pid ]; then
- if kill -0 `cat $pid` > /dev/null 2>&1; then
+ TARGET_ID=`cat $pid`
+ if [[ `ps -p $TARGET_ID -o args=` =~ $command ]]; then
echo stopping $command
- kill `cat $pid`
+ kill $TARGET_ID
else
echo no $command to stop
--- End diff --
I know this was not part of your changes, but we should also quote this whole string here, too.
```
echo "no $command to stop"
```
---
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-4832][Deploy]some other processes might...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66781962
If the process with the pid stored in pid file is not spark daemon, it cannot prove spark daemon didn't exist cleanly but just some other process take that pid after spark daemon ends.
I don't think the pid file should be deleted as we cannot make sure the process exit cleanly.
I'm not sure I understand what you mean totally. Correct me if I am wrong.
---
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-4832][Deploy]some other processes might...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-74220586
[Test build #27422 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27422/consoleFull) for PR 3683 at commit [`daa86a1`](https://github.com/apache/spark/commit/daa86a1c24a1cee41b41f32e7c378a60c94ec471).
* 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-4832][Deploy]some other processes might...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66746734
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24397/
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-4832][Deploy]some other processes might...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66779706
@srowen About the process isn't the one we expected, is it necessary to log a message? Ignoring it would be better cause it doesn't do any hurt 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-4832][Deploy]some other processes might...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66746729
[Test build #24397 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24397/consoleFull) for PR 3683 at commit [`f36cfb4`](https://github.com/apache/spark/commit/f36cfb42c01ee525878e3baac8962128c66d2093).
* 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-4832][Deploy]some other processes might...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66762205
[Test build #24405 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24405/consoleFull) for PR 3683 at commit [`cf4ecc6`](https://github.com/apache/spark/commit/cf4ecc638992cca6c2718c494a21f0da09f26e7a).
* 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-4832][Deploy]some other processes might...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/3683
---
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-4832][Deploy]some other processes might...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66765980
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24406/
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-4832][Deploy]some other processes might...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-74214516
@nchammas Updated as the feedback. Could you check it again, please? 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-4832][Deploy]some other processes might...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66758659
[Test build #24406 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24406/consoleFull) for PR 3683 at commit [`8befee7`](https://github.com/apache/spark/commit/8befee71d4678e93e1ad8d24185e4966782100ee).
* 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-4832][Deploy]some other processes might...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66785659
In case that user might start a daemon that is already running, we use pid file to check and give a message. Another case is that daemon might not exit after user stop it, according to the pid file we can prompt them.
---
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-4832][Deploy]some other processes might...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66765972
[Test build #24406 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24406/consoleFull) for PR 3683 at commit [`8befee7`](https://github.com/apache/spark/commit/8befee71d4678e93e1ad8d24185e4966782100ee).
* 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-4832][Deploy]some other processes might...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on a diff in the pull request:
https://github.com/apache/spark/pull/3683#discussion_r24640698
--- Diff: sbin/spark-daemon.sh ---
@@ -164,9 +165,10 @@ case $option in
(stop)
if [ -f $pid ]; then
- if kill -0 `cat $pid` > /dev/null 2>&1; then
+ TARGET_ID=`cat $pid`
--- End diff --
Same feedback as [the identical line above](https://github.com/apache/spark/pull/3683/files#diff-92634b2c881c4bf504852e7b963e7894R132).
---
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-4832][Deploy]some other processes might...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-74180612
I think we can finish and merge this. The current logic of the script is "if anything is running at the PID given in the file, fail". Your change is "if the Spark daemon we expect is running at the PID given in the file, fail" -- meaning in the case where somehow the PID files wasn't cleaned up and another process has that PID, we continue.
That's good. I had suggested cleaning up the PID file when it's known it's not the Spark daemon. But I see that in the start case, the PID file is overwritten anyway. In the stop case, it's not cleared, but, maybe that's not so important.
So, I think that is actually already addressed. Could I get @nchammas to render an opinion on the bash syntax -- are we not doing back ticks? Otherwise I think it's a good one to merge.
---
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-4832][Deploy]some other processes might...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on a diff in the pull request:
https://github.com/apache/spark/pull/3683#discussion_r24640629
--- Diff: sbin/spark-daemon.sh ---
@@ -129,8 +129,9 @@ case $option in
mkdir -p "$SPARK_PID_DIR"
if [ -f $pid ]; then
- if kill -0 `cat $pid` > /dev/null 2>&1; then
- echo $command running as process `cat $pid`. Stop it first.
+ TARGET_ID=`cat $pid`
--- End diff --
Bash style: Use `$(...)` instead of backticks, which are discouraged.
Also, quote the output of the subshell, as well as the variable argument to `cat`:
```
TARGET_ID="$(cat "$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-4832][Deploy]some other processes might...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66782725
Hm, shouldn't the process or its script clean up a PID file? If the daemon is not running at that process ID, when would you want to leave a PID file saying it is?
---
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-4832][Deploy]some other processes might...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66762212
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24405/
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-4832][Deploy]some other processes might...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-74216847
LGTM Bash-wise.
---
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-4832][Deploy]some other processes might...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-74195395
Generally looks good. Left a bunch of feedback about Bash style and protecting against word splitting bugs.
---
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-4832][Deploy]some other processes might...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66754690
[Test build #24405 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24405/consoleFull) for PR 3683 at commit [`cf4ecc6`](https://github.com/apache/spark/commit/cf4ecc638992cca6c2718c494a21f0da09f26e7a).
* 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-4832][Deploy]some other processes might...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66780011
It means the process didn't exit cleanly before, and the PID file is invalid. It can't hurt to note that. The PID file should probably be deleted too.
---
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-4832][Deploy]some other processes might...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-74220599
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27422/
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: [Deploy]some other processes might take the pi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3683#issuecomment-66740852
[Test build #24397 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24397/consoleFull) for PR 3683 at commit [`f36cfb4`](https://github.com/apache/spark/commit/f36cfb42c01ee525878e3baac8962128c66d2093).
* 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-4832][Deploy]some other processes might...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3683#discussion_r21734526
--- Diff: sbin/spark-daemon.sh ---
@@ -129,9 +129,12 @@ case $option in
mkdir -p "$SPARK_PID_DIR"
if [ -f $pid ]; then
- if kill -0 `cat $pid` > /dev/null 2>&1; then
- echo $command running as process `cat $pid`. Stop it first.
- exit 1
+ TARGET_ID=`cat $pid`
+ if kill -0 $TARGET_ID > /dev/null 2>&1; then
+ if [[ `ps -p $TARGET_ID -o args=` =~ $command ]]; then
--- End diff --
I agree this looks like a better test, but isn't it redundant with `kill`?
Or: in case you have detected that the PID file existed but isn't the expected process, log a message?
You could use `ps -p` for consistency in both cases.
---
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-4832][Deploy]some other processes might...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on a diff in the pull request:
https://github.com/apache/spark/pull/3683#discussion_r24640641
--- Diff: sbin/spark-daemon.sh ---
@@ -129,8 +129,9 @@ case $option in
mkdir -p "$SPARK_PID_DIR"
if [ -f $pid ]; then
- if kill -0 `cat $pid` > /dev/null 2>&1; then
- echo $command running as process `cat $pid`. Stop it first.
+ TARGET_ID=`cat $pid`
+ if [[ `ps -p $TARGET_ID -o args=` =~ $command ]]; then
--- End diff --
Same here: Please convert the backticks to `$(...)`.
---
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-4832][Deploy]some other processes might...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on a diff in the pull request:
https://github.com/apache/spark/pull/3683#discussion_r21736185
--- Diff: sbin/spark-daemon.sh ---
@@ -129,9 +129,12 @@ case $option in
mkdir -p "$SPARK_PID_DIR"
if [ -f $pid ]; then
- if kill -0 `cat $pid` > /dev/null 2>&1; then
- echo $command running as process `cat $pid`. Stop it first.
- exit 1
+ TARGET_ID=`cat $pid`
+ if kill -0 $TARGET_ID > /dev/null 2>&1; then
+ if [[ `ps -p $TARGET_ID -o args=` =~ $command ]]; then
--- End diff --
Yep it is redundant with `kill`.
But that reminds me that we will use `kill` command to check if the daemon pid exists in other places, in which condition that if some other process take that pid, we might kill others.
---
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