From d79847f072b2d43b68d829478ba9f2c545b0f4ee Mon Sep 17 00:00:00 2001 From: Francisco Javier Tirado Sarti Date: Fri, 27 Sep 2024 15:34:53 +0200 Subject: [PATCH] [JBPM-10242] Always skipping linear search Unless explicilty stated with property org.jbpm.ejb.timer.linear.search --- .../services/ejb/timer/EJBTimerScheduler.java | 124 +++++++++--------- .../ejb/timer/EJBTimerSchedulerTest.java | 8 ++ 2 files changed, 70 insertions(+), 62 deletions(-) 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 c5cc94c366..ab3fea23f4 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 @@ -140,7 +140,7 @@ else if (ejbTimerJob.getTimerJobInstance().getTrigger().hasNextFireTime() != nul // because of the transaction, so we need to do this here. tx = timerJobInstance -> { logger.warn("Execution of time failed Interval Trigger failed. Skipping {}", timerJobInstance); - if (removeJob(timerJobInstance.getJobHandle(), timer, true)) { + if (removeJob(timerJobInstance.getJobHandle(), timer)) { internalSchedule(timerJobInstance); } else { logger.debug("Interval trigger {} was removed before rescheduling", timerJobInstance); @@ -247,11 +247,12 @@ private Serializable removeTransientFields(Serializable info) { return info; } - public boolean removeJob(JobHandle jobHandle, Timer ejbTimer) { - return removeJob(jobHandle, ejbTimer, false); + private boolean performLinearSearch() { + return Boolean.getBoolean("org.jbpm.ejb.timer.linear.search"); } + - public boolean removeJob(JobHandle jobHandle, Timer ejbTimer, boolean searchIfFailed) { + public boolean removeJob(JobHandle jobHandle, Timer ejbTimer) { EjbGlobalJobHandle ejbHandle = (EjbGlobalJobHandle) jobHandle; if (useLocalCache) { boolean removedFromCache = localCache.remove(ejbHandle.getUuid()) != null; @@ -263,10 +264,8 @@ public boolean removeJob(JobHandle jobHandle, Timer ejbTimer, boolean searchIfFa ejbTimer.cancel(); return true; } catch (Exception e) { - logger.warn("Timer cancel error for handle {}", ejbHandle, e); - if (!searchIfFailed) { - return false; - } + logger.warn("Timer cancel error for handle {}", ejbHandle.getUuid(), e); + return false; } } @@ -289,36 +288,36 @@ public boolean removeJob(JobHandle jobHandle, Timer ejbTimer, boolean searchIfFa logger.warn("No timerJobInstance available for {}", ejbHandle); } - if (!searchIfFailed) { - logger.warn("Timer not found for {} and {}, skipping list search", ejbHandle, ejbTimer); + if (performLinearSearch()) { + for (Timer timer : timerService.getTimers()) { + try { + Serializable info = timer.getInfo(); + if (info instanceof EjbTimerJob) { + EjbTimerJob job = (EjbTimerJob) info; + + EjbGlobalJobHandle handle = (EjbGlobalJobHandle) job.getTimerJobInstance().getJobHandle(); + if (handle.getUuid().equals(ejbHandle.getUuid())) { + logger.info("Job handle {} does match timer and is going to be canceled", jobHandle); + + try { + timer.cancel(); + } catch (Throwable e) { + logger.warn("Timer cancel error for handle {}", handle, e); + return false; + } + return true; + } + } + } catch (NoSuchObjectLocalException e) { + logger.debug("Timer {} has already expired or was canceled ", timer); + } + } + logger.info("Job handle {} does not match any timer on {} scheduler service", jobHandle, this); + return false; + } else { + logger.warn("Skipping linear search to delete uuid {}", ejbHandle.getUuid()); return false; } - - for (Timer timer : timerService.getTimers()) { - try { - Serializable info = timer.getInfo(); - if (info instanceof EjbTimerJob) { - EjbTimerJob job = (EjbTimerJob) info; - - EjbGlobalJobHandle handle = (EjbGlobalJobHandle) job.getTimerJobInstance().getJobHandle(); - if (handle.getUuid().equals(ejbHandle.getUuid())) { - logger.info("Job handle {} does match timer and is going to be canceled", jobHandle); - - try { - timer.cancel(); - } catch (Throwable e) { - logger.warn("Timer cancel error for handle {}", handle, e); - return false; - } - return true; - } - } - } catch (NoSuchObjectLocalException e) { - logger.debug("Timer {} has already expired or was canceled ", timer); - } - } - logger.info("Job handle {} does not match any timer on {} scheduler service", jobHandle, this); - return false; } @@ -330,33 +329,34 @@ public TimerJobInstance getTimerByName(String jobName) { return localCache.get(jobName); } } - TimerJobInstance found = null; - - for (Timer timer : timerService.getTimers()) { - try { - Serializable info = timer.getInfo(); - if (info instanceof EjbTimerJob) { - EjbTimerJob job = (EjbTimerJob) info; - - EjbGlobalJobHandle handle = (EjbGlobalJobHandle) job.getTimerJobInstance().getJobHandle(); - - if (handle.getUuid().equals(jobName)) { - found = handle.getTimerJobInstance(); - if (useLocalCache) { - localCache.putIfAbsent(jobName, found); - } - logger.debug("Job {} does match timer and is going to be returned {}", jobName, found); - - break; - } - } - } catch (NoSuchObjectLocalException e) { - logger.debug("Timer info for {} was not found ", timer); + TimerJobInstance found = null; + if (performLinearSearch()) { + for (Timer timer : timerService.getTimers()) { + try { + Serializable info = timer.getInfo(); + if (info instanceof EjbTimerJob) { + EjbTimerJob job = (EjbTimerJob) info; + + EjbGlobalJobHandle handle = (EjbGlobalJobHandle) job.getTimerJobInstance().getJobHandle(); + + if (handle.getUuid().equals(jobName)) { + found = handle.getTimerJobInstance(); + if (useLocalCache) { + localCache.putIfAbsent(jobName, found); + } + logger.debug("Job {} does match timer and is going to be returned {}", jobName, found); + break; + } + } + } catch (NoSuchObjectLocalException e) { + logger.debug("Timer info for {} was not found ", timer); + } } - } - - return found; - } + } else { + logger.warn("Skipping linear search to find uuid {}", jobName); + } + return found; + } public void evictCache(JobHandle jobHandle) { String jobName = ((EjbGlobalJobHandle) jobHandle).getUuid(); diff --git a/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/test/java/org/jbpm/services/ejb/timer/EJBTimerSchedulerTest.java b/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/test/java/org/jbpm/services/ejb/timer/EJBTimerSchedulerTest.java index 8356362503..c470532b4e 100644 --- a/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/test/java/org/jbpm/services/ejb/timer/EJBTimerSchedulerTest.java +++ b/jbpm-services/jbpm-services-ejb/jbpm-services-ejb-timer/src/test/java/org/jbpm/services/ejb/timer/EJBTimerSchedulerTest.java @@ -28,6 +28,7 @@ import org.drools.core.time.impl.TimerJobInstance; import org.jbpm.persistence.timer.GlobalJpaTimerJobInstance; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -41,6 +42,8 @@ public class EJBTimerSchedulerTest { @Before public void setup() { + + System.setProperty("org.jbpm.ejb.timer.linear.search", "true"); TimerService timerService = mock(TimerService.class); when(timerService.getTimers()).thenReturn(timers); @@ -58,6 +61,11 @@ public void setup() { scheduler.timerService = timerService; } + @After + public void cleanup() { + System.clearProperty("org.jbpm.ejb.timer.linear.search"); + } + @Test public void testEjbTimerSchedulerTestOnTimerLoop() { // first call to go over list of timers should not add anything to the cache as there is no matching timers