Don't use hazelcast executor in task scheduling #10122

enonic · Apr 24, 2023 · eb67690 · eb67690
1 parent b0c0263
commit eb67690
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 15 deletions.
diff --git a/modules/core/core-api/src/main/java/com/enonic/xp/scheduler/ScheduleCalendar.java b/modules/core/core-api/src/main/java/com/enonic/xp/scheduler/ScheduleCalendar.java
@@ -3,7 +3,6 @@
 import java.io.Serializable;
 import java.time.Duration;
 import java.time.Instant;
-import java.time.ZonedDateTime;
 import java.util.Optional;
 
 import com.enonic.xp.annotation.PublicApi;
@@ -12,7 +11,7 @@
 public interface ScheduleCalendar
     extends Serializable
 {
-    Optional<ZonedDateTime> nextExecution( Instant instant );
+    Optional<Instant> nextExecution( Instant instant );
 
     Optional<Duration> nextExecution();
 

diff --git a/...re-scheduler/src/main/java/com/enonic/xp/impl/scheduler/distributed/CronCalendarImpl.java b/...re-scheduler/src/main/java/com/enonic/xp/impl/scheduler/distributed/CronCalendarImpl.java
@@ -59,9 +59,9 @@ public static Builder create()
     }
 
     @Override
-    public Optional<ZonedDateTime> nextExecution( final Instant instant )
+    public Optional<Instant> nextExecution( final Instant instant )
     {
-        return this.executionTime.nextExecution( ZonedDateTime.ofInstant( instant, timeZone.toZoneId() ) );
+        return this.executionTime.nextExecution( ZonedDateTime.ofInstant( instant, timeZone.toZoneId() ) ).map( ZonedDateTime::toInstant );
     }
 
     @Override

diff --git a/...scheduler/src/main/java/com/enonic/xp/impl/scheduler/distributed/OneTimeCalendarImpl.java b/...scheduler/src/main/java/com/enonic/xp/impl/scheduler/distributed/OneTimeCalendarImpl.java
@@ -2,8 +2,6 @@
 
 import java.time.Duration;
 import java.time.Instant;
-import java.time.ZoneId;
-import java.time.ZonedDateTime;
 import java.util.Optional;
 
 import com.google.common.base.Preconditions;
@@ -41,10 +39,9 @@ public Optional<Duration> nextExecution()
     }
 
     @Override
-    public Optional<ZonedDateTime> nextExecution( final Instant instant )
+    public Optional<Instant> nextExecution( final Instant instant )
     {
-        final ZonedDateTime timeToRun = value.atZone( ZoneId.systemDefault() );
-        return Optional.ofNullable( instant.atZone( ZoneId.systemDefault() ).isBefore( timeToRun ) ? timeToRun : null );
+        return Optional.ofNullable( instant != null ? instant.isBefore( value ) ? value : null : value );
     }
 
     @Override

diff --git a/...core-scheduler/src/main/java/com/enonic/xp/impl/scheduler/distributed/RescheduleTask.java b/...core-scheduler/src/main/java/com/enonic/xp/impl/scheduler/distributed/RescheduleTask.java
@@ -97,7 +97,7 @@ private static void scheduleCronJobs( final Map<ScheduledJobName, ScheduledJob>
                 if ( actualLastRun != null )
                 {
                     calendar.nextExecution( actualLastRun ).ifPresent( nextExecution -> {
-                        if ( nextExecution.isBefore( now.atZone( calendar.getTimeZone().toZoneId() ) ) )
+                        if ( nextExecution.isBefore( now ) )
                         {
                             QUEUE.offer( new JobToRun( job.getName(), now ) );
                         }

diff --git a/...scheduler/src/test/java/com/enonic/xp/impl/scheduler/distributed/OneTimeCalendarTest.java b/...scheduler/src/test/java/com/enonic/xp/impl/scheduler/distributed/OneTimeCalendarTest.java
@@ -36,7 +36,9 @@ public void create()
         OneTimeCalendarImpl calendar = OneTimeCalendarImpl.create().value( now.plus( Duration.of( 1, ChronoUnit.MINUTES ) ) ).build();
 
         assertFalse( calendar.nextExecution().get().isNegative() );
+        assertTrue( calendar.nextExecution( null ).isPresent() );
         assertTrue( calendar.nextExecution( now.plus( Duration.of( 2, ChronoUnit.MINUTES ) ) ).isEmpty() );
+        assertTrue( calendar.nextExecution( now.minus( Duration.of( 2, ChronoUnit.MINUTES ) ) ).isPresent() );
 
         assertEquals( ScheduleCalendarType.ONE_TIME, calendar.getType() );
 

diff --git a/...-scheduler/src/test/java/com/enonic/xp/impl/scheduler/distributed/RescheduleTaskTest.java b/...-scheduler/src/test/java/com/enonic/xp/impl/scheduler/distributed/RescheduleTaskTest.java
@@ -263,18 +263,43 @@ public void submitJobAsUser()
     public void submitCronJob()
     {
         ScheduledJob job1 = mockCronJob( "job1", "* * * * *", Instant.parse( "2021-02-26T10:44:33.170079900Z" ) );
+        ScheduledJob job2 = mockCronJob( "job2", "* * * * *", Instant.now() );
 
-        when( schedulerService.list() ).thenReturn( List.of( job1 ) );
+        when( schedulerService.list() ).thenReturn( List.of( job1, job2 ) );
 
         final Node node = mockNode();
         when( nodeService.update( isA( UpdateNodeParams.class ) ) ).thenReturn( node );
 
-        when( taskService.submitTask( isA( SubmitTaskParams.class ) ) ).thenReturn( TaskId.from( "1" ) );
+        when( taskService.submitTask( isA( SubmitTaskParams.class ) ) ).thenReturn( TaskId.from( "1" ) ).thenReturn( TaskId.from( "2" ) );
         when( securityService.authenticate( tokenCaptor.capture() ) ).thenReturn( mock( AuthenticationInfo.class ) );
 
         createAndRunTask();
 
         verify( taskService, times( 1 ) ).submitTask( taskCaptor.capture() );
+        assertEquals( "job1", taskCaptor.getValue().getDescriptorKey().getName() );
+    }
+
+    @Test
+    public void jobWasRemoved()
+        throws InterruptedException
+    {
+        final Instant plus = null;
+
+        ScheduledJob job1 = mockCronJob( "job1", "* * * * *", plus );
+        ScheduledJob job2 = mockCronJob( "job2", "* * * * *", plus );
+
+        when( schedulerService.list() ).thenReturn( List.of( job1, job2 ) );
+
+        createAndRunTask();
+
+        Thread.sleep( 61000 );
+
+        when( schedulerService.list() ).thenReturn( List.of( job2 ) );
+
+        createAndRunTask();
+
+        verify( taskService, times( 1 ) ).submitTask( taskCaptor.capture() );
+        assertEquals( "job2", taskCaptor.getValue().getDescriptorKey().getName() );
     }
 
     @Test

diff --git a/...erver/server-rest/src/test/java/com/enonic/xp/impl/server/rest/SchedulerResourceTest.java b/...erver/server-rest/src/test/java/com/enonic/xp/impl/server/rest/SchedulerResourceTest.java
@@ -2,7 +2,6 @@
 
 import java.time.Duration;
 import java.time.Instant;
-import java.time.ZonedDateTime;
 import java.util.List;
 import java.util.Optional;
 import java.util.TimeZone;
@@ -63,7 +62,7 @@ public Optional<Duration> nextExecution()
             }
 
             @Override
-            public Optional<ZonedDateTime> nextExecution( final Instant instant )
+            public Optional<Instant> nextExecution( final Instant instant )
             {
                 return Optional.empty();
             }
@@ -96,7 +95,7 @@ public ScheduleCalendarType getType()
             }
 
             @Override
-            public Optional<ZonedDateTime> nextExecution( final Instant instant )
+            public Optional<Instant> nextExecution( final Instant instant )
             {
                 return Optional.empty();
             }