[GitHub] [incubator-dolphinscheduler] legen618 commented on a change in pull request #3328: [bugfix] issue #3324

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

legen618 commented on a change in pull request #3328:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3328#discussion_r462291941



##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/master/runner/MasterExecThread.java
##########
@@ -682,10 +687,16 @@ private DependResult isTaskDepsComplete(String taskName) {
                 return DependResult.WAITING;
             }
             ExecutionStatus depTaskState = completeTaskList.get(depsNode).getState();
-            // conditions task would not return failed.
-            if(depTaskState.typeIsFailure()
-                    && !DagHelper.haveConditionsAfterNode(depsNode, dag )
-                    && !dag.getNode(depsNode).isConditionsTask()){
+            // conditions task should be handled separately
+            if (dag.getNode(depsNode).isConditionsTask()) {
+                List<String> tmpTaskList = parseConditionTask(depsNode);
+                if (tmpTaskList.contains(taskName)){

Review comment:
       In fact, in the loop it is supposed to return FAILED when dependency is not satisfied. And if all dependencies are satisfied it will **return SUCCESS outside the loop.**

##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/master/runner/MasterExecThread.java
##########
@@ -670,6 +670,11 @@ private DependResult isTaskDepsComplete(String taskName) {
         }
 
         TaskNode taskNode = dag.getNode(taskName);
+        // condition node directly return success
+        if (taskNode.isConditionsTask()) {
+            return DependResult.SUCCESS;

Review comment:
       sorry, I didn't think it through. You are right, it needs to check all depend nodes. **So, the three lines here is not necessary.**




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org