From 77659c0e6ffb1de730f22ac0c0eec8272faa26f1 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 10 Jan 2024 11:53:58 +0100 Subject: [PATCH] [JBPM-10088] Removing timers when rollback (#2365) (#2371) * [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 61b735b4370031e6729fc7b419aa2bf9ea110d3b. * [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> --- .../core/timer/GlobalSchedulerService.java | 3 + .../impl/DefaultRuntimeEnvironment.java | 3 + .../services/ejb/timer/EJBTimerScheduler.java | 12 +++- .../ejb/timer/EjbSchedulerService.java | 68 +++++++------------ 4 files changed, 39 insertions(+), 47 deletions(-) diff --git a/jbpm-flow/src/main/java/org/jbpm/process/core/timer/GlobalSchedulerService.java b/jbpm-flow/src/main/java/org/jbpm/process/core/timer/GlobalSchedulerService.java index 15f50d1d1b..2bf4f2b2c6 100644 --- a/jbpm-flow/src/main/java/org/jbpm/process/core/timer/GlobalSchedulerService.java +++ b/jbpm-flow/src/main/java/org/jbpm/process/core/timer/GlobalSchedulerService.java @@ -21,6 +21,7 @@ import org.drools.core.time.TimerService; import org.drools.core.time.impl.TimerJobInstance; import org.jbpm.process.core.timer.impl.GlobalTimerService.GlobalJobHandle; +import org.kie.internal.runtime.manager.RuntimeEnvironment; /** * Implementations of these interface are responsible for scheduled jobs in global manner, @@ -87,4 +88,6 @@ default void invalidate(JobHandle jobHandle) { default TimerJobInstance getTimerJobInstance(long processInstanceId, long timerId) { return null; } + + default void setEnvironment(RuntimeEnvironment environment) {} } diff --git a/jbpm-runtime-manager/src/main/java/org/jbpm/runtime/manager/impl/DefaultRuntimeEnvironment.java b/jbpm-runtime-manager/src/main/java/org/jbpm/runtime/manager/impl/DefaultRuntimeEnvironment.java index 53b1aaf4ca..827cd6d62a 100644 --- a/jbpm-runtime-manager/src/main/java/org/jbpm/runtime/manager/impl/DefaultRuntimeEnvironment.java +++ b/jbpm-runtime-manager/src/main/java/org/jbpm/runtime/manager/impl/DefaultRuntimeEnvironment.java @@ -62,6 +62,9 @@ public DefaultRuntimeEnvironment(EntityManagerFactory emf, GlobalSchedulerServic this.usePersistence = true; this.userGroupCallback = UserDataServiceProvider.getUserGroupCallback(); this.userInfo = UserDataServiceProvider.getUserInfo(); + if (globalSchedulerService != null) { + globalSchedulerService.setEnvironment(this); + } } public DefaultRuntimeEnvironment(EntityManagerFactory emf, boolean usePersistence) { diff --git a/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EJBTimerScheduler.java b/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EJBTimerScheduler.java index 1a574fad68..48f09f2c6f 100644 --- a/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EJBTimerScheduler.java +++ b/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EJBTimerScheduler.java @@ -42,6 +42,7 @@ import javax.ejb.Timer; import javax.ejb.TimerConfig; import javax.ejb.TimerHandle; +import javax.ejb.TimerService; import javax.ejb.TransactionAttribute; import javax.ejb.TransactionAttributeType; @@ -73,7 +74,7 @@ public class EJBTimerScheduler { private ConcurrentMap localCache = new ConcurrentHashMap(); @Resource - protected javax.ejb.TimerService timerService; + protected TimerService timerService; @Resource protected SessionContext ctx; @@ -111,7 +112,7 @@ public void executeTimerJob(Timer timer) { Thread.currentThread().interrupt(); } try { - invokeTransaction(this::executeTimerJobInstance, timerJobInstance); + executeTimerJobInstance(timerJobInstance); } catch (Exception e) { recoverTimerJobInstance(timerJob, timer, e); } @@ -186,7 +187,12 @@ private interface Transaction { @TransactionAttribute(value = TransactionAttributeType.REQUIRES_NEW) public void transaction(Transaction operation, I item) throws Exception { - operation.doWork(item); + try { + operation.doWork(item); + } catch (Exception transactionEx) { + ctx.setRollbackOnly(); + throw transactionEx; + } } private void invokeTransaction (Transaction operation, I item) throws Exception { diff --git a/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EjbSchedulerService.java b/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EjbSchedulerService.java index 0b0a7be72b..421be2affd 100644 --- a/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EjbSchedulerService.java +++ b/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/main/java/org/jbpm/services/ejb/timer/EjbSchedulerService.java @@ -28,18 +28,13 @@ import javax.naming.InitialContext; import javax.naming.NamingException; import javax.persistence.EntityManager; -import javax.persistence.EntityManagerFactory; -import org.drools.core.time.InternalSchedulerService; import org.drools.core.time.Job; import org.drools.core.time.JobContext; import org.drools.core.time.JobHandle; import org.drools.core.time.TimerService; import org.drools.core.time.Trigger; import org.drools.core.time.impl.TimerJobInstance; -import org.drools.persistence.api.TransactionManager; -import org.drools.persistence.api.TransactionManagerFactory; -import org.drools.persistence.jta.JtaTransactionManager; import org.jbpm.process.core.timer.GlobalSchedulerService; import org.jbpm.process.core.timer.JobNameHelper; import org.jbpm.process.core.timer.NamedJobContext; @@ -47,9 +42,10 @@ import org.jbpm.process.core.timer.impl.DelegateSchedulerServiceInterceptor; import org.jbpm.process.core.timer.impl.GlobalTimerService; import org.jbpm.process.core.timer.impl.GlobalTimerService.GlobalJobHandle; +import org.jbpm.runtime.manager.impl.SimpleRuntimeEnvironment; import org.jbpm.runtime.manager.impl.jpa.EntityManagerFactoryManager; import org.jbpm.runtime.manager.impl.jpa.TimerMappingInfo; -import org.kie.internal.runtime.manager.InternalRuntimeManager; +import org.kie.internal.runtime.manager.RuntimeEnvironment; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -57,20 +53,17 @@ public class EjbSchedulerService implements GlobalSchedulerService { private static final Logger logger = LoggerFactory.getLogger(EjbSchedulerService.class); - private static final Boolean TRANSACTIONAL = Boolean.parseBoolean(System.getProperty("org.jbpm.ejb.timer.tx", "true")); - private AtomicLong idCounter = new AtomicLong(); - private TimerService globalTimerService; + private GlobalTimerService globalTimerService; private EJBTimerScheduler scheduler; private SchedulerServiceInterceptor interceptor = new DelegateSchedulerServiceInterceptor(this); - @Override public JobHandle scheduleJob(Job job, JobContext ctx, Trigger trigger) { long id = idCounter.getAndIncrement(); String jobName = getJobName(ctx, id); - EjbGlobalJobHandle jobHandle = new EjbGlobalJobHandle(id, jobName, ((GlobalTimerService) globalTimerService).getTimerServiceId()); + EjbGlobalJobHandle jobHandle = new EjbGlobalJobHandle(id, jobName, globalTimerService.getTimerServiceId()); TimerJobInstance jobInstance = null; // check if given timer job is marked as new timer meaning it was never scheduled before, @@ -89,7 +82,7 @@ public JobHandle scheduleJob(Job job, JobContext ctx, Trigger trigger) { ctx, trigger, jobHandle, - (InternalSchedulerService) globalTimerService); + globalTimerService); jobHandle.setTimerJobInstance((TimerJobInstance) jobInstance); interceptor.internalSchedule(jobInstance); @@ -100,10 +93,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) { - logger.warn("EJB timer is null for uuid {} and transactional flag is enabled", uuid); - return false; - } boolean result = scheduler.removeJob(jobHandle, ejbTimer); logger.debug("Remove job returned {}", result); return result; @@ -113,7 +102,6 @@ private TimerJobInstance getTimerJobInstance (String uuid) { return unwrapTimerJobInstance(getEjbTimer(getTimerMappinInfo(uuid))); } - @Override public TimerJobInstance getTimerJobInstance(long processInstanceId, long timerId) { return unwrapTimerJobInstance(getEjbTimer(getTimerMappinInfo(processInstanceId, timerId))); @@ -124,10 +112,9 @@ private Timer getEjbTimer(TimerMappingInfo timerMappingInfo) { if(timerMappingInfo == null || timerMappingInfo.getInfo() == null) { return null; } - byte[] data = timerMappingInfo.getInfo(); - return ((TimerHandle) new ObjectInputStream(new ByteArrayInputStream(data)).readObject()).getTimer(); + return ((TimerHandle) new ObjectInputStream(new ByteArrayInputStream(timerMappingInfo.getInfo())).readObject()).getTimer(); } catch (Exception e) { - logger.warn("wast not able to deserialize info field from timer info for uuid"); + logger.warn("Problem retrieving timer for uuid {}", timerMappingInfo.getUuid(), e); return null; } } @@ -146,30 +133,17 @@ private TimerMappingInfo getTimerMappinInfo(long processInstanceId, long timerId } private TimerMappingInfo getTimerMappingInfo(Function> func) { - InternalRuntimeManager manager = ((GlobalTimerService) globalTimerService).getRuntimeManager(); - String pu = ((InternalRuntimeManager) manager).getDeploymentDescriptor().getPersistenceUnit(); - EntityManagerFactory emf = EntityManagerFactoryManager.get().getOrCreate(pu); - EntityManager em = emf.createEntityManager(); - JtaTransactionManager tm = (JtaTransactionManager) TransactionManagerFactory.get().newTransactionManager(); - boolean txOwner = false; + EntityManager em = EntityManagerFactoryManager.get() + .getOrCreate(globalTimerService.getRuntimeManager() + .getDeploymentDescriptor().getPersistenceUnit()) + .createEntityManager(); try { - if (tm != null && tm.getStatus() == TransactionManager.STATUS_ROLLEDBACK) { - txOwner = tm.begin(); - } List info = func.apply(em); - if (!info.isEmpty()) { - return info.get(0); - } else { - return null; - } - + return !info.isEmpty() ? info.get(0) : null; } catch (Exception ex) { logger.warn("Error getting mapping info ",ex); return null; } finally { - if (tm != null) { - tm.commit(txOwner); - } em.close(); } } @@ -200,7 +174,7 @@ public void internalSchedule(TimerJobInstance timerJobInstance) { @Override public void initScheduler(TimerService timerService) { - this.globalTimerService = timerService; + this.globalTimerService = (GlobalTimerService)timerService; try { this.scheduler = InitialContext.doLookup("java:module/EJBTimerScheduler"); } catch (NamingException e) { @@ -211,18 +185,17 @@ public void initScheduler(TimerService timerService) { @Override public void shutdown() { // managed by container - no op - } @Override public JobHandle buildJobHandleForContext(NamedJobContext ctx) { - return new EjbGlobalJobHandle(-1, getJobName(ctx, -1L), ((GlobalTimerService) globalTimerService).getTimerServiceId()); + return new EjbGlobalJobHandle(-1, getJobName(ctx, -1L), globalTimerService.getTimerServiceId()); } @Override public boolean isTransactional() { - return TRANSACTIONAL; + return true; } @Override @@ -237,11 +210,18 @@ public void setInterceptor(SchedulerServiceInterceptor interceptor) { @Override public boolean isValid(GlobalJobHandle jobHandle) { - - return true; + return true; } protected String getJobName(JobContext ctx, long id) { return JobNameHelper.getJobName(ctx, id); } + + @Override + public + void setEnvironment(RuntimeEnvironment environment) { + if (environment instanceof SimpleRuntimeEnvironment) { + ((SimpleRuntimeEnvironment)environment).addToEnvironment("IS_TIMER_CMT", true); + } + } }