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

Make Parallel Join Spec Compliant #182

Merged
merged 2 commits into from
Aug 29, 2024
Merged

Conversation

sombrek
Copy link
Contributor

@sombrek sombrek commented Aug 6, 2024

Proposed Changes

Closes #130

Parallel gateways now remember which sequence flows already provided a token.
The join activates every time at least one token was provided on all incoming flows. One token per flow is consumed on join.

@barmac
Exiting a sequence flow now provides a source where a token came from. That idea was taken from #143, but this PR does not cover inclusive joins. If the solution is accepted, it could be a starting point for inclusive gateways.

Visual demo

visual-demo

@nikku
Copy link
Member

nikku commented Aug 12, 2024

@sombrek This looks great! I'll check if there is a better way to store incoming tokens than global state tomorrow. Potentially these could be stored in the parent scope.

@nikku nikku self-requested a review August 16, 2024 21:03
@nikku nikku added the needs review Review pending label Aug 16, 2024
@nikku
Copy link
Member

nikku commented Aug 29, 2024

Thanks again for your contribution @sombrek! I was stumbling over the global state you introduced and thought there would be a simpler way, given that we already collect execution via scopes, and hence track "active executions for a given element".

The only missing bit was to apply your filtering logic to the existing scopes scopes on this node, and properly tag initiators when entering a scope (from a sequence flow exit).

Cf. ecfaba1 greatly simplifying your implementation and 59c8511 refactoring the behavior just a bit.

@nikku nikku merged commit c6f00d2 into bpmn-io:main Aug 29, 2024
5 checks passed
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Aug 29, 2024
@nikku
Copy link
Member

nikku commented Aug 29, 2024

CC @barmac because the "source" is gone (again). We track such information via scopes and their hierarchy.

@nikku
Copy link
Member

nikku commented Aug 29, 2024

@sombrek
Copy link
Contributor Author

sombrek commented Aug 29, 2024

I was stumbling over the global state you introduced and thought there would be a simpler way, given that we already collect execution via scopes, and hence track "active executions for a given element".

@nikku Thank you for all the additional work you've put in. I reckon I could have done it without the global state, but I failed to properly translate scopes to my understanding of tokens. I'll study your changes to get a better understanding.

@sombrek sombrek deleted the fix-parallel-join branch August 29, 2024 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parallel Join not behaving correctly
2 participants