Conversation
If there is a branch that fork from a cycle in the workflow graph, during runtime and the workflow runs the cycle more than once, tasks under the forked branch will not execute a second time. The issue is caused by the networkx graph not recognizing the forked branch as part of the cycle. The conductor is changed to determine a task is executed more than once as part of the cycle when the last task entry is completed and the new task status is one of the starting execution statuses.
| toil: | ||
| action: core.echo message="This is hard work." | ||
| next: | ||
| - do: query |
There was a problem hiding this comment.
The workflow here is infinity loop, right?
There was a problem hiding this comment.
@jinpingh If you look at the workflow definition and the unit test closely, it is not an infinitely loop practically. In the unit test, I'm controlling the result that is returned by the query task which controls when the loop terminates.
There was a problem hiding this comment.
Understood that unit test takes care of when the loop terminates. Just want to make sure that I understand how this fork working. :)
There was a problem hiding this comment.
Gotcha. You are asking the right questions.
| # misses forks that extends from the cycle. The check here assumes that the | ||
| # last task entry is already completed and the new task status is one of the | ||
| # starting statuses, then there is high likelihood that this is a cycle. | ||
| if (task_state_entry.get('status') in statuses.COMPLETED_STATUSES and |
There was a problem hiding this comment.
Above line 655 commented If task is already completed and in cycle, the code here did not check if task is in cycle self.graph.in_cycle(task_id).
There was a problem hiding this comment.
The comment If task is already completed and in cycle is saying what must be checked. But if you look at the description right after that, because the method in networkx library to check cycle is too simple, here is what we are doing.
There was a problem hiding this comment.
Should change comment to remove cycle?
There was a problem hiding this comment.
No, we shouldn't change comment to remove "cycle". We want to detect whether the current task is part of a cycle. It is just that the method in networkx is unable to determine that this very specific use case with a fork stems from a cycle.
|
@jinpingh |
Thanks! |
If there is a branch that fork from a cycle in the workflow graph, during runtime and the workflow runs the cycle more than once, tasks under the forked branch will not execute a second time. The issue is caused by the networkx graph not recognizing the forked branch as part of the cycle. The conductor is changed to determine a task is executed more than once as part of the cycle when the last task entry is completed and the new task status is one of the starting execution statuses. Fixes #169