feat(engine): allow exclusive jobs across sub-processes #4146

psavidis · 2024-03-04T11:23:44Z

Related-to: #4004

imports, static imports, spaces, newLines

engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ProcessEngineConfigurationImpl.java

- create / drop - upgrade

- create / drop - upgrade - use small letter for DDL

`enableExclusivenessAcrossProcessInstances` controls the feature

- Parameterize the query `selectNextJobsToExecute` base on the config `enableExclusivenessAcrossProcessInstances` - When the feature is disabled, the select columns and the selection of exclusive jobs will not consider the new column at all - The legacy query select queries will be preserved when the feature is turned off.

Rename config to `jobExecutorAcquireExclusiveOverProcessHierarchies` - Respects the `jobExecutorAcquire` convention - Emphasizes the feature is about process hierarchies and the application of exclusive jobs there

What: - Rename all test cases to better reflect their purpose - Add test for feature disabled - Add java doc and comments - Reformat all test class

- For testing the default legacy acquisition behaviour (parallel execution) of 7.20 processes with multiple hierarchies - For testing the new behaviour of sequential behaviour of 7.20 processes with multiple hierarchies - 7.20 processes will have the new column `rootProcessId` with `null` value in the batch table

# Conflicts: # engine/src/main/resources/org/camunda/bpm/engine/db/upgrade/mssql_engine_7.20_to_7.21.sql

- MariaDB - MySQL - SQLServer

psavidis · 2024-03-14T15:55:10Z

Update

Context: Manual testing the feature
What: camunda-run & the new feature-flag

Deployed a root process having a multi-instance call activity with cardinality=1000 and a subprocess with a groovy script task which prints a "hello world from thread:" + Thread.currentThread().name
Given the feature is disabled & a new process instance is started
- The "hello world" message gets printed numerous times on the console & there are various logs with different task executor threads printing OptimisticLockingException instances; This is the reproduction of the legacy behaviour
Given the feature is enabled & a new process instance is started
- The are no more OptimisticLockingException instances in the logs and the print statements follow sequential thread execution according to the DefaultJobExecutor's queueSize=3 & corePoolSize=3

Screenshot of logs with feature disabled

Screenshot of logs with feature enabled

psavidis · 2024-03-15T07:58:54Z

Update

All Pipelines passed:

There is no need to wait for the last build since the commit concerns fixing a typo & homogenising the comments in the unit tests

engine/src/test/java/org/camunda/bpm/engine/test/jobexecutor/SequentialJobAcquisitionTest.java

- Extract tests into ExclusiveJobAcquisitionTest & refactor setup / tearDown logic - Extract job executor wait logic into separate JobExecutorWaitUtils class

psavidis · 2024-03-20T15:12:02Z

Update

Context: The Unit tests were expected to fail with db2 and mssql
Why: The adjustment on the SQL for h2 & db2 was not commited when the CI was executed.
Problem: The tests were h2-specific and they were using a custom ProcessEngineConfiguration setup. Thus, they wouldn't run against all databases.
Solution: This commit makes the tests db-agnostic instead of h2-specific; that means the ExclusiveJobAcquisitionTest will be executed in h2 or any other db profile including db2 & mssql, allowing to reveal any vendor-specific SQL issues

psavidis · 2024-03-20T16:21:23Z

Update

The Code Review can proceed in the meantime; the last two points (pipeline build with all-dbs & revert label) can be done in parallel (the approval can wait till the build passes)

Assigning to @yanavasileva for a second review round.

Oracle returns group batches in lists which contain elements in different order. This commit changes the groups from list into sets to avoid the ordering issue for databases which might return elements in different order.

Detected flakiness during oracle run due to clock discrepancy (1h). Adding the clock reset in the class to make it stable

psavidis · 2024-03-21T08:10:35Z

Update

The last build using Oracle failed in DecisionDefinitionTest with a clock discrepancy that looks unrelated to the adjustments of this PR (ExclusiveJobAcquisitionTest does not depend on Clock). For the sake of making the build pass, added a clock reset on DecisionDefinitionTest to fix the flakiness

Screenshot

Context: The HTTL Period expressions in days are affected by the date the tests run.
Problem: The expected date are calculated and have discrepancies with DST
Solution: Make the Test consider a fixed date that belongs to the past and make sure the data is bounded and unaffected by the DST.
Link: Commit that fixes the issue

yanavasileva

👍 Nice. I only have suggestions for improvements in the tests.

engine/src/test/java/org/camunda/bpm/engine/test/jobexecutor/ExclusiveJobAcquisitionTest.java

psavidis · 2024-03-21T14:58:11Z

engine/src/test/java/org/camunda/bpm/engine/test/dmn/deployment/DecisionDefinitionTest.java

    this.repositoryService = engineRule.getRepositoryService();
    this.decisionService = engineRule.getDecisionService();
    this.historyService = engineRule.getHistoryService();
+
+    SimpleDateFormat sdf = new SimpleDateFormat("dd/MM/yyyy hh:mm:ss.SSS");


Context: Fixes Flaky DST issues with this test
Why: The change is not related to this PR but will be committed together since it was identified

Don't forget to put a note for the fix in the commit message upon merge.
I think the fix for the test is good to go.

Co-authored-by: yanavasileva <yanavasileva@users.noreply.github.com>

- Rename asArrayOfSets method - Remove ProcessEngineState - Reorder & group instance fields to be symmetric

psavidis · 2024-03-21T15:59:23Z

Update

All Code review points are addressed. Will assign after a successful build for the final approval.

Next steps after approval: Revert the commit in the stage-types.yml file.

yanavasileva

👍 Thank you for considering my feedback. Please wait for the CI before merging.

psavidis · 2024-03-22T08:14:54Z

Update

All pipelines passed*

The failure of CockroachDB is not related to this PR

Screenshot

Proceeding to revert the change on stage files and merge.

This reverts commit 5af9bec.

danielkelemen added 3 commits March 4, 2024 13:20

feat(engine): allow exclusive jobs across sub-processes

824c15f

test

65e3d68

some fixes

4e3da2e

psavidis self-assigned this Mar 4, 2024

This was referenced Mar 4, 2024

Extend job executor acquisition to work with rootProcessInstanceId #4004

Closed

feat(engine): allow exclusive jobs across sub-processes #3960

Closed

psavidis changed the title ~~Define exclusive jobs across subprocesses new~~ feat(engine): allow exclusive jobs across sub-processes Mar 4, 2024

psavidis added the ci:all-db Runs the builds for all databases. label Mar 4, 2024

chore(engine): Adjust SequentialJobAcquisitionTest

f8455c1

imports, static imports, spaces, newLines

tasso94 reviewed Mar 5, 2024

View reviewed changes

engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ProcessEngineConfigurationImpl.java Outdated Show resolved Hide resolved

chore(engine): Add index to db scripts

a6ada83

- create / drop - upgrade

psavidis force-pushed the define-exclusive-jobs-across-subprocesses-new branch from c9d161f to 590308d Compare March 6, 2024 08:17

psavidis added 6 commits March 7, 2024 13:00

chore(engine): Add index to db scripts

59127ed

- create / drop - upgrade - use small letter for DDL

chore(engine): Add Config Flag

c0399e7

`enableExclusivenessAcrossProcessInstances` controls the feature

test(engine): improve test readability

be3ab3b

chore(engine): fix AcquireJobsCmd

4fcfba7

refactor(engine): Rename Config

675a832

Rename config to `jobExecutorAcquireExclusiveOverProcessHierarchies` - Respects the `jobExecutorAcquire` convention - Emphasizes the feature is about process hierarchies and the application of exclusive jobs there

psavidis force-pushed the define-exclusive-jobs-across-subprocesses-new branch from 590308d to 675a832 Compare March 7, 2024 11:01

psavidis added 4 commits March 11, 2024 17:45

chore(engine): Add test with the feature disabled

ddb309a

What: - Rename all test cases to better reflect their purpose - Add test for feature disabled - Add java doc and comments - Reformat all test class

Merge branch 'master' into define-exclusive-jobs-across-subprocesses-new

6cd7b6b

# Conflicts: # engine/src/main/resources/org/camunda/bpm/engine/db/upgrade/mssql_engine_7.20_to_7.21.sql

chore(engine): Fix Drop Engine Statements

8eb176f

- MariaDB - MySQL - SQLServer

psavidis force-pushed the define-exclusive-jobs-across-subprocesses-new branch from 937f133 to 8eb176f Compare March 14, 2024 07:51

psavidis added ci:mariadb Runs the builds for the MariaDB database. and removed ci:mariadb Runs the builds for the MariaDB database. labels Mar 14, 2024

psavidis commented Mar 15, 2024

View reviewed changes

engine/src/test/java/org/camunda/bpm/engine/test/jobexecutor/SequentialJobAcquisitionTest.java Show resolved Hide resolved

psavidis marked this pull request as ready for review March 15, 2024 08:46

psavidis added ci:db2 Runs the builds for the DB2 database. ci:sqlserver Runs the builds for the MS SQL Server database. and removed ci:all-db Runs the builds for all databases. labels Mar 20, 2024

test(engine): Adjust tests to be db agnostic

6e2c1ae

- Extract tests into ExclusiveJobAcquisitionTest & refactor setup / tearDown logic - Extract job executor wait logic into separate JobExecutorWaitUtils class

psavidis added ci:all-db Runs the builds for all databases. and removed ci:db2 Runs the builds for the DB2 database. ci:sqlserver Runs the builds for the MS SQL Server database. labels Mar 20, 2024

test(engine): Add javadoc and fix spaces

e9de1d5

psavidis requested a review from yanavasileva March 20, 2024 16:25

psavidis added ci:oracle Runs the builds for the Oracle database. and removed ci:all-db Runs the builds for all databases. labels Mar 20, 2024

psavidis added 2 commits March 20, 2024 23:32

test(engine): Convert JobGroups to Sets

c4ad024

Oracle returns group batches in lists which contain elements in different order. This commit changes the groups from list into sets to avoid the ordering issue for databases which might return elements in different order.

test(engine): Reset Clock for DecisionDefinitionTest

14b2718

Detected flakiness during oracle run due to clock discrepancy (1h). Adding the clock reset in the class to make it stable

fix flaky DST test

19605e8

psavidis force-pushed the define-exclusive-jobs-across-subprocesses-new branch from 07cf324 to 19605e8 Compare March 21, 2024 12:14

yanavasileva added the ci:cockroachdb Runs the builds for the CockroachDB database. label Mar 21, 2024

yanavasileva reviewed Mar 21, 2024

View reviewed changes

psavidis commented Mar 21, 2024

View reviewed changes

psavidis and others added 2 commits March 21, 2024 17:28

Apply suggestions from code review

ea5331a

Co-authored-by: yanavasileva <yanavasileva@users.noreply.github.com>

test(engine): Conform to Code Review Points

b89eb44

- Rename asArrayOfSets method - Remove ProcessEngineState - Reorder & group instance fields to be symmetric

yanavasileva removed the ci:cockroachdb Runs the builds for the CockroachDB database. label Mar 21, 2024

yanavasileva approved these changes Mar 21, 2024

View reviewed changes

Revert "Enable All-dbs for instance-migration"

d47111a

This reverts commit 5af9bec.

psavidis merged commit 94024a0 into master Mar 22, 2024
2 of 3 checks passed

psavidis deleted the define-exclusive-jobs-across-subprocesses-new branch March 22, 2024 10:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(engine): allow exclusive jobs across sub-processes #4146

feat(engine): allow exclusive jobs across sub-processes #4146

psavidis commented Mar 4, 2024 •

edited

psavidis commented Mar 14, 2024

psavidis commented Mar 15, 2024 •

edited

psavidis commented Mar 20, 2024 •

edited

psavidis commented Mar 20, 2024 •

edited

psavidis commented Mar 21, 2024 •

edited

yanavasileva left a comment

psavidis Mar 21, 2024

yanavasileva Mar 21, 2024

psavidis commented Mar 21, 2024 •

edited

yanavasileva left a comment

psavidis commented Mar 22, 2024 •

edited

feat(engine): allow exclusive jobs across sub-processes #4146

feat(engine): allow exclusive jobs across sub-processes #4146

Conversation

psavidis commented Mar 4, 2024 • edited

psavidis commented Mar 14, 2024

Update

psavidis commented Mar 15, 2024 • edited

Update

psavidis commented Mar 20, 2024 • edited

Update

psavidis commented Mar 20, 2024 • edited

Update

psavidis commented Mar 21, 2024 • edited

Update

yanavasileva left a comment

Choose a reason for hiding this comment

psavidis Mar 21, 2024

Choose a reason for hiding this comment

yanavasileva Mar 21, 2024

Choose a reason for hiding this comment

psavidis commented Mar 21, 2024 • edited

Update

yanavasileva left a comment

Choose a reason for hiding this comment

psavidis commented Mar 22, 2024 • edited

Update

psavidis commented Mar 4, 2024 •

edited

psavidis commented Mar 15, 2024 •

edited

psavidis commented Mar 20, 2024 •

edited

psavidis commented Mar 20, 2024 •

edited

psavidis commented Mar 21, 2024 •

edited

psavidis commented Mar 21, 2024 •

edited

psavidis commented Mar 22, 2024 •

edited