Trigger boundary events after an interrupting event subprocess is triggered #9175

saig0 · 2022-04-20T03:45:39Z

Description

Fixing the issue that boundary events can't be triggered after an interrupting event subprocess is triggered on the scope.

Solve the issue by:

avoid that event subscriptions of boundary event are removed when an interrupting event scope is triggered
- introduce a filter when removing the event subscriptions
- filter by event subprocesses and ignore other event subscriptions
extend the event scope instance state to distinguish interrupting event subscriptions from boundary events
- add the property boundaryElementIds for event scopes to distinguish boundary events from other events in the scope
- add the property interrupted for event scopes to mark if an interrupting event (subprocess) is triggered
- if the event scope is interrupted then no other interrupting or non-interrupting events can be triggered
- only boundary events can be triggered for an interrupted event scope
- if an interrupting boundary event is triggered then the event scope doesn't accept any other event

Maintenance:

remove methods from the event scope state that are not used in production code
migrate the event scope state test to JUnit 5

Related issues

closes #6874

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

The changes are backwards compatibility with previous versions
If it fixes a bug then PRs are created to backport the fix to the last two minor versions. You can trigger a backport by assigning labels (e.g. backport stable/1.3) to the PR, in case that fails you need to create backports manually.

Testing:

There are unit/integration tests that verify all acceptance criterias of the issue
New tests are written to ensure backwards compatibility with further versions
The behavior is tested manually
The change has been verified by a QA run
The impact of the changes is verified by a benchmark

Documentation:

The documentation is updated (e.g. BPMN reference, configuration, examples, get-started guides, etc.)
New content is added to the release announcement
If the PR changes how BPMN processes are validated (e.g. support new BPMN element) then the Camunda modeling team should be informed to adjust the BPMN linting.

* if an interrupting event subprocess is triggered then remove the subscriptions of other event subprocesses in the same flow scope * but don't remove the subscriptions of boundary events that are attached to the scope * the boundary events can be still triggered because they are attached to the scope and are not inside of it

* remove methods from event scope instance state that are only used in tests but not in production code * these methods were used previously but are not needed anymore

* migrate the test case from JUnit 4 to JUnit 5 using the Zeebe state extension

* handle interrupting event subprocesses and interrupting boundary events differently by introducing boundary element ids * an interrupting event subprocess interrupts the event scope, but it can still be triggered by boundary events * after an interrupting boundary event is triggered, no other event can be triggered for the event scope

pihme · 2022-04-20T13:53:52Z

Off the top of my head: Wouldn't we need some sort of migration to initialize io.camunda.zeebe.engine.state.instance.EventScopeInstance#boundaryElementIdsProp?

saig0 · 2022-04-21T04:14:48Z

Wouldn't we need some sort of migration to initialize io.camunda.zeebe.engine.state.instance.EventScopeInstance#boundaryElementIdsProp

Good question 👍 We could migrate the state to fix the behavior of the existing event scopes.

If we don't migrate the state then the behavior is fixed only for event scopes that are created are the update to the new version. But I think this is okay. We're not aware of critical processes that are stuck in this situation.

menski · 2022-04-21T09:08:57Z

I think it would be of course nice to have a fix which also works for existing scopes, do we have any feeling about how complex this migration would be?

We're not aware of critical processes that are stuck in this situation.

I'm always not sure how much we should rely on this, as I'm looking from a user perspective how would I know or understand that the engine has a bug in this case? I.e. a user would have to notice that a boundary event was not triggered, how would the user figure that out?

saig0 · 2022-04-21T09:35:34Z

do we have any feeling about how complex this migration would be?

Mid. We would need to fetch some data to update the event scopes properly. And we would need to test the migration. We could do it but it would take more time.

I think it is not worth implementing the migration. Currently, we have this bug. When the cluster is updated to the new version then the bug will be fixed for any new process instance (i.e. or any new event scope of existing process instances).

If a user has an existing process instance and runs into the bug then the user can start a new process instance to mitigate the issue.

pihme

Reviewed all changes except for tests. I have a couple of questions to further my understanding.

Please have a look. Let's first clear up the questions either here or f2f, as you deem more effective and then I would look at the tests again.

engine/src/main/java/io/camunda/zeebe/engine/processing/common/CatchEventBehavior.java

engine/src/main/java/io/camunda/zeebe/engine/processing/common/EventTriggerBehavior.java

...ain/java/io/camunda/zeebe/engine/processing/deployment/model/element/ExecutableActivity.java

engine/src/main/java/io/camunda/zeebe/engine/processing/timer/TriggerTimerProcessor.java

...ain/java/io/camunda/zeebe/engine/state/appliers/ProcessInstanceElementActivatingApplier.java

engine/src/main/java/io/camunda/zeebe/engine/state/instance/DbEventScopeInstanceState.java

saig0 · 2022-04-22T05:04:51Z

@pihme I applied some of your review hints and answer your questions ✔️

pihme

From my side, I would like to see a change regarding the naming of the methods I mentioned in earlier comments.

Regarding whether or not to add db migration code + tests, I am open to defer this to a follow up issue (which can then be prioritized independently). But in general, I feel we should make it a habit to write migration code and tests whenever we make changes to the database.

engine/src/test/java/io/camunda/zeebe/engine/state/instance/EventScopeInstanceStateTest.java

pihme

Looking forward to seeing this merged!

saig0 · 2022-04-27T11:08:21Z

bors merge

9175: Trigger boundary events after an interrupting event subprocess is triggered r=saig0 a=saig0 ## Description Fixing the issue that boundary events can't be triggered after an interrupting event subprocess is triggered on the scope. Solve the issue by: * avoid that event subscriptions of boundary event are removed when an interrupting event scope is triggered * introduce a filter when removing the event subscriptions * filter by event subprocesses and ignore other event subscriptions * extend the event scope instance state to distinguish interrupting event subscriptions from boundary events * add the property `boundaryElementIds` for event scopes to distinguish boundary events from other events in the scope * add the property `interrupted` for event scopes to mark if an interrupting event (subprocess) is triggered * if the event scope is interrupted then no other interrupting or non-interrupting events can be triggered * only boundary events can be triggered for an interrupted event scope * if an interrupting boundary event is triggered then the event scope doesn't accept any other event Maintenance: * remove methods from the event scope state that are not used in production code * migrate the event scope state test to JUnit 5 ## Related issues closes #6874 Co-authored-by: Philipp Ossler <philipp.ossler@gmail.com>

zeebe-bors-camunda · 2022-04-27T11:38:25Z

Build failed:

continuous-integration/jenkins/branch

saig0 · 2022-04-27T11:48:55Z

bors merge

zeebe-bors-camunda · 2022-04-27T12:16:30Z

Build succeeded:

github-actions · 2022-04-27T12:17:34Z

Backport failed for stable/1.3, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin stable/1.3
git worktree add -d .worktree/backport-9175-to-stable/1.3 origin/stable/1.3
cd .worktree/backport-9175-to-stable/1.3
git checkout -b backport-9175-to-stable/1.3
ancref=$(git merge-base 7905399440295deee368274a23877744a283596c 7791618b9c2c4f44870e64d0255e7fe1472af546)
git cherry-pick -x $ancref..7791618b9c2c4f44870e64d0255e7fe1472af546

github-actions · 2022-04-27T12:17:37Z

Backport failed for stable/8.0, because it was unable to create a new branch.

Please cherry-pick the changes locally.

git fetch origin stable/8.0
git worktree add -d .worktree/backport-9175-to-stable/8.0 origin/stable/8.0
cd .worktree/backport-9175-to-stable/8.0
git checkout -b backport-9175-to-stable/8.0
ancref=$(git merge-base 7905399440295deee368274a23877744a283596c 7791618b9c2c4f44870e64d0255e7fe1472af546)
git cherry-pick -x $ancref..7791618b9c2c4f44870e64d0255e7fe1472af546

9241: [Backport 8.0] Trigger boundary events after an interrupting event subprocess is triggered r=pihme a=saig0 ## Description Backport of #9175 ## Related issues relates to #6874 Co-authored-by: Philipp Ossler <philipp.ossler@gmail.com>

9242: [Backport 1.3] Trigger boundary events after an interrupting event subprocess is triggered r=pihme a=saig0 ## Description Backport of #9175 ## Related issues relates to #6874 Co-authored-by: Philipp Ossler <philipp.ossler@gmail.com>

9241: [Backport 8.0] Trigger boundary events after an interrupting event subprocess is triggered r=saig0 a=saig0 ## Description Backport of #9175 ## Related issues relates to #6874 Co-authored-by: Philipp Ossler <philipp.ossler@gmail.com>

9242: [Backport 1.3] Trigger boundary events after an interrupting event subprocess is triggered r=saig0 a=saig0 ## Description Backport of #9175 ## Related issues relates to #6874 Co-authored-by: Philipp Ossler <philipp.ossler@gmail.com>

9241: [Backport 8.0] Trigger boundary events after an interrupting event subprocess is triggered r=saig0 a=saig0 ## Description Backport of #9175 ## Related issues relates to #6874 Co-authored-by: Philipp Ossler <philipp.ossler@gmail.com>

saig0 added backport stable/1.3 labels Apr 20, 2022

saig0 added 3 commits April 20, 2022 06:41

refactor(engine): remove unused state methods

17a66f6

* remove methods from event scope instance state that are only used in tests but not in production code * these methods were used previously but are not needed anymore

refactor(test): migrate to JUnit 5

2851144

* migrate the test case from JUnit 4 to JUnit 5 using the Zeebe state extension

saig0 force-pushed the 6874-boundary-event branch from fd346e8 to b9423e3 Compare April 20, 2022 09:27

saig0 force-pushed the 6874-boundary-event branch from b9423e3 to 07cca46 Compare April 20, 2022 11:19

saig0 marked this pull request as ready for review April 20, 2022 11:49

saig0 requested a review from pihme April 20, 2022 11:50

pihme reviewed Apr 21, 2022

View reviewed changes

saig0 added 5 commits April 22, 2022 07:02

refactor(engine): remove unused return value

f87e8f1

refactor(engine): move unsubscribe event method

5a449a7

refactor(engine): use only immutable states

c57ced5

refactor(engine): add comment for clarification

84c267f

refactor(engine): add comment for clarification

be2d3d6

saig0 force-pushed the 6874-boundary-event branch from 2357a9d to be2d3d6 Compare April 22, 2022 05:02

saig0 requested a review from pihme April 22, 2022 05:03

pihme suggested changes Apr 25, 2022

View reviewed changes

engine/src/test/java/io/camunda/zeebe/engine/state/instance/EventScopeInstanceStateTest.java Show resolved Hide resolved

refactor(engine): rename method for clarity

7791618

saig0 requested a review from pihme April 27, 2022 09:03

pihme approved these changes Apr 27, 2022

View reviewed changes

zeebe-bors-camunda bot merged commit 2f4d8c8 into main Apr 27, 2022

zeebe-bors-camunda bot deleted the 6874-boundary-event branch April 27, 2022 12:16

This was referenced Apr 28, 2022

[Backport 8.0] Trigger boundary events after an interrupting event subprocess is triggered #9241

Merged

[Backport 1.3] Trigger boundary events after an interrupting event subprocess is triggered #9242

Merged

npepinpe added release/8.0.2 version:1.3.8 labels May 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trigger boundary events after an interrupting event subprocess is triggered #9175

Trigger boundary events after an interrupting event subprocess is triggered #9175

saig0 commented Apr 20, 2022 •

edited

pihme commented Apr 20, 2022

saig0 commented Apr 21, 2022

menski commented Apr 21, 2022

saig0 commented Apr 21, 2022 •

edited by menski

pihme left a comment

saig0 commented Apr 22, 2022

pihme left a comment

pihme left a comment

saig0 commented Apr 27, 2022

zeebe-bors-camunda bot commented Apr 27, 2022

saig0 commented Apr 27, 2022

zeebe-bors-camunda bot commented Apr 27, 2022

github-actions bot commented Apr 27, 2022

github-actions bot commented Apr 27, 2022

Trigger boundary events after an interrupting event subprocess is triggered #9175

Trigger boundary events after an interrupting event subprocess is triggered #9175

Conversation

saig0 commented Apr 20, 2022 • edited

Description

Related issues

Definition of Done

pihme commented Apr 20, 2022

saig0 commented Apr 21, 2022

menski commented Apr 21, 2022

saig0 commented Apr 21, 2022 • edited by menski

pihme left a comment

Choose a reason for hiding this comment

saig0 commented Apr 22, 2022

pihme left a comment

Choose a reason for hiding this comment

pihme left a comment

Choose a reason for hiding this comment

saig0 commented Apr 27, 2022

zeebe-bors-camunda bot commented Apr 27, 2022

saig0 commented Apr 27, 2022

zeebe-bors-camunda bot commented Apr 27, 2022

github-actions bot commented Apr 27, 2022

github-actions bot commented Apr 27, 2022

saig0 commented Apr 20, 2022 •

edited

saig0 commented Apr 21, 2022 •

edited by menski