[JBPM-10088] Removing timers when rollback #2365

fjtirado · 2023-11-16T10:58:22Z

@martinweiler Can you try your reproducer for JBPM-8688 with this one? Thanks in advance.

fjtirado · 2023-11-16T11:04:01Z

...b/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EjbSchedulerService.java

@@ -100,10 +100,6 @@ public JobHandle scheduleJob(Job job, JobContext ctx, Trigger trigger) {
 public boolean removeJob(JobHandle jobHandle) {
 String uuid = ((EjbGlobalJobHandle) jobHandle).getUuid();
 final Timer ejbTimer = getEjbTimer(getTimerMappinInfo(uuid));
- if (TRANSACTIONAL && ejbTimer == null) {


Timer might be expired (getEJBTimer will return null) and we still want try to remove it just in case.

Good catch!

fjtirado · 2023-11-16T16:32:20Z

...ejb/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EJBTimerScheduler.java

@@ -186,7 +186,12 @@ private interface Transaction<I> {

 @TransactionAttribute(value = TransactionAttributeType.REQUIRES_NEW)
 public <I> void transaction(Transaction<I> operation, I item) throws Exception {
- operation.doWork(item);
+ try {


The transaction should be rollback if there is any exception

This reverts commit 61b735b.

rollback

fjtirado · 2023-11-22T15:35:46Z

...b/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EjbSchedulerService.java

@@ -146,30 +139,17 @@ private TimerMappingInfo getTimerMappinInfo(long processInstanceId, long timerId
 }

 private TimerMappingInfo getTimerMappingInfo(Function<EntityManager, List<TimerMappingInfo>> func) {
- InternalRuntimeManager manager = ((GlobalTimerService) globalTimerService).getRuntimeManager();


When using CMT, I do not think we need to create a different transaction here anymore

fjtirado · 2023-11-22T15:36:11Z

...b/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EjbSchedulerService.java

@@ -222,7 +201,7 @@ public JobHandle buildJobHandleForContext(NamedJobContext ctx) {

 @Override
 public boolean isTransactional() {
- return TRANSACTIONAL;
+ return true;


There is not point in keeping the variable. CMT is always transactional.

fjtirado · 2023-11-22T15:36:38Z

...b/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EjbSchedulerService.java

+ public
+ void setEnvironment(RuntimeEnvironment environment) {
+ if (environment instanceof SimpleRuntimeEnvironment) {
+ ((SimpleRuntimeEnvironment)environment).addToEnvironment("IS_TIMER_CMT", true);


We need to tell GlobalJpaTimerJobInsance that we are using CMT

fjtirado · 2023-11-22T15:40:14Z

...ejb/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EJBTimerScheduler.java

@@ -111,7 +113,7 @@ public void executeTimerJob(Timer timer) {
 Thread.currentThread().interrupt();
 } 
 try {
- invokeTransaction(this::executeTimerJobInstance, timerJobInstance);
+ executeTimerJobInstance(timerJobInstance);


We do not need to create a new transaction here. Also PersistableRunner is on the middle, handling the transaction itself. Because that transaction is not longer usable once PersistableRunner commits it, we need to create a new one to execute recoverTimerJob.

before this was provoking an exception at setRollbackOnly call in the new end-2-end test, because there was no transaction

...jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/timer/GlobalJpaTimerJobInstance.java

sonarcloud · 2023-11-24T13:00:03Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

71.4% Coverage
0.0% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

gmunozfe · 2023-11-27T10:14:34Z

jenkins do fdb

gmunozfe

Looks good to me, works fine with jbpm-playground samples, awesome work @fjtirado !
Just add the labels for backporting before merge

* [JBPM-10088] Removing timers when rollback * [JBPM-10088] Dealing with transactions in different way * [JBPM-10088] Set to no transactional * Revert "[JBPM-10088] Set to no transactional" This reverts commit 61b735b. * [JBPM-10088] Setting CMT property in environment * [JBPM-10088] REmoving new transaction created while querying during rollback * [JBPM-10088] Do not create transaction for regular timer task * [JBPM-10088] Sonar comments * [JBPM-10088] Some test failed * [JBPM-10088] Removing unneeded casting

* [JBPM-10088] Removing timers when rollback * [JBPM-10088] Dealing with transactions in different way * [JBPM-10088] Set to no transactional * Revert "[JBPM-10088] Set to no transactional" This reverts commit 61b735b. * [JBPM-10088] Setting CMT property in environment * [JBPM-10088] REmoving new transaction created while querying during rollback * [JBPM-10088] Do not create transaction for regular timer task * [JBPM-10088] Sonar comments * [JBPM-10088] Some test failed * [JBPM-10088] Removing unneeded casting Co-authored-by: Francisco Javier Tirado Sarti <65240126+fjtirado@users.noreply.github.com>

sonarcloud · 2024-03-04T18:43:04Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[JBPM-10088] Removing timers when rollback

27c29b1

fjtirado force-pushed the JBPM-10088 branch from 37be302 to 27c29b1 Compare November 16, 2023 11:01

fjtirado commented Nov 16, 2023

View reviewed changes

fjtirado force-pushed the JBPM-10088 branch 4 times, most recently from 4b24ea1 to b7e701c Compare November 16, 2023 12:33

fjtirado commented Nov 16, 2023

View reviewed changes

fjtirado marked this pull request as ready for review November 16, 2023 19:08

fjtirado requested a review from gmunozfe November 16, 2023 19:09

fjtirado mentioned this pull request Nov 20, 2023

[JBPM-10209] Switching to CMT #2362

Merged

[JBPM-10088] Dealing with transactions in different way

60f0b24

fjtirado force-pushed the JBPM-10088 branch from b7e701c to 60f0b24 Compare November 20, 2023 12:54

fjtirado added 3 commits November 22, 2023 11:01

[JBPM-10088] Set to no transactional

61b735b

Revert "[JBPM-10088] Set to no transactional"

6c364e7

This reverts commit 61b735b.

[JBPM-10088] Setting CMT property in environment

7dda523

fjtirado force-pushed the JBPM-10088 branch from 1127709 to 7dda523 Compare November 22, 2023 11:15

fjtirado added 2 commits November 22, 2023 13:05

[JBPM-10088] REmoving new transaction created while querying during

ab096b6

rollback

[JBPM-10088] Do not create transaction for regular timer task

475164c

fjtirado force-pushed the JBPM-10088 branch from d4afafc to 475164c Compare November 22, 2023 15:19

fjtirado commented Nov 22, 2023

View reviewed changes

fjtirado force-pushed the JBPM-10088 branch 4 times, most recently from 03bf3b4 to 00e98d7 Compare November 22, 2023 15:50

fjtirado commented Nov 22, 2023

View reviewed changes

...jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/timer/GlobalJpaTimerJobInstance.java Outdated Show resolved Hide resolved

[JBPM-10088] Sonar comments

5e2aed8

fjtirado force-pushed the JBPM-10088 branch 4 times, most recently from 459a47a to 3bba9a7 Compare November 23, 2023 10:40

[JBPM-10088] Some test failed

5f98688

fjtirado force-pushed the JBPM-10088 branch from 3bba9a7 to 5f98688 Compare November 23, 2023 15:59

[JBPM-10088] Removing unneeded casting

583e8fa

gmunozfe approved these changes Nov 28, 2023

View reviewed changes

fjtirado added backport-7.67.x Generate backport PR for 7.67.x branch backport-7.67.x-blue Generate backport PR for 7.67.x-blue branch labels Nov 28, 2023

fjtirado merged commit 80efbc1 into kiegroup:main Dec 1, 2023
9 of 10 checks passed

This was referenced Dec 1, 2023

[7.67.x] [JBPM-10088] Removing timers when rollback #2370

Merged

[7.67.x-blue] [JBPM-10088] Removing timers when rollback #2371

Merged

fjtirado changed the title ~~[JBPM-10088] Removing timers when rollback~~ [JBPM-8688] Removing timers when rollback Mar 4, 2024

fjtirado changed the title ~~[JBPM-8688] Removing timers when rollback~~ [JBPM-10088] Removing timers when rollback Mar 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JBPM-10088] Removing timers when rollback #2365

[JBPM-10088] Removing timers when rollback #2365

fjtirado commented Nov 16, 2023 •

edited

Loading

fjtirado Nov 16, 2023

gmunozfe Nov 16, 2023

fjtirado Nov 16, 2023

fjtirado Nov 22, 2023 •

edited

Loading

fjtirado Nov 22, 2023

fjtirado Nov 22, 2023

fjtirado Nov 22, 2023 •

edited

Loading

gmunozfe Nov 23, 2023 •

edited

Loading

sonarcloud bot commented Nov 24, 2023

gmunozfe commented Nov 27, 2023

gmunozfe left a comment

sonarcloud bot commented Mar 4, 2024

[JBPM-10088] Removing timers when rollback #2365

[JBPM-10088] Removing timers when rollback #2365

Conversation

fjtirado commented Nov 16, 2023 • edited Loading

fjtirado Nov 16, 2023

Choose a reason for hiding this comment

gmunozfe Nov 16, 2023

Choose a reason for hiding this comment

fjtirado Nov 16, 2023

Choose a reason for hiding this comment

fjtirado Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

fjtirado Nov 22, 2023

Choose a reason for hiding this comment

fjtirado Nov 22, 2023

Choose a reason for hiding this comment

fjtirado Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

gmunozfe Nov 23, 2023 • edited Loading

Choose a reason for hiding this comment

sonarcloud bot commented Nov 24, 2023

gmunozfe commented Nov 27, 2023

gmunozfe left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Mar 4, 2024

Quality Gate passed

fjtirado commented Nov 16, 2023 •

edited

Loading

fjtirado Nov 22, 2023 •

edited

Loading

fjtirado Nov 22, 2023 •

edited

Loading

gmunozfe Nov 23, 2023 •

edited

Loading