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/11/23 13:50:56 UTC
[GitHub] [dolphinscheduler] DarkAssassinator opened a new pull request, #12980: [Improvement-12907]adjust log level and refactor
DarkAssassinator opened a new pull request, #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980
<!--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
+ This PR will close #12907
## Brief change log
+ Adjust log level: info -> debug. Because these heartbeat log will print each 10s.
+ Migrate some private methods to listener class. Becasue these private method is only invoked by an inner class.
+ Remove some fields, because these field will hides the field declared aboved.
## 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.* -->
(or)
If your pull request contain incompatible change, you should also add it to `docs/docs/en/guide/upgrede/incompatible.md`
--
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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1031019955
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -382,10 +366,8 @@ public synchronized void addWorkerInfoChangeListener(WorkerInfoChangeListener li
}
private void notifyWorkerInfoChangeListeners() {
- Map<String, Set<String>> workerGroupNodes = getWorkerGroupNodes();
- Map<String, WorkerHeartBeat> workerNodeInfo = getWorkerNodeInfo();
for (WorkerInfoChangeListener listener : workerInfoChangeListeners) {
- listener.notify(workerGroupNodes, workerNodeInfo);
+ listener.notify(getWorkerGroupNodes(), getWorkerNodeInfo());
Review Comment:
ohh, my mistake. This is a loop, my original intention is to remove the fields that have the same name as the fields defined above. I will rollback it and rename these two fields.
--
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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#issuecomment-1325143899
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12980)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list) [14.3% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&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 diff in pull request #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1031001423
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -382,10 +366,8 @@ public synchronized void addWorkerInfoChangeListener(WorkerInfoChangeListener li
}
private void notifyWorkerInfoChangeListeners() {
- Map<String, Set<String>> workerGroupNodes = getWorkerGroupNodes();
- Map<String, WorkerHeartBeat> workerNodeInfo = getWorkerNodeInfo();
for (WorkerInfoChangeListener listener : workerInfoChangeListeners) {
- listener.notify(workerGroupNodes, workerNodeInfo);
+ listener.notify(getWorkerGroupNodes(), getWorkerNodeInfo());
Review Comment:
> I don't think this will make the code clean.
+1, I think we should not call `getWorkerGroupNodes` and `getWorkerNodeInfo` in the loop, although they just operate in memory.
--
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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#issuecomment-1326486773
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12980)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list) [11.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&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] DarkAssassinator commented on a diff in pull request #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1031022659
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -153,29 +153,15 @@ class WorkerNodeInfoAndGroupDbSyncTask implements Runnable {
@Override
public void run() {
try {
-
// sync worker node info
updateWorkerNodes();
updateWorkerGroupMappings();
- notifyWorkerInfoChangeListeners();
Review Comment:
> Why you remove this? this is used to notifier the `LowerWeightHostManager`
Because `updateWorkerGroupMapping` has notify(has invoke `notifyWorkerInfoChangeListener`). So we do not need to repeat it a second time.
--
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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1031002739
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -382,10 +366,8 @@ public synchronized void addWorkerInfoChangeListener(WorkerInfoChangeListener li
}
private void notifyWorkerInfoChangeListeners() {
- Map<String, Set<String>> workerGroupNodes = getWorkerGroupNodes();
- Map<String, WorkerHeartBeat> workerNodeInfo = getWorkerNodeInfo();
for (WorkerInfoChangeListener listener : workerInfoChangeListeners) {
- listener.notify(workerGroupNodes, workerNodeInfo);
+ listener.notify(getWorkerGroupNodes(), getWorkerNodeInfo());
Review Comment:
> I don't think this will make the code clean.
+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] ruanwenjun merged pull request #12980: [Improvement-12907] Change heartbeat log level to debug
Posted by GitBox <gi...@apache.org>.
ruanwenjun merged PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980
--
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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#issuecomment-1326611327
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12980)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list) [9.1% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#issuecomment-1325136656
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12980)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list) [14.3% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&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 diff in pull request #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1030989212
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -241,8 +235,7 @@ private void updateMasterNodes() {
try {
registryClient.getLock(nodeLock);
Collection<String> currentNodes = registryClient.getMasterNodesDirectly();
- List<Server> masterNodes = registryClient.getServerList(NodeType.MASTER);
- syncMasterNodes(currentNodes, masterNodes);
+ syncMasterNodes(currentNodes, registryClient.getServerList(NodeType.MASTER));
Review Comment:
There is no need to do this code change.
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -153,29 +153,15 @@ class WorkerNodeInfoAndGroupDbSyncTask implements Runnable {
@Override
public void run() {
try {
-
// sync worker node info
updateWorkerNodes();
updateWorkerGroupMappings();
- notifyWorkerInfoChangeListeners();
Review Comment:
Why you remove this? this is used to notifier the `LowerWeightHostManager`
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -382,10 +366,8 @@ public synchronized void addWorkerInfoChangeListener(WorkerInfoChangeListener li
}
private void notifyWorkerInfoChangeListeners() {
- Map<String, Set<String>> workerGroupNodes = getWorkerGroupNodes();
- Map<String, WorkerHeartBeat> workerNodeInfo = getWorkerNodeInfo();
for (WorkerInfoChangeListener listener : workerInfoChangeListeners) {
- listener.notify(workerGroupNodes, workerNodeInfo);
+ listener.notify(getWorkerGroupNodes(), getWorkerNodeInfo());
Review Comment:
I don't think this will make the code clean.
--
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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1031544865
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -153,29 +153,15 @@ class WorkerNodeInfoAndGroupDbSyncTask implements Runnable {
@Override
public void run() {
try {
-
// sync worker node info
updateWorkerNodes();
updateWorkerGroupMappings();
- notifyWorkerInfoChangeListeners();
Review Comment:
> It's better to remove the method in `updateWorkerGroupMappings`
but others also will invoke `updateWorkerGroupMappings`, such as spring bean init. May `notifyWorkerInfoChangeListener` keep in `updateWorkerGroupMappings` should be more clear. 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] ruanwenjun commented on a diff in pull request #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1031527776
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -153,29 +153,15 @@ class WorkerNodeInfoAndGroupDbSyncTask implements Runnable {
@Override
public void run() {
try {
-
// sync worker node info
updateWorkerNodes();
updateWorkerGroupMappings();
- notifyWorkerInfoChangeListeners();
Review Comment:
It's better to remove the method in `updateWorkerGroupMappings`
--
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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1031025062
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -241,8 +235,7 @@ private void updateMasterNodes() {
try {
registryClient.getLock(nodeLock);
Collection<String> currentNodes = registryClient.getMasterNodesDirectly();
- List<Server> masterNodes = registryClient.getServerList(NodeType.MASTER);
- syncMasterNodes(currentNodes, masterNodes);
+ syncMasterNodes(currentNodes, registryClient.getServerList(NodeType.MASTER));
Review Comment:
> There is no need to do this code change.
Because `masterNodes` local variable should not shadow class fields. Override or shadow a variable declared in the outer scope can strongly impact the readability,and it will lead mantainers to introduce bugs because they think they are using one variable but are really using another. 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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#issuecomment-1326482729
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12980)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list) [11.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&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] DarkAssassinator commented on a diff in pull request #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1031629397
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -153,29 +153,15 @@ class WorkerNodeInfoAndGroupDbSyncTask implements Runnable {
@Override
public void run() {
try {
-
// sync worker node info
updateWorkerNodes();
updateWorkerGroupMappings();
- notifyWorkerInfoChangeListeners();
Review Comment:
so i move all update worker node info methods to a private methods.
--
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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1031510924
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -382,10 +366,8 @@ public synchronized void addWorkerInfoChangeListener(WorkerInfoChangeListener li
}
private void notifyWorkerInfoChangeListeners() {
- Map<String, Set<String>> workerGroupNodes = getWorkerGroupNodes();
- Map<String, WorkerHeartBeat> workerNodeInfo = getWorkerNodeInfo();
for (WorkerInfoChangeListener listener : workerInfoChangeListeners) {
- listener.notify(workerGroupNodes, workerNodeInfo);
+ listener.notify(getWorkerGroupNodes(), getWorkerNodeInfo());
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] codecov-commenter commented on pull request #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#issuecomment-1326476160
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/12980?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 [#12980](https://codecov.io/gh/apache/dolphinscheduler/pull/12980?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1bf482) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/50779ea1e6acdbca721dcc3d6331e13687ac9544?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (50779ea) will **decrease** coverage by `0.02%`.
> The diff coverage is `6.06%`.
```diff
@@ Coverage Diff @@
## dev #12980 +/- ##
============================================
- Coverage 39.36% 39.34% -0.03%
+ Complexity 4272 4269 -3
============================================
Files 1067 1067
Lines 40120 40068 -52
Branches 4605 4585 -20
============================================
- Hits 15795 15765 -30
+ Misses 22549 22524 -25
- Partials 1776 1779 +3
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/12980?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/master/dispatch/host/LowerWeightHostManager.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L0xvd2VyV2VpZ2h0SG9zdE1hbmFnZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...uler/server/master/registry/ServerNodeManager.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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.00% <0.00%> (ø)` | |
| [...eduler/server/master/task/MasterHeartBeatTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci90YXNrL01hc3RlckhlYXJ0QmVhdFRhc2suamF2YQ==) | `4.00% <0.00%> (ø)` | |
| [...eduler/server/worker/task/WorkerHeartBeatTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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-ZG9scGhpbnNjaGVkdWxlci13b3JrZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL1dvcmtlckhlYXJ0QmVhdFRhc2suamF2YQ==) | `81.25% <100.00%> (ø)` | |
| [...eduler/api/dto/schedule/ScheduleCreateRequest.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2R0by9zY2hlZHVsZS9TY2hlZHVsZUNyZWF0ZVJlcXVlc3QuamF2YQ==) | `83.33% <0.00%> (-2.39%)` | :arrow_down: |
| [...dolphinscheduler/remote/future/ResponseFuture.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL2Z1dHVyZS9SZXNwb25zZUZ1dHVyZS5qYXZh) | `81.96% <0.00%> (-1.64%)` | :arrow_down: |
| [...e/dolphinscheduler/service/queue/TaskPriority.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvcXVldWUvVGFza1ByaW9yaXR5LmphdmE=) | `51.31% <0.00%> (-1.25%)` | :arrow_down: |
| [...r/plugin/task/sqoop/parameter/SqoopParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stc3Fvb3Avc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svc3Fvb3AvcGFyYW1ldGVyL1Nxb29wUGFyYW1ldGVycy5qYXZh) | `51.19% <0.00%> (-1.20%)` | :arrow_down: |
| [...cheduler/plugin/alert/dingtalk/DingTalkSender.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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: |
| [...pache/dolphinscheduler/common/utils/DateUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0RhdGVVdGlscy5qYXZh) | `68.13% <0.00%> (-0.73%)` | :arrow_down: |
| ... and [28 more](https://codecov.io/gh/apache/dolphinscheduler/pull/12980/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] ruanwenjun commented on a diff in pull request #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#discussion_r1031525086
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -241,8 +235,7 @@ private void updateMasterNodes() {
try {
registryClient.getLock(nodeLock);
Collection<String> currentNodes = registryClient.getMasterNodesDirectly();
- List<Server> masterNodes = registryClient.getServerList(NodeType.MASTER);
- syncMasterNodes(currentNodes, masterNodes);
+ syncMasterNodes(currentNodes, registryClient.getServerList(NodeType.MASTER));
Review Comment:
Make sense.
--
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 #12980: [Improvement-12907]adjust log level and refactor private methods.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12980:
URL: https://github.com/apache/dolphinscheduler/pull/12980#issuecomment-1326620009
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12980)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12980&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list) [9.1% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12980&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