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 2021/10/27 14:18:17 UTC
[GitHub] [dolphinscheduler] caishunfeng opened a new pull request #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
caishunfeng opened a new pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621
this pr close #6616
<!--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
<!--(For example: This pull request adds checkstyle plugin).-->
## Brief change log
<!--*(for example:)*
- *Add maven-checkstyle-plugin to root pom.xml*
-->
## Verify this pull request
<!--*(Please pick either of the following options)*-->
This pull request is code cleanup without any test coverage.
*(or)*
This pull request is already covered by existing tests, such as *(please describe tests)*.
(or)
This change added tests and can be verified as follows:
<!--*(example:)*
- *Added dolphinscheduler-dao tests for end-to-end.*
- *Added CronUtilsTest to verify the change.*
- *Manually verified the change by testing locally.* -->
--
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 #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621#issuecomment-953477275
Kudos, SonarCloud Quality Gate passed! 
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_duplicated_lines_density&view=list)
--
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 a change in pull request #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on a change in pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621#discussion_r737960724
##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/registry/WorkerRegistryClient.java
##########
@@ -122,15 +122,20 @@ public void registry() {
* remove registry info
*/
public void unRegistry() {
- String address = getLocalAddress();
- Set<String> workerZkPaths = getWorkerZkPaths();
- for (String workerZkPath : workerZkPaths) {
- registryClient.remove(workerZkPath);
- logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
+ try {
+ String address = getLocalAddress();
+ Set<String> workerZkPaths = getWorkerZkPaths();
+ for (String workerZkPath : workerZkPaths) {
+ registryClient.remove(workerZkPath);
+ logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
+ }
+ this.heartBeatExecutor.shutdownNow();
+ logger.info("heartbeat executor shutdown");
+ registryClient.close();
+ } catch (Exception ex) {
Review comment:
I think it should catch all exception here, because it is the final catch for remove registry info. If it juse catch some exception, the problem will arise 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] sonarcloud[bot] commented on pull request #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621#issuecomment-952994642
Kudos, SonarCloud Quality Gate passed! 
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_duplicated_lines_density&view=list)
--
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] removed a comment on pull request #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621#issuecomment-952994642
Kudos, SonarCloud Quality Gate passed! 
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6621&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6621&metric=new_duplicated_lines_density&view=list)
--
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] ruanwenjun commented on a change in pull request #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621#discussion_r737981062
##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/registry/WorkerRegistryClient.java
##########
@@ -122,15 +122,20 @@ public void registry() {
* remove registry info
*/
public void unRegistry() {
- String address = getLocalAddress();
- Set<String> workerZkPaths = getWorkerZkPaths();
- for (String workerZkPath : workerZkPaths) {
- registryClient.remove(workerZkPath);
- logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
+ try {
+ String address = getLocalAddress();
+ Set<String> workerZkPaths = getWorkerZkPaths();
+ for (String workerZkPath : workerZkPaths) {
+ registryClient.remove(workerZkPath);
+ logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
+ }
+ this.heartBeatExecutor.shutdownNow();
+ logger.info("heartbeat executor shutdown");
+ registryClient.close();
+ } catch (Exception ex) {
Review comment:
What I concern is that when the connection lost, will it reconnect again? Since the catch scope here may cause the registry.close() will not be executed.
And other thing need to confirm, will it cause some thread fail to exit? I am not clear.
--
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] ruanwenjun commented on a change in pull request #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621#discussion_r737943289
##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/registry/WorkerRegistryClient.java
##########
@@ -122,15 +122,20 @@ public void registry() {
* remove registry info
*/
public void unRegistry() {
- String address = getLocalAddress();
- Set<String> workerZkPaths = getWorkerZkPaths();
- for (String workerZkPath : workerZkPaths) {
- registryClient.remove(workerZkPath);
- logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
+ try {
+ String address = getLocalAddress();
+ Set<String> workerZkPaths = getWorkerZkPaths();
+ for (String workerZkPath : workerZkPaths) {
+ registryClient.remove(workerZkPath);
+ logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
+ }
+ this.heartBeatExecutor.shutdownNow();
+ logger.info("heartbeat executor shutdown");
+ registryClient.close();
+ } catch (Exception ex) {
Review comment:
It might better just catch `KeeperException.ConnectionLossException` here?
```suggestion
try {
String address = getLocalAddress();
Set<String> workerZkPaths = getWorkerZkPaths();
for (String workerZkPath : workerZkPaths) {
registryClient.remove(workerZkPath);
logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
}
} catch (KeeperException.ConnectionLossException ex) {
logger.error("remove worker zk path exception", ex)
}
this.heartBeatExecutor.shutdownNow();
logger.info("heartbeat executor shutdown");
registryClient.close();
```
--
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 a change in pull request #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on a change in pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621#discussion_r737991848
##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/registry/WorkerRegistryClient.java
##########
@@ -122,15 +122,20 @@ public void registry() {
* remove registry info
*/
public void unRegistry() {
- String address = getLocalAddress();
- Set<String> workerZkPaths = getWorkerZkPaths();
- for (String workerZkPath : workerZkPaths) {
- registryClient.remove(workerZkPath);
- logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
+ try {
+ String address = getLocalAddress();
+ Set<String> workerZkPaths = getWorkerZkPaths();
+ for (String workerZkPath : workerZkPaths) {
+ registryClient.remove(workerZkPath);
+ logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
+ }
+ this.heartBeatExecutor.shutdownNow();
+ logger.info("heartbeat executor shutdown");
+ registryClient.close();
+ } catch (Exception ex) {
Review comment:
Yes, you're right. It should be better to just try catch of remove zk path, exclude heartBeatExecutor and registryClient close.
--
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] lenboo merged pull request #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
Posted by GitBox <gi...@apache.org>.
lenboo merged pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621
--
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 #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621#issuecomment-953475106
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/6621?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 [#6621](https://codecov.io/gh/apache/dolphinscheduler/pull/6621?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ae7cc57) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/cbeedba3e2cfdf80553da0d798fc71aaa448c6e0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cbeedba) will **decrease** coverage by `0.00%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/dolphinscheduler/pull/6621?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## dev #6621 +/- ##
============================================
- Coverage 38.51% 38.50% -0.01%
- Complexity 3221 3222 +1
============================================
Files 646 646
Lines 25840 25844 +4
Branches 2799 2799
============================================
- Hits 9952 9951 -1
- Misses 14974 14980 +6
+ Partials 914 913 -1
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/6621?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...r/server/worker/registry/WorkerRegistryClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6621/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci9yZWdpc3RyeS9Xb3JrZXJSZWdpc3RyeUNsaWVudC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/dolphinscheduler/server/log/LoggerServer.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6621/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL2xvZy9Mb2dnZXJTZXJ2ZXIuamF2YQ==) | `78.26% <0.00%> (-8.70%)` | :arrow_down: |
| [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6621/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `83.33% <0.00%> (+5.55%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/6621?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/6621?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [cbeedba...ae7cc57](https://codecov.io/gh/apache/dolphinscheduler/pull/6621?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] caishunfeng commented on a change in pull request #6621: [DS-6616][WorkerServer] fix worker stop fail and fakes death
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on a change in pull request #6621:
URL: https://github.com/apache/dolphinscheduler/pull/6621#discussion_r737991848
##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/registry/WorkerRegistryClient.java
##########
@@ -122,15 +122,20 @@ public void registry() {
* remove registry info
*/
public void unRegistry() {
- String address = getLocalAddress();
- Set<String> workerZkPaths = getWorkerZkPaths();
- for (String workerZkPath : workerZkPaths) {
- registryClient.remove(workerZkPath);
- logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
+ try {
+ String address = getLocalAddress();
+ Set<String> workerZkPaths = getWorkerZkPaths();
+ for (String workerZkPath : workerZkPaths) {
+ registryClient.remove(workerZkPath);
+ logger.info("worker node : {} unRegistry from ZK {}.", address, workerZkPath);
+ }
+ this.heartBeatExecutor.shutdownNow();
+ logger.info("heartbeat executor shutdown");
+ registryClient.close();
+ } catch (Exception ex) {
Review comment:
Yes, you're right. It should be better to just try catch of remove zk path, exclude heartBeatExecutor and registryClient close.
I have modified 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