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/19 12:58:18 UTC
[GitHub] [dolphinscheduler] DarkAssassinator opened a new pull request, #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
DarkAssassinator opened a new pull request, #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050
<!--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
fix #12000
## Brief change log
Add `registryWorkerGroupNodes` and `dbWorkerGroupNodes` to save worker group from registry center and db.
registry remove event will remove the records from `registryWorkerGroupNodes`, update/insert will keep same.
`dbWorkerGroupNodes` will clear and then addAll each 10s.
## Verify this pull request
has test on TR.
--
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 pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1256159671
> > > When we delete a workerGroup, the TaskInstance should dispatch failed, I think this is our expected. Can we only clear the `workerGroupNodes` map?
> >
> >
> > @ruanwenjun but if we just clear the `workerGroupNodes`, it will affect other normal taskInstance. and i think it will better that keep the memory same as the db and registry center. WDYT
>
> Yes, we are talking about how to make the data in memory is the same with db and registry, currently remove the worker which is deleted, just replace the worker group exist. I think we can replace the workerGroup data in memory when sync.
`WorkerGroup` in the current memory is composed of two parts of data, one of which is synchronized from the database every 10 seconds(just add but never delete), and the other is obtained from the registry center( also just add and never delete).
But we cannot make sure this WorkerGroup from DB or Registry center, so cannot make sure which record has been deleted from db or registry center, unless only `Default` keep on registry center. Then my PR just **replace all worker group from DB when sync, and remove this worker group when this record was delete from registry center**, then combing these two part and refresh `workerGroupNodes`.
--
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 #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#discussion_r977841470
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -331,15 +353,68 @@
MASTER_SIZE = nodes.size();
MASTER_SLOT = index;
} else {
- logger.warn("current addr:{} is not in active master list", masterConfig.getMasterAddress());
- }
- logger.info("update master nodes, master size: {}, slot: {}, addr: {}", MASTER_SIZE, MASTER_SLOT,
+ logger.warn("current addr:{} is not in active master list",
masterConfig.getMasterAddress());
+ }
+ logger.info("update master nodes, master size: {}, slot: {}, addr: {}", MASTER_SIZE,
+ MASTER_SLOT, masterConfig.getMasterAddress());
} finally {
masterLock.unlock();
}
}
+ /**
+ * sync worker group nodes from registry center
+ *
+ * @param workerGroup worker group
+ * @param nodes worker nodes
+ * @param type event type
+ */
+ private void syncWorkerGroupNodesFromRegistry(String workerGroup, Collection<String> nodes,
+ Type type) {
+ registryWorkerGroupWriteLock.lock();
+ try {
+ if (type == Type.REMOVE) {
+ if (!registryWorkerGroupNodes.contains(workerGroup)) {
Review Comment:
done
--
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 pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1257355617
> > > > When we delete a workerGroup, the TaskInstance should dispatch failed, I think this is our expected. Can we only clear the `workerGroupNodes` map?
> > >
> > >
> > > @ruanwenjun but if we just clear the `workerGroupNodes`, it will affect other normal taskInstance. and i think it will better that keep the memory same as the db and registry center. WDYT
> >
> >
> > Yes, we are talking about how to make the data in memory is the same with db and registry, currently remove the worker which is deleted, just replace the worker group exist. I think we can replace the workerGroup data in memory when sync.
>
> `WorkerGroup` in the current memory is composed of two parts of data, one of which is synchronized from the database every 10 seconds(just add but never delete), and the other is obtained from the registry center( also just add and never delete). But we cannot make sure this WorkerGroup from DB or Registry center, so cannot make sure which record has been deleted from db or registry center, unless only `Default` keep on registry center. Then my PR just **replace all worker group from DB when sync, and remove this worker group when this record was delete from registry center**, then combing these two part and refresh `workerGroupNodes`.
We don't need to care about the workgroup in registry center, we only need to make sure the workgroup in memory is same with db, the worker group in registry center will not display in ui, and it is not correcttly, in same case will cause new problems, e.g. we only config the workerGroupA:10.10.10.10 in ui, but we register 11.11.11.11 as workerGroupA in registry, so the task belongs to workerGroupA can be dispatched 11.11.11.11, this is not our expected.
--
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 pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1252418300
could help to ru-run this CodeQL check?
--
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 #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1253097017
> could help to ru-run this CodeQL check?
The Github actions can be triggered by `close/reopen` this PR or `git push`.
--
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 diff in pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#discussion_r978446024
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -81,11 +82,29 @@ public class ServerNodeManager implements InitializingBean {
private final ReentrantReadWriteLock.ReadLock workerNodeInfoReadLock = workerNodeInfoLock.readLock();
private final ReentrantReadWriteLock.WriteLock workerNodeInfoWriteLock = workerNodeInfoLock.writeLock();
+ private final ReentrantReadWriteLock registryWorkerGroupLock = new ReentrantReadWriteLock();
+ private final ReentrantReadWriteLock.ReadLock registryWorkerGroupReadLock = registryWorkerGroupLock.readLock();
+ private final ReentrantReadWriteLock.WriteLock registryWorkerGroupWriteLock = registryWorkerGroupLock.writeLock();
+
+ private final ReentrantReadWriteLock dbWorkerGroupLock = new ReentrantReadWriteLock();
+ private final ReentrantReadWriteLock.ReadLock dbWorkerGroupReadLock = dbWorkerGroupLock.readLock();
+ private final ReentrantReadWriteLock.WriteLock dbWorkerGroupWriteLock = dbWorkerGroupLock.writeLock();
Review Comment:
I don' t think we need to add these locks and the `registryWorkerGroupNodes`, `dbWorkerGroupNodes` these two map.
--
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 #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#discussion_r978602838
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -81,11 +82,29 @@ public class ServerNodeManager implements InitializingBean {
private final ReentrantReadWriteLock.ReadLock workerNodeInfoReadLock = workerNodeInfoLock.readLock();
private final ReentrantReadWriteLock.WriteLock workerNodeInfoWriteLock = workerNodeInfoLock.writeLock();
+ private final ReentrantReadWriteLock registryWorkerGroupLock = new ReentrantReadWriteLock();
+ private final ReentrantReadWriteLock.ReadLock registryWorkerGroupReadLock = registryWorkerGroupLock.readLock();
+ private final ReentrantReadWriteLock.WriteLock registryWorkerGroupWriteLock = registryWorkerGroupLock.writeLock();
+
+ private final ReentrantReadWriteLock dbWorkerGroupLock = new ReentrantReadWriteLock();
+ private final ReentrantReadWriteLock.ReadLock dbWorkerGroupReadLock = dbWorkerGroupLock.readLock();
+ private final ReentrantReadWriteLock.WriteLock dbWorkerGroupWriteLock = dbWorkerGroupLock.writeLock();
Review Comment:
> I don' t think we need to add these locks and the `registryWorkerGroupNodes`, `dbWorkerGroupNodes` these two map.
yes, u are right, these locks are really unnecessary
--
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 pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1255988702
> > When we delete a workerGroup, the TaskInstance should dispatch failed, I think this is our expected. Can we only clear the `workerGroupNodes` map?
>
> @ruanwenjun but if we just clear the `workerGroupNodes`, it will affect other normal taskInstance. and i think it will better that keep the memory same as the db and registry center. WDYT
Yes, we are talking about how to make the data in memory is the same with db and registry, currently remove the worker which is deleted, just replace the worker group exist. I think we can replace the workerGroup data in memory when sync.
--
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 #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1255358545
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12050)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&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] commented on pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1256384137
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12050)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&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] codecov-commenter commented on pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1255171322
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/12050?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 [#12050](https://codecov.io/gh/apache/dolphinscheduler/pull/12050?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (630625b) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/8cddb10773fe8f7b58533b8e089cabc87622a039?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8cddb10) will **decrease** coverage by `0.01%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## dev #12050 +/- ##
============================================
- Coverage 38.65% 38.64% -0.02%
+ Complexity 4005 4004 -1
============================================
Files 1002 1001 -1
Lines 37213 37431 +218
Branches 4249 4257 +8
============================================
+ Hits 14386 14465 +79
- Misses 21195 21321 +126
- Partials 1632 1645 +13
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/12050?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uler/server/master/registry/ServerNodeManager.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9yZWdpc3RyeS9TZXJ2ZXJOb2RlTWFuYWdlci5qYXZh) | `0.95% <0.00%> (-0.24%)` | :arrow_down: |
| [...scheduler/plugin/task/mlflow/MlflowParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stbWxmbG93L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL21sZmxvdy9NbGZsb3dQYXJhbWV0ZXJzLmphdmE=) | `18.18% <0.00%> (-56.05%)` | :arrow_down: |
| [...heduler/plugin/task/jupyter/JupyterParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stanVweXRlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vdGFzay9qdXB5dGVyL0p1cHl0ZXJQYXJhbWV0ZXJzLmphdmE=) | `0.00% <0.00%> (-33.34%)` | :arrow_down: |
| [...hinscheduler/plugin/alert/script/ScriptSender.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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==) | `53.33% <0.00%> (-7.28%)` | :arrow_down: |
| [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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: |
| [...org/apache/dolphinscheduler/remote/utils/Host.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL3V0aWxzL0hvc3QuamF2YQ==) | `42.55% <0.00%> (-2.13%)` | :arrow_down: |
| [.../dolphinscheduler/plugin/task/spark/SparkTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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%> (-2.00%)` | :arrow_down: |
| [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `51.42% <0.00%> (-1.43%)` | :arrow_down: |
| [...phinscheduler/plugin/task/jupyter/JupyterTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stanVweXRlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vdGFzay9qdXB5dGVyL0p1cHl0ZXJUYXNrLmphdmE=) | `62.85% <0.00%> (-0.61%)` | :arrow_down: |
| [...cheduler/api/service/impl/ExecutorServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9FeGVjdXRvclNlcnZpY2VJbXBsLmphdmE=) | `43.33% <0.00%> (-0.58%)` | :arrow_down: |
| ... and [45 more](https://codecov.io/gh/apache/dolphinscheduler/pull/12050/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 #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1255358675
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12050)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&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 merged pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
caishunfeng merged PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050
--
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 pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1255735828
> When we delete a workerGroup, the TaskInstance should dispatch failed, I think this is our expected. Can we only clear the `workerGroupNodes` map?
@ruanwenjun but if we just clear the `workerGroupNodes`, it will affect other normal taskInstance. and i think it will better that keep the memory same as the db and registry center. 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] sonarcloud[bot] commented on pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1256378018
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12050)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12050&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12050&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] github-code-scanning[bot] commented on a diff in pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#discussion_r977765178
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -331,15 +353,68 @@
MASTER_SIZE = nodes.size();
MASTER_SLOT = index;
} else {
- logger.warn("current addr:{} is not in active master list", masterConfig.getMasterAddress());
- }
- logger.info("update master nodes, master size: {}, slot: {}, addr: {}", MASTER_SIZE, MASTER_SLOT,
+ logger.warn("current addr:{} is not in active master list",
masterConfig.getMasterAddress());
+ }
+ logger.info("update master nodes, master size: {}, slot: {}, addr: {}", MASTER_SIZE,
+ MASTER_SLOT, masterConfig.getMasterAddress());
} finally {
masterLock.unlock();
}
}
+ /**
+ * sync worker group nodes from registry center
+ *
+ * @param workerGroup worker group
+ * @param nodes worker nodes
+ * @param type event type
+ */
+ private void syncWorkerGroupNodesFromRegistry(String workerGroup, Collection<String> nodes,
+ Type type) {
+ registryWorkerGroupWriteLock.lock();
+ try {
+ if (type == Type.REMOVE) {
+ if (!registryWorkerGroupNodes.contains(workerGroup)) {
Review Comment:
## Type mismatch on container access
Actual argument type 'String' is incompatible with expected argument type 'Set'.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1534)
--
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 pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#issuecomment-1255722763
When we delete a workerGroup, the TaskInstance should dispatch failed, I think this is our expected.
--
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 diff in pull request #12050: [fix#12000]Cannot remove the WorkerGroup from the master service
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #12050:
URL: https://github.com/apache/dolphinscheduler/pull/12050#discussion_r978691330
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -140,48 +151,42 @@ public static int getMasterSize() {
return MASTER_SIZE;
}
+
/**
* init listener
*
* @throws Exception if error throws Exception
*/
@Override
public void afterPropertiesSet() throws Exception {
- /**
- * load nodes from zookeeper
- */
+
+ // load nodes from zookeeper
load();
- /**
- * init executor service
- */
+
+ // init executor service
executorService =
- Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("ServerNodeManagerExecutor"));
+ Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("ServerNodeManagerExecutor"));
executorService.scheduleWithFixedDelay(new WorkerNodeInfoAndGroupDbSyncTask(), 0, 10, TimeUnit.SECONDS);
- /*
- * init MasterNodeListener listener
- */
+
+ // init MasterNodeListener listener
registryClient.subscribe(REGISTRY_DOLPHINSCHEDULER_MASTERS, new MasterDataListener());
- /*
- * init WorkerNodeListener listener
- */
+
+ // init WorkerNodeListener listener
registryClient.subscribe(REGISTRY_DOLPHINSCHEDULER_WORKERS, new WorkerDataListener());
}
/**
* load nodes from zookeeper
*/
public void load() {
- /*
- * master nodes from zookeeper
- */
+ // master nodes from zookeeper
updateMasterNodes();
- /*
- * worker group nodes from zookeeper
- */
+ // worker group nodes from zookeeper
Collection<String> workerGroups = registryClient.getWorkerGroupDirectly();
for (String workerGroup : workerGroups) {
- syncWorkerGroupNodes(workerGroup, registryClient.getWorkerGroupNodesDirectly(workerGroup));
+ syncWorkerGroupNodesFromRegistry(workerGroup,
+ registryClient.getWorkerGroupNodesDirectly(workerGroup), null);
Review Comment:
The null type seems a little weird.
```suggestion
registryClient.getWorkerGroupNodesDirectly(workerGroup), Type.ADD);
```
--
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