[GitHub] [dolphinscheduler] DarkAssassinator commented on a diff in pull request #12051: [Improvement][Master] fix issue#12001

[GitBox <gi...@apache.org>]

DarkAssassinator commented on code in PR #12051:
URL: https://github.com/apache/dolphinscheduler/pull/12051#discussion_r975420411


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/task/CommonTaskProcessor.java:
##########
@@ -126,6 +133,14 @@ public boolean dispatchTask() {
                 return false;
             }
 
+            // check in advance to avoid invalid infinite loops in TaskPriorityQueueConsumer
+            if (CollectionUtils.isEmpty(serverNodeManager.getWorkerGroupNodes(taskExecutionContext.getWorkerGroup()))) {

Review Comment:
   > We cannot make this change easily, the workgroup may loss in some cases, and recovery automically.
   > 
   > And this is not safe, the workflow may exist in the processor but loss in dispatch part.
   > 
   > BTY, it's not suggested to do this validation master, we need to do the validation in api-side.
   
   but if ds want to worker group recovery automically, we also should not add this validation in api-side,just keep loop waiting. WDYT @ruanwenjun 



-- 
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