You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/09/23 03:22:55 UTC
[GitHub] [dolphinscheduler] rickchengx opened a new pull request, #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
rickchengx opened a new pull request, #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118
<!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
## Purpose of the pull request
close: #12109
If the user starts DS twice (or more) using `dolphinscheduler-daemon.sh`, the `pid` file will be overwritten, making it impossible to stop the initially started DS cluster.
## Brief change log
* Use `pid` to check the status of the server
* If the server is already started, cancel this launch
## Verify this pull request
manually tested
1. Start all the server and `ps -ef`as below
<img width="896" alt="image" src="https://user-images.githubusercontent.com/38122586/191886250-332762be-0201-445f-a0cd-64d6176bad81.png">
2. Start all the server again
<img width="896" alt="截屏2022-09-23 11 21 00" src="https://user-images.githubusercontent.com/38122586/191886323-5d037d5e-47a4-40cb-8477-8170f23aeb47.png">
3. the result of `ps -ef` is unchanged
<img width="894" alt="image" src="https://user-images.githubusercontent.com/38122586/191886400-cd994978-a38b-464e-bb19-df9a74417c6e.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] hstdream commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
hstdream commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1296463791
hello, pr 12591 ,you can compare this pr to see which merge is suitable? @rickchengx @caishunfeng @SbloodyS @EricGao888
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1261660941
> IMHO, i think that pid file may not a good idea, because pid just can keep only one process pid
>
> 1. If there are two Master process running at same time, but this status function just find one. Becuase if use just call start.sh may start a process again.
> 2. If user use `kill` command, cannot clear this pid
> 3. Some misopeation may clear or modify this pid file.
> i think the result of using the `ps -ef` check is more accurate and reliable. Personal opinion,just for reference.
Hi, @DarkAssassinator , thanks a lot for the comment. Sorry for the late response.
> If the user starts DS twice (or more) using dolphinscheduler-daemon.sh, the pid file will be overwritten, making it impossible to stop the initially started DS cluster.
1. In fact, the purpose of this PR is to limit the user to start DS twice. When the user starts a server, it will first determine whether it has been started. If the server is already started, the script will cancel this launch.
2. As for using pid file to check the status may not be a good option, for example pid may be modified artificially. But currently the script uses the pid file to kill the process, facing the same problem. And the original approach (`grep #DOLPHINSCHEDULER_HOME`) is also likely to cause errors. Since there is no complete guarantee that other processes do not meet the filter criteria.
3. I think the script should take the **same approach** to get the process when checking the process status and killing the process (`grep ...` or check the `pid`). And I personally prefer to check the `pid` file because it's a way used by a lot of programs on linux.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] DarkAssassinator commented on a diff in pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#discussion_r1007975986
##########
script/dolphinscheduler-daemon.sh:
##########
@@ -81,8 +79,26 @@ else
exit 1
fi
+state=""
+function get_server_running_status() {
+ state="STOP"
+ if [ -f $pid ]; then
Review Comment:
Hi @rickchengx, sorry for later reply. But I still think that using the file method to record the process is unreliable.
Our purpose is just to check whether the master/worker process exists, the current check command is relatively accurate. And this command has contain $DOLPHINSCHEDULER_HOME, so will not check other install path.
```
serverCount=`ps -ef | grep "java" | grep "$DOLPHINSCHEDULER_HOME" | grep "$CLASS" | grep -v "grep" | wc -l`
```
So i think that we just need use this command to check the server status before start the services. If running, no need start again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1272902690
> > 2. And if user starts more than one masters or worker servers with different ports in one host, I agree that the way to check pid will miss some servers. May I ask that is this way of launching the server using a different port officially supported? I wonder how user can do this. Copy another `master-server` directory, and then modify the `master.listen-port` in `master-server-copied/application.yaml`? Will there be any problems communicating with other servers after changing the port? I'm just curious, would be grateful if you could reply.
> >
> > If the second case needs to be considered, I think the way you proposed (`1624841.pid`) is good, I will continue to revise this PR.
>
> I think the second case needs to be considered. Please revise it.
Sure, I will look into it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1255773018
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12118)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1276918604
> @caishunfeng Thanks a lot for the comment.
>
> So in the same directory:
>
> * If the user start `master-server` twice without changing the port, it will cause the error `master does not exist`.
> * If the user start `master-server` twice and change the port, it can work but the log is shared. And also the `pid` file will be overwritten so the first started `master-server` can not be killed through the `pid` file.
>
> So I think we should not allow users to start `master-server` (or other server) twice or more **in the same directory**, because this is likely to be a user's misoperation.
>
> If users need to start 2 `master-server` on the same machine, they should start in another directory with different ports.
>
> WDYT
>
> And this PR avoids secondary startup in the same directory by querying the server status through `pid` file before startup. If the server is already started, the script will cancel this launch.
Looks good to me, cc @SbloodyS @DarkAssassinator
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1296524848
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12118)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1297966548
https://github.com/apache/spark/blob/a01b86f9d4651562f960745fc621d710ae9295cc/sbin/spark-daemon.sh#L161-L173
Hi, @caishunfeng I refer to `spark-daemon.sh` and make some modifications as below
* Use `if [[ $(ps -p "$TARGET_PID" -o comm=) =~ "bash" ]]; then` since ds use `bash <server>/bin/start.sh` to start the process.
* echo the process PID `echo "$command running as process $TARGET_PID. Stop it first."`
Test Result
<img width="832" alt="截屏2022-11-01 11 05 54" src="https://user-images.githubusercontent.com/38122586/199150617-986c9dce-af8a-40d9-b666-ff55ad7b1c41.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1297987400
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12118)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1296521378
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/12118?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#12118](https://codecov.io/gh/apache/dolphinscheduler/pull/12118?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (33ccf83) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/e6da1ccf813ea619797f18eaa205babf16263188?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6da1cc) will **decrease** coverage by `0.03%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## dev #12118 +/- ##
============================================
- Coverage 39.09% 39.06% -0.04%
+ Complexity 4194 4191 -3
============================================
Files 1044 1044
Lines 39486 39454 -32
Branches 4537 4519 -18
============================================
- Hits 15439 15412 -27
+ Misses 22292 22289 -3
+ Partials 1755 1753 -2
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/12118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...erver/master/processor/queue/TaskEventService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvcXVldWUvVGFza0V2ZW50U2VydmljZS5qYXZh) | `69.64% <0.00%> (-10.72%)` | :arrow_down: |
| [...r/plugin/registry/zookeeper/ZookeeperRegistry.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZWdpc3RyeS9kb2xwaGluc2NoZWR1bGVyLXJlZ2lzdHJ5LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1yZWdpc3RyeS16b29rZWVwZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3JlZ2lzdHJ5L3pvb2tlZXBlci9ab29rZWVwZXJSZWdpc3RyeS5qYXZh) | `43.54% <0.00%> (-6.46%)` | :arrow_down: |
| [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | :arrow_down: |
| [...hinscheduler/plugin/alert/script/ScriptSender.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9kb2xwaGluc2NoZWR1bGVyLWFsZXJ0LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1hbGVydC1zY3JpcHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL2FsZXJ0L3NjcmlwdC9TY3JpcHRTZW5kZXIuamF2YQ==) | `70.58% <0.00%> (-1.64%)` | :arrow_down: |
| [...cheduler/plugin/alert/dingtalk/DingTalkSender.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9kb2xwaGluc2NoZWR1bGVyLWFsZXJ0LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1hbGVydC1kaW5ndGFsay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vYWxlcnQvZGluZ3RhbGsvRGluZ1RhbGtTZW5kZXIuamF2YQ==) | `34.13% <0.00%> (-0.78%)` | :arrow_down: |
| [...rver/master/runner/task/BlockingTaskProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvdGFzay9CbG9ja2luZ1Rhc2tQcm9jZXNzb3IuamF2YQ==) | `75.86% <0.00%> (-0.55%)` | :arrow_down: |
| [...inscheduler/plugin/registry/etcd/EtcdRegistry.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZWdpc3RyeS9kb2xwaGluc2NoZWR1bGVyLXJlZ2lzdHJ5LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1yZWdpc3RyeS1ldGNkL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9yZWdpc3RyeS9ldGNkL0V0Y2RSZWdpc3RyeS5qYXZh) | `50.69% <0.00%> (-0.35%)` | :arrow_down: |
| [.../dolphinscheduler/plugin/task/datax/DataxTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stZGF0YXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svZGF0YXgvRGF0YXhUYXNrLmphdmE=) | `36.62% <0.00%> (-0.26%)` | :arrow_down: |
| [.../dolphinscheduler/plugin/task/spark/SparkTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stc3Bhcmsvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svc3BhcmsvU3BhcmtUYXNrLmphdmE=) | `72.56% <0.00%> (-0.25%)` | :arrow_down: |
| [...dolphinscheduler/service/storage/impl/S3Utils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2Uvc3RvcmFnZS9pbXBsL1MzVXRpbHMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| ... and [17 more](https://codecov.io/gh/apache/dolphinscheduler/pull/12118/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1296526779
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12118)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1291804729
> > @caishunfeng Thanks a lot for the comment.
> > So in the same directory:
> >
> > * If the user start `master-server` twice without changing the port, it will cause the error `master does not exist`.
> > * If the user start `master-server` twice and change the port, it can work but the log is shared. And also the `pid` file will be overwritten so the first started `master-server` can not be killed through the `pid` file.
> >
> > So I think we should not allow users to start `master-server` (or other server) twice or more **in the same directory**, because this is likely to be a user's misoperation.
> > If users need to start 2 `master-server` on the same machine, they should start in another directory with different ports.
> > WDYT
> > And this PR avoids secondary startup in the same directory by querying the server status through `pid` file before startup. If the server is already started, the script will cancel this launch.
>
> Looks good to me, cc @SbloodyS @DarkAssassinator
Hi, @SbloodyS @DarkAssassinator , could you please help review this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1298232081
> https://github.com/apache/spark/blob/a01b86f9d4651562f960745fc621d710ae9295cc/sbin/spark-daemon.sh#L161-L173 
>
> Hi, @caishunfeng I refer to `spark-daemon.sh` and make some modifications as below
>
> * Use `if [[ $(ps -p "$TARGET_PID" -o comm=) =~ "bash" ]]; then` to check the status of process since ds use `bash <server>/bin/start.sh` to start the process.
> * echo the process PID `echo "$command running as process $TARGET_PID. Stop it first."`
>
> Test Result
>
> <img alt="截屏2022-11-01 11 05 54" width="832" src="https://user-images.githubusercontent.com/38122586/199150617-986c9dce-af8a-40d9-b666-ff55ad7b1c41.png">
Nice work :+1:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1297926431
> 1.The pid will be considered modified or suspended, making it impossible to kill the thread. This happens often. 2.The pid in the pid file is inconsistent with the actual process number @rickchengx
Hi, @hstdream , thanks for your comment.
I got your concern. Actually, the `pid-file` method is commonly used on linux.
For example, Apache Spark `spark-daemon.sh` also use `pid-file` to check the status of process and kill the process as below:
https://github.com/apache/spark/blob/a01b86f9d4651562f960745fc621d710ae9295cc/sbin/spark-daemon.sh#L161-L173
<img width="1279" alt="image" src="https://user-images.githubusercontent.com/38122586/199143574-e836abf9-b4ec-4f44-ac4d-e8ee7bb717a5.png">
And I think the location of `pid-file` is in $DOLPHINSCHEDULER_HOME, which is a directory that only admin can attach. It cannot be easily modified.
WDYT
cc @caishunfeng @SbloodyS
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1255772632
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12118)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] DarkAssassinator commented on a diff in pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#discussion_r979265778
##########
script/dolphinscheduler-daemon.sh:
##########
@@ -81,8 +81,26 @@ else
exit 1
fi
+state=""
+function get_server_running_status() {
+ state="STOP"
+ if [ -f $pid ]; then
+ TARGET_PID=`cat $pid`
+ server_running_count=`ps -p $TARGET_PID --no-headers | wc -l`
+ if [[ $server_running_count -gt 0 ]];then
+ state="RUNNING"
+ fi
+ fi
+}
Review Comment:
IMHO, i think that pid file may not a good idea, because pid just can keep only one process pid
1. If there are two Master process running at same time, but this status function just find one. Becuase if use just call start.sh may start a process again.
2. If user use `kill` command, cannot clear this pid
3. Some misopeation may clear or modify this pid file.
i think the result of using the `ps -ef` check is more accurate and reliable. Personal opinion,just for reference.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1272821796
If user start more than one masters or worker servers with different ports in one host, the way to check pid will miss some servers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1272889435
> If user start more than one masters or worker servers with different ports in one host, the way to check pid will miss some servers.
>
> Or we can set more pid file, such as `1624841.pid` as pid file name, 1624841 is pid. WDYT?
@caishunfeng Thanks a lot for the comment!
1. If user deploys the DS through `install_env.sh` as below, there cannot be more than one masters (or workers ) in one host.
<img width="914" alt="image" src="https://user-images.githubusercontent.com/38122586/194814402-463315eb-fdba-4133-aef6-a7a66a7aa13d.png">
2. And if user starts more than one masters or worker servers with different ports in one host, I agree that the way to check pid will miss some servers. May I ask that is this way of launching the server using a different port officially supported? I wonder how user can do this? Copy another `master-server` directory, and then modify the `master.listen-port` in `master-server-copied/application.yaml`. Will there be any problems communicating with other servers after changing the port? I'm just curious, would be grateful if you could reply.
If the second case needs to be considered, I think the way you proposed (`1624841.pid`) is good, I will continue to revise this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1272949238
Hi, @caishunfeng @SbloodyS , thanks for the comment.
Taking `master-server` as an example:
Whether should we allow users to start `master-server` twice (or more) in the**same directory.**
1. If allowed, then different pids can be recorded by `<number>.pid`
2. If not allowed, then there is only one pid file in directory `master-server/`.
I think it should be avoided that the user starts 2 times in the same directory, because this may be the user's misoperation (such as forgetting to stop and start again), and bring unpredictable consequences.
And after my test, launching 2 (or more) `master-server` in the same directory will cause the entire DS service to be unavailable.
WDYT
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng merged pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
caishunfeng merged PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1296502744
Hi, @caishunfeng @SbloodyS @DarkAssassinator
**It seems that we need to discuss how to check the status of the server, `pid-file` or `grep <conditions>`.**
1. Now `dolphinscheduler-daemon.sh` uses `pid-file` to kill the process rather than getting the process pid through `grep <conditions>` and then killing it. This is a common approach on linux.
2. I think that checking the server status should be consistent with killing the process. So maybe it's better to check the server status through `pid-file`.
3. And also `grep <conditions>` method has no complete guarantee that other processes do not meet the filter criteria. For example, some non-ds process that may also contains the "java" & "$DOLPHINSCHEDULER_HOME" & "$CLASS". This will cause the script to misjudge the status of the server.
4. And $CLASS is now hardcoded, the class name maybe changed. And using the `pid-file` can avoid this hardcoded problem.
<img width="741" alt="image" src="https://user-images.githubusercontent.com/38122586/198926803-70c111c5-1dc7-4e13-ad58-bf9fdd387b69.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1261671367
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12118)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1275882805
@caishunfeng Thanks a lot for the comment.
So in the same directory:
* If the user start `master-server` twice without changing the port, it will cause the error `master does not exist`.
* If the user start `master-server` twice and change the port, it can work but the log is shared. And also the `pid` file will be overwritten so the first started `master-server` can not be killed through the `pid` file.
So I think we should not allow users to start `master-server` (or other server) twice or more **in the same directory**, because this is likely to be a user's misoperation.
If users need to start 2 `master-server` on the same machine, they should start in another directory with different ports.
WDYT
And this PR avoids secondary startup by querying the status before startup. If the server is already started, the script will cancel this launch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1273972599
I start the master server by `./bin/dolphinscheduler-daemon.sh start master-server`, and just change the port to `6678` and `6679`, it can work.
But I found it will print log in one file `dolphinscheduler-master.log`, this might also be problematic.
>Whether should we allow users to start master-server twice (or more) in the same directory.
If so, I think it's better to avoid start one server in the same directory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1297985412
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12118)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1272891216
> 2. And if user starts more than one masters or worker servers with different ports in one host, I agree that the way to check pid will miss some servers. May I ask that is this way of launching the server using a different port officially supported? I wonder how user can do this. Copy another `master-server` directory, and then modify the `master.listen-port` in `master-server-copied/application.yaml`? Will there be any problems communicating with other servers after changing the port? I'm just curious, would be grateful if you could reply.
>
> If the second case needs to be considered, I think the way you proposed (`1624841.pid`) is good, I will continue to revise this PR.
I think the second case needs to be considered. Please revise it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1261672227
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12118)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12118&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12118&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#discussion_r1007904585
##########
script/dolphinscheduler-daemon.sh:
##########
@@ -81,8 +79,26 @@ else
exit 1
fi
+state=""
+function get_server_running_status() {
+ state="STOP"
+ if [ -f $pid ]; then
+ TARGET_PID=`cat $pid`
+ server_running_count=`ps -p $TARGET_PID --no-headers | wc -l`
Review Comment:
It seems like `--no-headers` can not be running in `MacOS`. It's better to use a compatible way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] hstdream commented on pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
hstdream commented on PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#issuecomment-1297909110
1.The pid will be considered modified or suspended, making it impossible to kill the thread. This happens often.
2.The pid in the pid file is inconsistent with the actual process number
@rickchengx
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on a diff in pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on code in PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#discussion_r1009002835
##########
script/dolphinscheduler-daemon.sh:
##########
@@ -81,8 +79,26 @@ else
exit 1
fi
+state=""
+function get_server_running_status() {
+ state="STOP"
+ if [ -f $pid ]; then
+ TARGET_PID=`cat $pid`
+ server_running_count=`ps -p $TARGET_PID --no-headers | wc -l`
Review Comment:
> It seems like `--no-headers` can not be running in `MacOS`. It's better to use a compatible way.
@SbloodyS Thanks a lot for pointing out that, I have removed `--ho-headers`.
<img width="370" alt="image" src="https://user-images.githubusercontent.com/38122586/198925427-aa31bfb2-ed5a-4d77-b295-256e762a4cdc.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] rickchengx commented on a diff in pull request #12118: [Fix-12109] Fix the errors when starting 2 times with dolphinscheduler-daemon.sh
Posted by GitBox <gi...@apache.org>.
rickchengx commented on code in PR #12118:
URL: https://github.com/apache/dolphinscheduler/pull/12118#discussion_r1009002835
##########
script/dolphinscheduler-daemon.sh:
##########
@@ -81,8 +79,26 @@ else
exit 1
fi
+state=""
+function get_server_running_status() {
+ state="STOP"
+ if [ -f $pid ]; then
+ TARGET_PID=`cat $pid`
+ server_running_count=`ps -p $TARGET_PID --no-headers | wc -l`
Review Comment:
> It seems like `--no-headers` can not be running in `MacOS`. It's better to use a compatible way.
@SbloodyS Thanks a lot for pointing out that, I have removed `--ho-headers`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org