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 ab3fea23f4..60f0ce1bf8 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 @@ -26,9 +26,13 @@ import java.time.ZonedDateTime; import java.time.temporal.ChronoUnit; import java.util.Date; +import java.util.Optional; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.function.BiFunction; +import java.util.function.Consumer; +import java.util.function.Function; import javax.annotation.PostConstruct; import javax.annotation.Resource; @@ -247,8 +251,8 @@ private Serializable removeTransientFields(Serializable info) { return info; } - private boolean performLinearSearch() { - return Boolean.getBoolean("org.jbpm.ejb.timer.linear.search"); + private boolean disableLinearSearch(String suffix) { + return Boolean.getBoolean("org.jbpm.ejb.timer.disable.linear." + suffix); } @@ -260,13 +264,7 @@ public boolean removeJob(JobHandle jobHandle, Timer ejbTimer) { } if (ejbTimer != null) { - try { - ejbTimer.cancel(); - return true; - } catch (Exception e) { - logger.warn("Timer cancel error for handle {}", ejbHandle.getUuid(), e); - return false; - } + return cancelTimer(ejbTimer, ejbHandle); } // small speed improvement using the ejb serializable info handler @@ -274,88 +272,66 @@ public boolean removeJob(JobHandle jobHandle, Timer ejbTimer) { if (timerJobInstance != null) { Object ejbTimerHandle = timerJobInstance.getTimerInfo(); if(ejbTimerHandle instanceof TimerHandle) { - try { - ((TimerHandle) ejbTimerHandle).getTimer().cancel(); - } catch (Throwable e) { - logger.warn("Timer cancel error for handle {}", ejbTimerHandle, e); - return false; - } - return true; - } else { - logger.warn("No TimerHandle found for {}: {}", ejbHandle, ejbTimerHandle); + return cancelTimer(((TimerHandle) ejbTimerHandle).getTimer(), ejbHandle); } - } else { - logger.warn("No timerJobInstance available for {}", ejbHandle); } + logger.info("No valid TimerJob instance {} available for Job handle {}", timerJobInstance, ejbHandle); - 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); + if (disableLinearSearch("remove")) { + logger.warn("Skipping linear search to delete UUID {}", ejbHandle.getUuid()); return false; } else { - logger.warn("Skipping linear search to delete uuid {}", ejbHandle.getUuid()); - return false; + return linearSearch(ejbHandle.getUuid(), + (timer, job) -> cancelTimer(timer, (EjbGlobalJobHandle) job.getJobHandle())).orElse(false); } - } + } + public TimerJobInstance getTimerByName(String jobName) { + if (useLocalCache && localCache.containsKey(jobName)) { + logger.debug("Found job {} in cache returning", jobName); + return localCache.get(jobName); + } else if (disableLinearSearch("search")) { + logger.warn("Skipping linear search to find UUID {}", jobName); + return null; + } else { + return linearSearch(jobName, (timer, job) -> { + if (useLocalCache) { + localCache.putIfAbsent(jobName, job); + } + return job; + }).orElse(null); + } + } + private boolean cancelTimer(Timer timer, EjbGlobalJobHandle ejbHandle) { + try { + timer.cancel(); + return true; + } catch (Exception ex) { + logger.warn("Timer cancel failed for handle {}", ejbHandle, ex); + return false; + } + } - public TimerJobInstance getTimerByName(String jobName) { - if (useLocalCache) { - if (localCache.containsKey(jobName)) { - logger.debug("Found job {} in cache returning", jobName); - return localCache.get(jobName); - } - } - 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; - } + private Optional linearSearch(String uuid, BiFunction function) { + logger.info("Searching UUID {} on {} scheduler service", uuid, this); + 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(uuid)) { + logger.debug("Handle {} does match timer {}", job.getTimerJobInstance()); + return Optional.of(function.apply(timer, job.getTimerJobInstance())); } - } catch (NoSuchObjectLocalException e) { - logger.debug("Timer info for {} was not found ", timer); } + } catch (NoSuchObjectLocalException e) { + logger.info("Timer info for {} is not there ", timer, e); } - } else { - logger.warn("Skipping linear search to find uuid {}", jobName); } - return found; + logger.info("UUID {} does not match any timer on {} scheduler service", uuid, this); + return Optional.empty(); } public void evictCache(JobHandle jobHandle) { 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 c470532b4e..8356362503 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,7 +28,6 @@ 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; @@ -42,8 +41,6 @@ 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); @@ -61,11 +58,6 @@ 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