umustafi commented on code in PR #3749:
URL: https://github.com/apache/gobblin/pull/3749#discussion_r1306195581


##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -725,23 +725,23 @@ public void executeImpl(JobExecutionContext context) 
throws JobExecutionExceptio
 
       // Obtain trigger timestamp from trigger to pass to jobProps
       Trigger trigger = context.getTrigger();
-      long triggerTimestampMillis;
+      // THIS current event has already fired if this method is called, so it 
now exists in <previousFireTime>
+      long triggerTimeMillis = asUTCEpochMillis(trigger.getPreviousFireTime());
       // If the trigger is a reminder type event then utilize the trigger time 
saved in job properties rather than the
       // actual firing time
       if (jobDetail.getKey().getName().contains("reminder")) {
-        triggerTimestampMillis = Long.parseLong(
-            
jobProps.getProperty(ConfigurationKeys.SCHEDULER_EVENT_TO_TRIGGER_TIMESTAMP_MILLIS_KEY,
 "0"));
-        long eventToRevisit = Long.parseLong(
-            
jobProps.getProperty(ConfigurationKeys.SCHEDULER_EVENT_TO_REVISIT_TIMESTAMP_MILLIS_KEY,
 "0"));
-        _log.info(jobSchedulerTracePrefixBuilder(jobProps) + "triggerTime: {} 
eventToRevisit: {} - Reminder job "
-            + "triggered by scheduler", triggerTimestampMillis, 
eventToRevisit);
+        long preservedConsensusEventTime = Long.parseLong(jobProps.getProperty(
+            
ConfigurationKeys.SCHEDULER_PRESERVED_CONSENSUS_EVENT_TIME_MILLIS_KEY, "0"));
+        long expectedReminderTime = Long.parseLong(
+            
jobProps.getProperty(ConfigurationKeys.SCHEDULER_EXPECTED_REMINDER_TIME_MILLIS_KEY,
 "0"));
+        _log.info(jobSchedulerTracePrefixBuilder(jobProps) + "triggerTime: {} 
expectedReminderTime: {} - Reminder job "
+            + "triggered by scheduler at {}", preservedConsensusEventTime, 
expectedReminderTime, triggerTimeMillis);
+        // TODO: add a metric if expected reminder time far exceeds system time
       } else {
-        // THIS current event has already fired if this method is called, so 
it now exists in <previousFireTime>
-        triggerTimestampMillis = 
asUTCEpochMillis(trigger.getPreviousFireTime());
-        
jobProps.setProperty(ConfigurationKeys.SCHEDULER_EVENT_TO_TRIGGER_TIMESTAMP_MILLIS_KEY,
-            String.valueOf(triggerTimestampMillis));
+        
jobProps.setProperty(ConfigurationKeys.SCHEDULER_PRESERVED_CONSENSUS_EVENT_TIME_MILLIS_KEY,
+            String.valueOf(triggerTimeMillis));

Review Comment:
   We use this timestamp when we call `orchestrate` to run the actual job 
forwarding from the scheduler to execution pt 
https://github.com/apache/gobblin/blob/90380fc4bcf5db13c306d2ac3b49b613d84830b8/gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java#L485C26-L485C26
 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to