Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the problem of Workflow terminates after parallel tasks execution, merge node not triggered #12498

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

lazyFrogLOL
Copy link

Summary

Fix #12492

Description

When both if conditions are satisfied, the subsequent node is executed incompletely due to the following reasons:

  1. When init a Graph object, the _recursively_add_parallels method is used to create the mapping for parallel branches. When a node is found to have multiple outgoing edges, it is identified as the starting point of a parallel branch.
  2. In the GraphEngine's _run method, when multiple outgoing edges are encountered:
else:
    parallel_generator = self._run_parallel_branches(
        edge_mappings=edge_mappings,
        in_parallel_id=in_parallel_id,
        parallel_start_node_id=parallel_start_node_id,
        handle_exceptions=handle_exceptions,
    )

_run_parallel_branches creates a new thread for each branch to execute.

However:

  • When one branch completes execution, it places the result in the queue.
  • Once all branches have completed execution, the next node is executed.
  • If multiple branches satisfy the conditions and point to the same target node, only the first branch to complete will actually execute the target node.

Major Modifications:

  1. Collect all target nodes from the branches.
  2. For multiple branches pointing to the same target node, check which branches satisfy the conditions.
  3. Ensure that all branches meeting the conditions can trigger the execution of the target node.

This ensures that when multiple if condition branches are satisfied, their shared subsequent nodes are executed correctly.

Screenshots

Before After
image image

Checklist

Important

Please review the checklist below before submitting your pull request.

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. 🌊 feat:workflow Workflow related stuff. 🐞 bug Something isn't working labels Jan 8, 2025
@lazyFrogLOL lazyFrogLOL changed the base branch from deploy/dev to main January 8, 2025 11:27
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jan 8, 2025
@crazywoola crazywoola requested a review from laipz8200 January 8, 2025 12:01
@lazyFrogLOL
Copy link
Author

I found that this issue still exists in some scenarios and has not been fully resolved. Please hold off on merging this pull request.

@lazyFrogLOL
Copy link
Author

The new code passes all graph engine unit tests and has been verified in several corner cases tested on the web app. Please review the code thoroughly and engage in a detailed discussion to ensure that the issue is truly resolved without breaking any existing functionality.

@yihong0618
Copy link
Contributor

The new code passes all graph engine unit tests and has been verified in several corner cases tested on the web app. Please review the code thoroughly and engage in a detailed discussion to ensure that the issue is truly resolved without breaking any existing functionality.

amazing work!

@yihong0618
Copy link
Contributor

you need to fix the mypy typehint

@lazyFrogLOL
Copy link
Author

you need to fix the mypy typehint

thx for the reminder.

@lazyFrogLOL
Copy link
Author

you need to fix the mypy typehint

thx for the reminder.

Done the file reformating and lint fixing.

@lazyFrogLOL
Copy link
Author

@yihong0618

you need to fix the mypy typehint

thx for the reminder.

Done the file reformating and lint fixing.

@yihong0618

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 🌊 feat:workflow Workflow related stuff. size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workflow terminates after parallel tasks execution, merge node not triggered
2 participants