You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by "umustafi (via GitHub)" <gi...@apache.org> on 2023/08/24 23:55:37 UTC
[GitHub] [gobblin] umustafi opened a new pull request, #3749: Refactor FlowTriggerHandler static functions and add unit tests
umustafi opened a new pull request, #3749:
URL: https://github.com/apache/gobblin/pull/3749
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
### JIRA
- [X] My PR addresses the following [Gobblin JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-1887
### Description
- [X] Here are some details about my PR, including screenshots (if applicable):
Increase abstraction and encapsulation in `FlowTriggerHandler` with minor refactoring to static helper functions. Unpack objects as late as possible to make `scheduleReminderForEvent` easier to read. Add unit tests for each of the static helper functions. Also ensure that the `reminderJobKey` is also used as the `reminderTrigger's` key.
### Tests
- [X] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
- testCreateSuffixForJobTrigger
- testConstructReminderJobKey
### Commits
- [X] My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] umustafi commented on a diff in pull request #3749: [GOBBLIN-1887] Increase Encapsulation in FlowTriggerHandler helper functions and Add Unit Tests
Posted by "umustafi (via GitHub)" <gi...@apache.org>.
umustafi commented on code in PR #3749:
URL: https://github.com/apache/gobblin/pull/3749#discussion_r1306022515
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/FlowTriggerHandler.java:
##########
@@ -181,36 +179,43 @@ private void scheduleReminderForEvent(Properties jobProps, MultiActiveLeaseArbit
this.jobDoesNotExistInSchedulerCount.inc();
return;
}
- JobKey reminderJobKey = constructReminderJobKey(origJobKey, status.getEventTimeMillis());
- JobDetailImpl jobDetail = createJobDetailForReminderEvent(origJobKey, reminderJobKey, cronExpression,
- status.getEventTimeMillis(), originalEventTimeMillis);
- // Create a new trigger that is set to fire at the minimum reminder wait time calculated
+ JobKey reminderJobKey = constructReminderJobKey(origJobKey, status);
+ JobDetailImpl jobDetail = createJobDetailForReminderEvent(origJobKey, reminderJobKey, status,
+ triggerEventTimeMillis, schedulerMaxBackoffMillis);
+ // Create a new trigger with a `reminder` suffix that is set to fire at the minimum reminder wait time calculated
Trigger reminderTrigger = JobScheduler.createTriggerForJob(reminderJobKey,
- (Properties) jobDetail.getJobDataMap().get(GobblinServiceJobScheduler.PROPERTIES_KEY), Optional.absent());
+ getJobPropertiesFromJobDetail(jobDetail), Optional.of(createSuffixForJobTrigger(status)));
// TODO: remove this comment once we've confirmed this function works
- log.info("Flow Trigger Handler - [{}, eventTimestamp: {}] - attempting to schedule reminder for event {}",
- flowAction, originalEventTimeMillis, status.getEventTimeMillis());
+ log.info("Flow Trigger Handler - [{}, eventTimestamp: {}] - attempting to schedule reminder for event {} with"
+ + "reminderJobKey {} and reminderTriggerKey {}", flowAction, triggerEventTimeMillis,
+ status.getEventTimeMillis(), reminderJobKey, reminderTrigger.getKey());
this.schedulerService.getScheduler().scheduleJob(jobDetail, reminderTrigger);
Review Comment:
Good suggestion, updated!
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] phet commented on a diff in pull request #3749: [GOBBLIN-1887] Increase Encapsulation in FlowTriggerHandler helper functions and Handle Reminder Events Properly
Posted by "phet (via GitHub)" <gi...@apache.org>.
phet commented on code in PR #3749:
URL: https://github.com/apache/gobblin/pull/3749#discussion_r1306186753
##########
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:
this `else` is when it's not a reminder trigger, but rather from the original cron schedule, correct? if so, why do we set this property?
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] Will-Lo merged pull request #3749: [GOBBLIN-1887] Increase Encapsulation in FlowTriggerHandler helper functions and Handle Reminder Events Properly
Posted by "Will-Lo (via GitHub)" <gi...@apache.org>.
Will-Lo merged PR #3749:
URL: https://github.com/apache/gobblin/pull/3749
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] phet commented on a diff in pull request #3749: [GOBBLIN-1887] Increase Encapsulation in FlowTriggerHandler helper functions and Add Unit Tests
Posted by "phet (via GitHub)" <gi...@apache.org>.
phet commented on code in PR #3749:
URL: https://github.com/apache/gobblin/pull/3749#discussion_r1306034249
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -725,12 +725,24 @@ public void executeImpl(JobExecutionContext context) throws JobExecutionExceptio
// Obtain trigger timestamp from trigger to pass to jobProps
Trigger trigger = context.getTrigger();
- // THIS current event has already fired if this method is called, so it now exists in <previousFireTime>
- long triggerTimestampMillis = asUTCEpochMillis(trigger.getPreviousFireTime());
- jobProps.setProperty(ConfigurationKeys.SCHEDULER_EVENT_TO_TRIGGER_TIMESTAMP_MILLIS_KEY,
- String.valueOf(triggerTimestampMillis));
- _log.info(jobSchedulerTracePrefixBuilder(jobProps) + "triggerTime: {} nextTriggerTime: {} - Job triggered by "
- + "scheduler", triggerTimestampMillis, asUTCEpochMillis(trigger.getNextFireTime()));
+ long triggerTimestampMillis;
+ // 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"));
Review Comment:
I'm getting these confused, but want to ensure they're not mixed up... where could I find a comment explaining the difference (or is it worth adding one here)?
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] phet commented on a diff in pull request #3749: [GOBBLIN-1887] Increase Encapsulation in FlowTriggerHandler helper functions and Handle Reminder Events Properly
Posted by "phet (via GitHub)" <gi...@apache.org>.
phet commented on code in PR #3749:
URL: https://github.com/apache/gobblin/pull/3749#discussion_r1306217756
##########
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:
to avoid confusion, since the cron-schedule event time is not "preserved consensus", how about another, third property for this purpose? have both the cron-schedule and reminder paths stick their relevant value into that "neutral" property.
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] phet commented on a diff in pull request #3749: [GOBBLIN-1887] Increase Encapsulation in FlowTriggerHandler helper functions and Add Unit Tests
Posted by "phet (via GitHub)" <gi...@apache.org>.
phet commented on code in PR #3749:
URL: https://github.com/apache/gobblin/pull/3749#discussion_r1305155379
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/FlowTriggerHandler.java:
##########
@@ -181,36 +179,43 @@ private void scheduleReminderForEvent(Properties jobProps, MultiActiveLeaseArbit
this.jobDoesNotExistInSchedulerCount.inc();
return;
}
- JobKey reminderJobKey = constructReminderJobKey(origJobKey, status.getEventTimeMillis());
- JobDetailImpl jobDetail = createJobDetailForReminderEvent(origJobKey, reminderJobKey, cronExpression,
- status.getEventTimeMillis(), originalEventTimeMillis);
- // Create a new trigger that is set to fire at the minimum reminder wait time calculated
+ JobKey reminderJobKey = constructReminderJobKey(origJobKey, status);
+ JobDetailImpl jobDetail = createJobDetailForReminderEvent(origJobKey, reminderJobKey, status,
+ triggerEventTimeMillis, schedulerMaxBackoffMillis);
Review Comment:
since `createJobDetailForReminderEvent` is not `static`, let's access `this.schedulerMaxBackoffMillis` behind-the-scenes (rather than passing as an arg)
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/FlowTriggerHandler.java:
##########
@@ -181,36 +179,43 @@ private void scheduleReminderForEvent(Properties jobProps, MultiActiveLeaseArbit
this.jobDoesNotExistInSchedulerCount.inc();
return;
}
- JobKey reminderJobKey = constructReminderJobKey(origJobKey, status.getEventTimeMillis());
- JobDetailImpl jobDetail = createJobDetailForReminderEvent(origJobKey, reminderJobKey, cronExpression,
- status.getEventTimeMillis(), originalEventTimeMillis);
- // Create a new trigger that is set to fire at the minimum reminder wait time calculated
+ JobKey reminderJobKey = constructReminderJobKey(origJobKey, status);
+ JobDetailImpl jobDetail = createJobDetailForReminderEvent(origJobKey, reminderJobKey, status,
+ triggerEventTimeMillis, schedulerMaxBackoffMillis);
+ // Create a new trigger with a `reminder` suffix that is set to fire at the minimum reminder wait time calculated
Trigger reminderTrigger = JobScheduler.createTriggerForJob(reminderJobKey,
- (Properties) jobDetail.getJobDataMap().get(GobblinServiceJobScheduler.PROPERTIES_KEY), Optional.absent());
+ getJobPropertiesFromJobDetail(jobDetail), Optional.of(createSuffixForJobTrigger(status)));
// TODO: remove this comment once we've confirmed this function works
- log.info("Flow Trigger Handler - [{}, eventTimestamp: {}] - attempting to schedule reminder for event {}",
- flowAction, originalEventTimeMillis, status.getEventTimeMillis());
+ log.info("Flow Trigger Handler - [{}, eventTimestamp: {}] - attempting to schedule reminder for event {} with"
+ + "reminderJobKey {} and reminderTriggerKey {}", flowAction, triggerEventTimeMillis,
+ status.getEventTimeMillis(), reminderJobKey, reminderTrigger.getKey());
this.schedulerService.getScheduler().scheduleJob(jobDetail, reminderTrigger);
Review Comment:
any reason not to continue "all the way" and combine these four closely-related cascading invocations that are merely sub-steps toward accomplishing the overall goal?
```
protected Trigger createAndScheduleReminder(
JobKey origJobKey,
MultiActiveLeaseArbiter.LeasedToAnotherStatus status,
long triggerEventTimeMillis
) {
String reminderSuffix = createSuffixForJobTrigger(status);
JobKey reminderJobKey = new JobKey(origJobKey.getName() + reminderSuffix + origJobKey.getGroup());
JobDetailImpl jobDetail = createJobDetailForReminderEvent(origJobKey, reminderJobKey, status, triggerEventTimeMillis, this.schedulerMaxBackoffMillis);
Trigger reminderTrigger = JobScheduler.createTriggerForJob(reminderJobKey,
getJobPropertiesFromJobDetail(jobDetail),
Optional.of(reminderSuffix));
// TODO: remove this comment once we've confirmed this function works
log.info("Flow Trigger Handler - [{}, eventTimestamp: {}] - attempting to schedule reminder for event {} with"
+ "reminderJobKey {} and reminderTriggerKey {}", flowAction, triggerEventTimeMillis,
status.getEventTimeMillis(), reminderJobKey, reminderTrigger.getKey());
this.schedulerService.getScheduler().scheduleJob(jobDetail, reminderTrigger);
return reminderTrigger;
}
```
(return `Trigger` for logging)
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] umustafi commented on a diff in pull request #3749: [GOBBLIN-1887] Increase Encapsulation in FlowTriggerHandler helper functions and Handle Reminder Events Properly
Posted by "umustafi (via GitHub)" <gi...@apache.org>.
umustafi commented on code in PR #3749:
URL: https://github.com/apache/gobblin/pull/3749#discussion_r1306065616
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -725,12 +725,24 @@ public void executeImpl(JobExecutionContext context) throws JobExecutionExceptio
// Obtain trigger timestamp from trigger to pass to jobProps
Trigger trigger = context.getTrigger();
- // THIS current event has already fired if this method is called, so it now exists in <previousFireTime>
- long triggerTimestampMillis = asUTCEpochMillis(trigger.getPreviousFireTime());
- jobProps.setProperty(ConfigurationKeys.SCHEDULER_EVENT_TO_TRIGGER_TIMESTAMP_MILLIS_KEY,
- String.valueOf(triggerTimestampMillis));
- _log.info(jobSchedulerTracePrefixBuilder(jobProps) + "triggerTime: {} nextTriggerTime: {} - Job triggered by "
- + "scheduler", triggerTimestampMillis, asUTCEpochMillis(trigger.getNextFireTime()));
+ long triggerTimestampMillis;
+ // 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"));
Review Comment:
I'll put a comment in `ConfigurationKeys` where they are defined because they are also referenced in `FlowTriggerHandler` and it's confusing there too I know.
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] umustafi commented on a diff in pull request #3749: [GOBBLIN-1887] Increase Encapsulation in FlowTriggerHandler helper functions and Handle Reminder Events Properly
Posted by "umustafi (via GitHub)" <gi...@apache.org>.
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org