From f2ea2b032b660ff10b1ac823dae13ac858bd7943 Mon Sep 17 00:00:00 2001 From: Matej Novotny Date: Tue, 30 Jul 2024 14:54:30 +0200 Subject: [PATCH 1/2] Quartz - Do not recreate job instances for re-fires --- .../quartz/test/DependentBeanJobTest.java | 16 ++++++++-------- .../io/quarkus/quartz/runtime/CdiAwareJob.java | 6 +++++- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/extensions/quartz/deployment/src/test/java/io/quarkus/quartz/test/DependentBeanJobTest.java b/extensions/quartz/deployment/src/test/java/io/quarkus/quartz/test/DependentBeanJobTest.java index 9a2943c1ab78a..410018b410a2b 100644 --- a/extensions/quartz/deployment/src/test/java/io/quarkus/quartz/test/DependentBeanJobTest.java +++ b/extensions/quartz/deployment/src/test/java/io/quarkus/quartz/test/DependentBeanJobTest.java @@ -86,10 +86,10 @@ public void testDependentBeanJobDestroyed() throws SchedulerException, Interrupt @Test public void testDependentBeanJobWithRefire() throws SchedulerException, InterruptedException { - // 5 one-off jobs should trigger construction/execution/destruction 10 times in total - CountDownLatch execLatch = service.initExecuteLatch(10); - CountDownLatch constructLatch = service.initConstructLatch(10); - CountDownLatch destroyedLatch = service.initDestroyedLatch(10); + // 5 one-off jobs should trigger construction/execution/destruction 5 times in total + CountDownLatch execLatch = service.initExecuteLatch(5); + CountDownLatch constructLatch = service.initConstructLatch(5); + CountDownLatch destroyedLatch = service.initDestroyedLatch(5); for (int i = 0; i < 5; i++) { Trigger trigger = TriggerBuilder.newTrigger() .withIdentity("myTrigger" + i, "myRefiringGroup") @@ -104,10 +104,10 @@ public void testDependentBeanJobWithRefire() throws SchedulerException, Interrup assertTrue(constructLatch.await(2, TimeUnit.SECONDS), "Latch count: " + constructLatch.getCount()); assertTrue(destroyedLatch.await(2, TimeUnit.SECONDS), "Latch count: " + destroyedLatch.getCount()); - // repeating job triggering three times; we expect six beans to exist for that due to refires - execLatch = service.initExecuteLatch(6); - constructLatch = service.initConstructLatch(6); - destroyedLatch = service.initDestroyedLatch(6); + // repeating job triggering three times; re-fires should NOT recreate the bean instance + execLatch = service.initExecuteLatch(3); + constructLatch = service.initConstructLatch(3); + destroyedLatch = service.initDestroyedLatch(3); JobDetail job = JobBuilder.newJob(RefiringJob.class) .withIdentity("myRepeatingJob", "myRefiringGroup") .build(); diff --git a/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/CdiAwareJob.java b/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/CdiAwareJob.java index 4e02136078ef9..fde25d16681d7 100644 --- a/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/CdiAwareJob.java +++ b/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/CdiAwareJob.java @@ -28,10 +28,14 @@ public CdiAwareJob(Instance jobInstance) { @Override public void execute(JobExecutionContext context) throws JobExecutionException { Instance.Handle handle = jobInstance.getHandle(); + boolean refire = false; try { handle.get().execute(context); + } catch (JobExecutionException e) { + refire = e.refireImmediately(); + throw e; } finally { - if (handle.getBean().getScope().equals(Dependent.class)) { + if (refire != true && handle.getBean().getScope().equals(Dependent.class)) { handle.destroy(); } } From 2498f5ca417255a11f857da06b3792458eeb57e1 Mon Sep 17 00:00:00 2001 From: Matej Novotny Date: Tue, 30 Jul 2024 15:21:58 +0200 Subject: [PATCH 2/2] Quartz - Use the same dep. bean instance for both, execution and interruption of CdiAwareJob --- .../programmatic/InterruptableJobTest.java | 11 +++++++++- .../quarkus/quartz/runtime/CdiAwareJob.java | 20 +++++++------------ .../quartz/runtime/QuartzSchedulerImpl.java | 2 +- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/extensions/quartz/deployment/src/test/java/io/quarkus/quartz/test/programmatic/InterruptableJobTest.java b/extensions/quartz/deployment/src/test/java/io/quarkus/quartz/test/programmatic/InterruptableJobTest.java index f7226ab5d5d4b..8381694a248cc 100644 --- a/extensions/quartz/deployment/src/test/java/io/quarkus/quartz/test/programmatic/InterruptableJobTest.java +++ b/extensions/quartz/deployment/src/test/java/io/quarkus/quartz/test/programmatic/InterruptableJobTest.java @@ -6,6 +6,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import jakarta.annotation.PostConstruct; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; @@ -40,6 +41,7 @@ public class InterruptableJobTest { static final CountDownLatch INTERRUPT_LATCH = new CountDownLatch(1); static final CountDownLatch EXECUTE_LATCH = new CountDownLatch(1); + static Integer initCounter = 0; static final CountDownLatch NON_INTERRUPTABLE_EXECUTE_LATCH = new CountDownLatch(1); static final CountDownLatch NON_INTERRUPTABLE_HOLD_LATCH = new CountDownLatch(1); @@ -66,7 +68,9 @@ public void testInterruptableJob() throws InterruptedException { throw new RuntimeException(e); } - assertTrue(INTERRUPT_LATCH.await(5, TimeUnit.SECONDS)); + assertTrue(INTERRUPT_LATCH.await(3, TimeUnit.SECONDS)); + // asserts that a single dep. scoped bean instance was used for both, execute() and interrupt() methods + assertTrue(initCounter == 1); } @Test @@ -102,6 +106,11 @@ public void testNonInterruptableJob() throws InterruptedException { @ApplicationScoped static class MyJob implements InterruptableJob { + @PostConstruct + public void postConstruct() { + initCounter++; + } + @Override public void execute(JobExecutionContext context) { EXECUTE_LATCH.countDown(); diff --git a/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/CdiAwareJob.java b/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/CdiAwareJob.java index fde25d16681d7..8c3e42c1becd0 100644 --- a/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/CdiAwareJob.java +++ b/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/CdiAwareJob.java @@ -19,18 +19,19 @@ */ class CdiAwareJob implements InterruptableJob { - private final Instance jobInstance; + private final Instance.Handle handle; + private final Job beanInstance; - public CdiAwareJob(Instance jobInstance) { - this.jobInstance = jobInstance; + public CdiAwareJob(Instance.Handle handle) { + this.handle = handle; + this.beanInstance = handle.get(); } @Override public void execute(JobExecutionContext context) throws JobExecutionException { - Instance.Handle handle = jobInstance.getHandle(); boolean refire = false; try { - handle.get().execute(context); + beanInstance.execute(context); } catch (JobExecutionException e) { refire = e.refireImmediately(); throw e; @@ -43,16 +44,9 @@ public void execute(JobExecutionContext context) throws JobExecutionException { @Override public void interrupt() throws UnableToInterruptJobException { - Instance.Handle handle = jobInstance.getHandle(); // delegate if possible; throw an exception in other cases if (InterruptableJob.class.isAssignableFrom(handle.getBean().getBeanClass())) { - try { - ((InterruptableJob) handle.get()).interrupt(); - } finally { - if (handle.getBean().getScope().equals(Dependent.class)) { - handle.destroy(); - } - } + ((InterruptableJob) beanInstance).interrupt(); } else { throw new UnableToInterruptJobException("Job " + handle.getBean().getBeanClass() + " can not be interrupted, since it does not implement " + InterruptableJob.class.getName()); diff --git a/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzSchedulerImpl.java b/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzSchedulerImpl.java index 7e08b2a596de5..572192f58a976 100644 --- a/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzSchedulerImpl.java +++ b/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzSchedulerImpl.java @@ -1246,7 +1246,7 @@ public Job newJob(TriggerFiredBundle bundle, org.quartz.Scheduler Scheduler) thr Instance instance = jobs.select(jobClass); if (instance.isResolvable()) { // This is a job backed by a CDI bean - return jobWithSpanWrapper(new CdiAwareJob(instance)); + return jobWithSpanWrapper(new CdiAwareJob(instance.getHandle())); } // Instantiate a plain job class return jobWithSpanWrapper(super.newJob(bundle, Scheduler));