ruanwenjun commented on code in PR #16883:
URL:
https://github.com/apache/dolphinscheduler/pull/16883#discussion_r1872822730
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/cluster/WorkerClusters.java:
##########
@@ -149,12 +169,12 @@ public void onServerAdded(WorkerServerMetadata
workerServer) {
@Override
public void onServerRemove(WorkerServerMetadata workerServer) {
workerMapping.remove(workerServer.getAddress(), workerServer);
- synchronized (workerGroupMapping) {
- List<String> removeWorkerGroupAddrList =
workerGroupMapping.get(workerServer.getWorkerGroup());
+ synchronized (configWorkerGroupMapping) {
+ List<String> removeWorkerGroupAddrList =
configWorkerGroupMapping.get(workerServer.getWorkerGroup());
Review Comment:
I just find the worker group at `WorkerServerMetadata` can be null, we need
to make sure this value not null when worker submit heartbeat, if the value is
null, we need to set to default at worker side.
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/cluster/WorkerClusters.java:
##########
@@ -59,27 +62,44 @@ public Optional<WorkerServerMetadata> getServer(final
String address) {
return Optional.ofNullable(workerMapping.get(address));
}
- public List<String> getWorkerServerAddressByGroup(String workerGroup) {
+ public List<String> getDbWorkerServerAddressByGroup(String workerGroup) {
+ if (WorkerGroupUtils.getDefaultWorkerGroup().equals(workerGroup)) {
+ return UnmodifiableList.unmodifiableList(new
ArrayList<>(workerMapping.keySet()));
+ }
Review Comment:
I am not sure if we still need to keep `default` logic here.
In the past, the default worker group is a logic worker group, it will
contain all worker.
Right now, if a worker configures its worker group at config e.g.
spark_worker_group, then we may don't need to auto put it at default group.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
