Parallel and inclusive Gateway not executing according to BPMN spec about camunda-bpm-platform HOT 8 OPEN

This comment was imported from JIRA and written by user wyV3oL0

Should I also post a new topic in the camunda-bpm-dev group?

from camunda-bpm-platform.

This comment was imported from JIRA and written by user @ThorbenLindhauer

Hi Filip,

Good finding and analysis. This is a known issue and is stated in the Javadocs of the ParallelGatewayActivityBehavior, see <1>. Regarding two tokens on each sequence flow, the intended behavior would be to activate the gateway twice, thus resulting in two instances of "Final Task" as you suppose.

In order to fix this, we would have to store for an Execution which sequence flow it has arrived on and this would also have to go to the database, since executions can arrive at the gateway in different transactions. I think this is also why this issue has not been addressed yet.

Cheers,
Thorben

<1> https://github.com/camunda/camunda-bpm-platform/blob/master/engine/src/main/java/org/camunda/bpm/engine/impl/bpmn/behavior/ParallelGatewayActivityBehavior.java

from camunda-bpm-platform.

This comment was imported from JIRA and written by user wyV3oL0

Hi Thorben,

I did not look at that class, I am sorry. I tried searching but nothing came up.

For the fix I was thinking in the same direction. You would definitely need to go to the database and somehow store the number of tokens on each sequence flow.

I had a quick look and this is what I think, you can add additional field in the PvmExecutionImpl for the Sequence Flow that led to it, and also add this field to the ACTRUEXECUTION table in the database and a method for it in the ActivityExecution. Then in the ParallelGatewayActivityBehavior instead of comparing the sizes of the lists, for each of the incoming transitions you check if there the appropriate sequence flow and create a list of only those, only then if the size of the incoming transitions is the same with the new list you do execution.takeall , otherwise it stays the same.

Does this sound reasonable for you?

The last issue with this is the order of the tokens. Probably it is important the first completed "Task 1" to be taken with the first completed "Task 2". If it is, is there some get the order of the executions?

Cheers
Filip

from camunda-bpm-platform.

This comment was imported from JIRA and written by user @ThorbenLindhauer

Hi Filip,

Your outline sounds good. The order of tokens is not important since the parallel gateway only makes a local decision about the tokens arrived on incoming sequence flows.

I discussed this issue with the team and we are not sure yet whether we should fix this behavior at the moment due to the additional database attribute that is required. On the hand, the current implementation is wrong, on the other hand, the additional attribute is only needed for this gateway (and perhaps the inclusive gateway) and new attributes always mean certain a performance impact. Furthermore, we have had very little complaints about the current behavior so far.

If you like, feel free to provide a pull request. However, I cannot guarantee that we will merge it immediately.

Cheers,
Thorben

from camunda-bpm-platform.

This comment was imported from JIRA and written by user wyV3oL0

Hi Thorben,

Thanks for your input. I understand that an addition ad a new attribute means impact to the performance. I completely agree that this is not a big issue since, it is only for some specific corner case.

In any case, I had some free time and I did something, if you want and have time you can have a look at what I did here <1>. I think that the entrySequenceFlowId should be changed any time the Transtion is changed, maybe it would be best to change call the setEntrySequenceFlowId in the setTransitioMethod of the PvmExecutionImpl.

I can also do a pull request and you can review it and comment it directly on git.

I also tried to do some performance benchmark as stated here <2>. But I couldn't since the benchmark profile is missing. Am I doing something wrong?

Cheers
Filip

<1> https://github.com/filiphr/camunda-bpm-platform/tree/bug-fix-parallel-gateway-activity-behavior
<2> https://github.com/camunda/camunda-bpm-platform/tree/master/qa/performance-tests-engine

from camunda-bpm-platform.

This comment was imported from JIRA and written by user @ThorbenLindhauer

Hi Filip,

Your code looks good. Some considerations:

• I agree that coupling the update of the transition id to the #setTransition method makes sense. This avoids some redundancy.
• You could rename entrySequenceFlowId to lastTransitionId or simply transitionId to make it more consistent with the term "transition" and the #setTransition method
• You could add the new column to the update scripts: https://github.com/camunda/camunda-bpm-platform/tree/master/distro/sql-script/upgrade

For the performance benchmark, I have no problem building the project with

mvn clean install -Ph2,benchmark

What is the error you get?

A pull request would be nice, makes it easier to comment on individual changes :)

Best regards,
Thorben

from camunda-bpm-platform.

This comment was imported from JIRA and written by user wyV3oL0

Hi Thorben,

Thanks for your input. I did the changes that you proposed, I agree that it should be changed to transitionId. I did a pull request <1> where we can discuss into more details on the individual changes.

As for the benchmark, I am not sure what I was doing, but it seems that I was doing something wrong. I managed to run it, I think that there is not a big difference. On some tests there is an increase and on some a decrease. Maybe it would be best if you can run it and see what it does. You would definitely understand it more than me :).

Cheers
Filip

<1> #137

from camunda-bpm-platform.

This comment was imported from JIRA and written by user @ThorbenLindhauer

Hi Filip,

Thanks for the pull request. I'll have a look at the performance tests when I've got some time. Can't say when at the moment though.

Cheers,
Thorben

from camunda-bpm-platform.