Reject duplicate parallel gateway activate command

[remcowesterhoud]

Description

Parallel gateways get activated by taking a sequence flow and checking if the number of taken flows is greater or equal to the number of incoming sequence flows. If this is the case an activate command is sent. The number of taken sequence flows get rest upon activation of the parallel gateway.

This proves troublesome when a "bad" model causes one of the incoming sequence flows to be taken twice. This could result in the activation command being sent twice. Imagine there is a parallel gateway with 2 incoming flows. What would happen is:

1. First flow is taken
2. Second flow is taken. Incoming flows == taken flows so an activate command is sent.
3. Second flow is taken again. The first activate command has not been processed yet. The number of taken flows has not been reset. As a result incoming flows < taken flows. A second activate command is sent.

This is solved by always sending an activate command when a sequence flow is taken. Once the BpmnStreamProcessor tries to process the record it will check if the state is valid. Here a check has been added to verify that when we receive an activate command for a parallel gateway we will first check if all the incoming flows have been taken. If this is not the case we will reject the command.

Related issues

closes #6778

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.

Please refer to our review guidelines.

[github-actions]

Unit Test Results

   792 files  ±    0     792 suites  ±0   1h 42m 3s ⏱️ + 1m 24s
6 219 tests +219  6 211 ✔️ +219  8 💤 ±0  0 ❌ ±0 
6 388 runs  +219  6 380 ✔️ +219  8 💤 ±0  0 ❌ ±0 

Results for commit aa9fab1. ± Comparison against base commit 275a7d2.

♻️ This comment has been updated with latest results.

[remcowesterhoud]

@korthout I have removed you as a reviewer for now, since the solution is flawed. Sorry!

[korthout]

@remcowesterhoud I like the idea of rejecting the activate command for the parallel gateway when it isn't yet ready to activate. It would allow us to remove the logic that determines whether or not to write the activate command, and entirely move it to the processor.

This also makes it more in line with other concurrent behaviors (e.g. an element might be planned to be completed because a COMPLETE_ELEMENT was written for it on the log, but before it is processed a TERMINATE_ELEMENT is processed due to some interruption like a boundary event, we then reject the COMPLETE_ELEMENT command when we try to process it later).

To make this idea work for Start Process Instance Anywhere, we need something special. We'll need to increment the number of taken sequence flows before processing the ACTIVATE_ELEMENT of the parallel gateway:

• for each parallel gateway that is targetted by a start instruction,
• for each incoming sequence flow,
• within the related flow scope instance.

We could do that by introducing a new event applier for ProcessInstanceCreation CREATED events, which is written (and applied) before the ACTIVATE_ELEMENT of the parallel gateway is processed.

The hard part is to find the related flow scope instance, because you don't have access to the key of the parallel gateway's element instance (i.e. the parallel gateway isn't created yet). There is no direct access to this, but you could traverse the process instance's process model using ElementInstanceState.getChildren(key).

@remcowesterhoud If you want we can talk about it in person.

[remcowesterhoud]

@korthout Thanks for your help ❤️

I have resolved the issues, please have a look!

[korthout]

🚀 Nice work @remcowesterhoud

🔧 Please make it clear in the PR description what this PR changes.
🔧 Please document the new Parallel Gateway behavior.

💭 This will probably not backport so easily to the older versions, because of the new event applier (those versions don't know about start instructions).

👍 LGTM

[remcowesterhoud]

bors merge

[zeebe-bors-camunda]

Build failed:

• Java checks

[remcowesterhoud]

bors retry

[zeebe-bors-camunda]

Build succeeded:

• Go linting
• Java checks
• Test summary

[backport-action]

Successfully created backport PR #9822 for stable/1.3.

[backport-action]

Successfully created backport PR #9823 for stable/8.0.

[korthout]

@remcowesterhoud Please add this to the release notes