Event subprocess attaches to inner instance of multi-instance only #12877
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
An event sub process existing inside a multi-instance sub process should exist for each inner instance of the multi-instance. Each event sub process should be able to be triggered individually. There was a bug #11578, where the event sub process's message start event could not create a message subscription because the correlation key expression referred to the input element. The input element exists on each inner instance, but the engine attempted to subscribe to the event sub process before activating any of the inner instances. In turn, this also means that the event sub processes couldn't be triggered. Instead of testing that we can subscribe to each event sub process, it makes more sense to also test that we can trigger each individually. This tests the reason why we want to be able to subscribe in the first place.
The event sub process should only be attached to the inner instance of the multi-instance marked element. Not to the body. The event sub process should be triggered for a specific instance of a multi-instance marked element only. It only exists in the flow scope of the inner instance, not in the flow scope of the multi-instance body. There was a bug #11578 with this as the root problem. It surfaced because the event sub process's start event had a correlation key expression that referred to the input element as variable. That variable had not yet been created when we subscribed the multi-instance body to the event sub process. We should not have attempted to subscribe the multi-instance body to the event sub process. Instead, each inner instance of the multi-instance should be subscribed to its own instance of the event sub process. See the previous commit for a test case with that scenario.
korthout
added
backport stable/8.0
backport stable/8.2
|
@saig0 The fix for this bug was removing a line of code you added purposefully in another pull request: Do you have a chance to review this? |
zeebe-bors-camunda bot
added a commit
that referenced
this pull request
May 30, 2023
12877: Event subprocess attaches to inner instance of multi-instance only r=saig0 a=korthout ## Description <!-- Please explain the changes you made here. --> Event sub processes inside a multi-instance sub process would be attached to the multi-instance body in addition to the inner instance. However, there is no reason why an event sub process should be attached to the multi-instance body. Instead, event sub processes should only be attached to the inner instances. **Example scenario** The parallel multi-instance sub process creates an `item` variable as input element in each inner instance. <img width="865" alt="Screenshot 2023-05-26 at 18 25 08" src="https://github.com/camunda/zeebe/assets/3511026/cf37717b-50cf-4478-84d3-d32009f54972"> Each message event sub process subscribes to the same message with a correlation key expression that uses the input element variable `item`. <img width="922" alt="Screenshot 2023-05-26 at 18 25 39" src="https://github.com/camunda/zeebe/assets/3511026/9fc40f17-a2e7-4628-a9fd-a70cb6bffe90"> The bug will surface as an incident on the multi-instance body stating: >failed to evaluate expression 'item.id': no variable found for name 'item' This was because we attempted to subscribe to the event sub process already when activating the multi-instance body, and **before** activating any of the inner instances. The problem is resolved by no longer attaching the event sub process to the multi-instance body. ## Related issues <!-- Which issues are closed by this PR or are related --> closes #11578 Co-authored-by: Nico Korthout <nico.korthout@camunda.com>
|
Build failed: |
|
Flaky test: bors retry |
zeebe-bors-camunda bot
added a commit
that referenced
this pull request
May 30, 2023
12698: Replace go-bindata with go's embed FS r=npepinpe a=npepinpe ## Description Took some slack time to complete this quick win: replaces usage of go-bindata, an external tool dependency, with the built-in capabilities of the go compiler using `go:embed` and the embedFS. The version file is embedded at build time into the binary, much like when we used go-bindata, and read back as a package field. ## Related issues closes #6034 12868: fix(journal): avoid gaps in logs due to reset r=npepinpe a=npepinpe ## Description Avoid gaps in the log when resetting the log by deleting the segments in reverse order. This may cause data loss, but that's acceptable in the context of a reset operation. Deleting segments in reverse order will ensure we have no gaps between the remaining snapshot and segments should the operation fail midway. Note that this does not make persisting a new snapshot/reseting the log atomic unfortunately. Unfortunately tests were also not ideal, and require heavy use of mocks. It's also not possible to write integration test without major refactoring. As this is a quick fix, the hope is that the proper solution to make the Raft install operation atomic will redeem this. Not marked as closing the original issue because this is, after all, a quick fix. It only fixes a specific case we're aware of. ## Related issues <!-- Which issues are closed by this PR or are related --> related to #12754 12877: Event subprocess attaches to inner instance of multi-instance only r=saig0 a=korthout ## Description <!-- Please explain the changes you made here. --> Event sub processes inside a multi-instance sub process would be attached to the multi-instance body in addition to the inner instance. However, there is no reason why an event sub process should be attached to the multi-instance body. Instead, event sub processes should only be attached to the inner instances. **Example scenario** The parallel multi-instance sub process creates an `item` variable as input element in each inner instance. <img width="865" alt="Screenshot 2023-05-26 at 18 25 08" src="https://github.com/camunda/zeebe/assets/3511026/cf37717b-50cf-4478-84d3-d32009f54972"> Each message event sub process subscribes to the same message with a correlation key expression that uses the input element variable `item`. <img width="922" alt="Screenshot 2023-05-26 at 18 25 39" src="https://github.com/camunda/zeebe/assets/3511026/9fc40f17-a2e7-4628-a9fd-a70cb6bffe90"> The bug will surface as an incident on the multi-instance body stating: >failed to evaluate expression 'item.id': no variable found for name 'item' This was because we attempted to subscribe to the event sub process already when activating the multi-instance body, and **before** activating any of the inner instances. The problem is resolved by no longer attaching the event sub process to the multi-instance body. ## Related issues <!-- Which issues are closed by this PR or are related --> closes #11578 Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com> Co-authored-by: Nico Korthout <nico.korthout@camunda.com>
|
Build failed (retrying...): |
|
Build succeeded: |
zeebe-bors-camunda
bot
deleted the
korthout-11578-event-subprocess-attached-to-multi-instance-body
branch
|
Successfully created backport PR for |
|
Successfully created backport PR for |
This was referenced May 30, 2023
Merged
|
Successfully created backport PR for |
zeebe-bors-camunda bot
added a commit
that referenced
this pull request
May 31, 2023
12890: [Backport stable/8.0] Event subprocess attaches to inner instance of multi-instance only r=korthout a=backport-action # Description Backport of #12877 to `stable/8.0`. relates to #11578 12908: [Backport stable/8.0] Remove JDK 8 specific build r=oleschoenburg a=npepinpe ## Description This PR backports removing the JDK 8 build, and upgrading Mockito to 5. ## Related issues backport #12533 Co-authored-by: Nico Korthout <nico.korthout@camunda.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
lenaschoenburg
added
version:8.1.13
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Event sub processes inside a multi-instance sub process would be attached to the multi-instance body in addition to the inner instance. However, there is no reason why an event sub process should be attached to the multi-instance body. Instead, event sub processes should only be attached to the inner instances.
Example scenario
The parallel multi-instance sub process creates an
itemvariable as input element in each inner instance.Each message event sub process subscribes to the same message with a correlation key expression that uses the input element variable
item.The bug will surface as an incident on the multi-instance body stating:
This was because we attempted to subscribe to the event sub process already when activating the multi-instance body, and before activating any of the inner instances.
The problem is resolved by no longer attaching the event sub process to the multi-instance body.
Related issues
closes #11578
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/1.3) to the PR, in case that fails you need to create backports manually.Testing:
Documentation:
Other teams:
If the change impacts another team an issue has been created for this team, explaining what they need to do to support this change.
Please refer to our review guidelines.