This is an automated email from the ASF dual-hosted git repository.

deepak pushed a commit to branch release18.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/release18.12 by this push:
     new 4c41b35560 Fixed Issue with OFBiz Job Scheduler and Daylight Saving 
Time  (OFBIZ-12864) (#674)
4c41b35560 is described below

commit 4c41b355608cbf6c414f2af7111a22aab37dbf7d
Author: Deepak Dixit <dee...@apache.org>
AuthorDate: Thu Nov 2 12:37:34 2023 +0530

    Fixed Issue with OFBiz Job Scheduler and Daylight Saving Time  
(OFBIZ-12864) (#674)
    
    * Fixed recurring job not scheduling when DST change
    The issue occurs when DST changes, and OFBiz fails to schedule recurring 
jobs properly. This is due to a condition in the 
PersistedServiceJob.createRecurrence method where it compares the next 
scheduled time (next) with the start time (startTime) for the job.
    To address the issue, adding a new field named JobSandbox.runTimeEpoch.
    This field would store the UTC format epoch milliseconds of the runtime 
date.
    When scheduling or rescheduling recurring jobs, the system would use the 
UTC epoch stored in JobSandbox.runTimeEpoch for comparison.
    This solution ensures that the system uses a consistent, UTC-based time for 
scheduling and rescheduling recurring jobs, even when DST changes affect the 
local time.
    To implement this solution, you would need to:
    
    Modify the PersistedServiceJob.createRecurrence method to calculate and 
store the UTC epoch milliseconds in the JobSandbox.runTimeEpoch field.
    
    Update the code responsible for polling and rescheduling jobs to use the 
JobSandbox.runTimeEpoch field when it is set. If the field is not set, you 
would fall back to getting the runtime date to filter the jobs.
    
    By using this approach, system should be able to handle recurring job 
scheduling more reliably, especially when DST changes are involved, as it 
ensures that all time comparisons are made in a consistent UTC format.
    
    * Checkstyle fixes
    
    * Store the runTimeEpoch while recurring job scheduling
---
 framework/service/entitydef/entitymodel.xml        |  1 +
 .../org/apache/ofbiz/service/job/JobManager.java   | 26 ++++++++++++++++-
 .../ofbiz/service/job/PersistedServiceJob.java     | 34 +++++++++++++++++++---
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/framework/service/entitydef/entitymodel.xml 
b/framework/service/entitydef/entitymodel.xml
index 7a1043a37d..78c246b61f 100644
--- a/framework/service/entitydef/entitymodel.xml
+++ b/framework/service/entitydef/entitymodel.xml
@@ -44,6 +44,7 @@ under the License.
         <field name="jobId" type="id"></field>
         <field name="jobName" type="name"></field>
         <field name="runTime" type="date-time"></field>
+        <field name="runTimeEpoch" type="numeric"/><!-- Store the Epoch 
millisecond to avoid DST change issue (when clock set 1 hr back) -->
         <field name="poolId" type="name"></field>
         <field name="statusId" type="id"></field>
         <field name="parentJobId" type="id"></field>
diff --git 
a/framework/service/src/main/java/org/apache/ofbiz/service/job/JobManager.java 
b/framework/service/src/main/java/org/apache/ofbiz/service/job/JobManager.java
index 93c189bc16..bf1d73d2fc 100644
--- 
a/framework/service/src/main/java/org/apache/ofbiz/service/job/JobManager.java
+++ 
b/framework/service/src/main/java/org/apache/ofbiz/service/job/JobManager.java
@@ -20,6 +20,10 @@ package org.apache.ofbiz.service.job;
 
 import java.io.IOException;
 import java.sql.Timestamp;
+import java.time.Duration;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
@@ -183,7 +187,27 @@ public final class JobManager {
             return Collections.emptyList();
         }
         // basic query
-        List<EntityExpr> expressions = 
UtilMisc.toList(EntityCondition.makeCondition("runTime", 
EntityOperator.LESS_THAN_EQUAL_TO, UtilDateTime.nowTimestamp()),
+        /*
+            By adding the runTimeEpoch field to handle DST changes in 
recurring job scheduling is a practical solution.
+            By storing the runtime epoch in UTC format, to make the system 
DST-aware, which helps prevent issues related to time changes,
+                especially when the clock is set back by 1 hour during the 
transition.
+            Additionally, keeping the runtime field while polling is a good 
practice,
+                as it ensures backward compatibility and provides flexibility 
in situations where the system sets the jobSandbox.runtime value.
+            This approach allows the system to work with both the new 
UTC-based runTimeEpoch field and the existing runtime field, as needed.
+            To summarize, by introducing the runTimeEpoch field and handling 
the transition between UTC epoch time and the runtime field,
+                to  make recurring job scheduling more robust and DST-aware,
+                which should help prevent scheduling issues during Daylight 
Saving Time changes.
+         */
+        List<EntityCondition> expressions = UtilMisc.toList(
+                EntityCondition.makeCondition(
+                        EntityCondition.makeCondition("runTimeEpoch",
+                                EntityOperator.LESS_THAN_EQUAL_TO, 
ZonedDateTime.now(ZoneId.of("UTC")).toInstant().toEpochMilli()),
+                        EntityOperator.OR,
+                        EntityCondition.makeCondition(
+                                EntityCondition.makeCondition("runTimeEpoch", 
null),
+                                EntityOperator.AND,
+                                EntityCondition.makeCondition("runTime",
+                                        EntityOperator.LESS_THAN_EQUAL_TO, 
UtilDateTime.nowTimestamp()))),
                 EntityCondition.makeCondition("startDateTime", 
EntityOperator.EQUALS, null),
                 EntityCondition.makeCondition("cancelDateTime", 
EntityOperator.EQUALS, null),
                 EntityCondition.makeCondition("runByInstanceId", 
EntityOperator.EQUALS, null));
diff --git 
a/framework/service/src/main/java/org/apache/ofbiz/service/job/PersistedServiceJob.java
 
b/framework/service/src/main/java/org/apache/ofbiz/service/job/PersistedServiceJob.java
index 369811f3f6..5683e783fe 100644
--- 
a/framework/service/src/main/java/org/apache/ofbiz/service/job/PersistedServiceJob.java
+++ 
b/framework/service/src/main/java/org/apache/ofbiz/service/job/PersistedServiceJob.java
@@ -20,6 +20,13 @@ package org.apache.ofbiz.service.job;
 
 import java.io.IOException;
 import java.sql.Timestamp;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeFormatterBuilder;
+import java.time.format.SignStyle;
+import java.time.temporal.ChronoField;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
@@ -60,6 +67,13 @@ import com.ibm.icu.util.Calendar;
 public class PersistedServiceJob extends GenericServiceJob {
 
     public static final String module = PersistedServiceJob.class.getName();
+    private static final DateTimeFormatter FORMATTER = new 
DateTimeFormatterBuilder()
+            .append(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"))
+            .appendOptional(
+                    new DateTimeFormatterBuilder()
+                            .appendLiteral('.')
+                            .appendValue(ChronoField.MICRO_OF_SECOND, 1, 3, 
SignStyle.NOT_NEGATIVE).toFormatter())
+            .toFormatter().withZone(ZonedDateTime.now().getZone());
 
     private final transient Delegator delegator;
     private long nextRecurrence = -1;
@@ -78,8 +92,14 @@ public class PersistedServiceJob extends GenericServiceJob {
         super(dctx, jobValue.getString("jobId"), 
jobValue.getString("jobName"), null, null, req);
         this.delegator = dctx.getDelegator();
         this.jobValue = jobValue;
-        Timestamp storedDate = jobValue.getTimestamp("runTime");
-        this.startTime = storedDate.getTime();
+        /*
+        This solution ensures that the system uses a consistent,
+         UTC-based time for scheduling and rescheduling recurring jobs, even 
when DST changes affect the local time.
+         */
+        ZonedDateTime startTimeZD = ZonedDateTime.parse(
+                jobValue.getString("runTime"), 
FORMATTER).withZoneSameInstant(ZoneId.of("UTC"));
+        this.startTime = UtilValidate.isNotEmpty(jobValue.get("runTimeEpoch"))
+                ? jobValue.getLong("runTimeEpoch") : 
startTimeZD.toInstant().toEpochMilli();
         this.maxRetry = jobValue.get("maxRetry") != null ? 
jobValue.getLong("maxRetry") : 0;
         Long retryCount = jobValue.getLong("currentRetryCount");
         if (retryCount != null) {
@@ -192,7 +212,12 @@ public class PersistedServiceJob extends GenericServiceJob 
{
         if (Debug.verboseOn()) {
             Debug.logVerbose("Next runtime returned: " + next, module);
         }
-        if (next > startTime) {
+        /*
+        This solution ensures that the system uses a consistent,
+        UTC-based time for scheduling and rescheduling recurring jobs, even 
when DST changes affect the local time.
+        */
+        ZonedDateTime nextRunTime = 
ZonedDateTime.ofInstant(Instant.ofEpochMilli(next), ZoneId.of("UTC"));
+        if (nextRunTime.toInstant().toEpochMilli() > startTime) {
             String pJobId = jobValue.getString("parentJobId");
             if (pJobId == null) {
                 pJobId = jobValue.getString("jobId");
@@ -204,7 +229,8 @@ public class PersistedServiceJob extends GenericServiceJob {
             newJob.set("statusId", "SERVICE_PENDING");
             newJob.set("startDateTime", null);
             newJob.set("runByInstanceId", null);
-            newJob.set("runTime", new java.sql.Timestamp(next));
+            newJob.set("runTime", Timestamp.from(nextRunTime.toInstant()));
+            newJob.set("runTimeEpoch", nextRunTime.toInstant().toEpochMilli());
             if (isRetryOnFailure) {
                 newJob.set("currentRetryCount", currentRetryCount + 1);
             } else {

Reply via email to