phet commented on code in PR #3737:
URL: https://github.com/apache/gobblin/pull/3737#discussion_r1294078711
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/FlowTriggerHandler.java:
##########
@@ -179,18 +179,31 @@ private void scheduleReminderForEvent(Properties
jobProps, MultiActiveLeaseArbit
String.valueOf(originalEventTimeMillis));
JobKey key = new JobKey(flowAction.getFlowName(),
flowAction.getFlowGroup());
// Create a new trigger for the flow in job scheduler that is set to fire
at the minimum reminder wait time calculated
- Trigger trigger = JobScheduler.createTriggerForJob(key, jobProps);
+ Trigger trigger = JobScheduler.createTriggerForJob(key, jobProps,
+ Optional.of(createSuffixForJobTrigger(status.getEventTimeMillis())));
try {
log.info("Flow Trigger Handler - [{}, eventTimestamp: {}] - attempting
to schedule reminder for event {} in {} millis",
flowAction, originalEventTimeMillis, status.getEventTimeMillis(),
trigger.getNextFireTime());
this.schedulerService.getScheduler().scheduleJob(trigger);
} catch (SchedulerException e) {
- log.warn("Failed to add job reminder due to SchedulerException for job
{} trigger event {} ", key, status.getEventTimeMillis(), e);
+ log.warn("Failed to add job reminder due to SchedulerException for job
{} trigger event {} ", key,
+ status.getEventTimeMillis(), e);
+ // TODO: emit a metric for failed job reminders
}
log.info(String.format("Flow Trigger Handler - [%s, eventTimestamp: %s] -
SCHEDULED REMINDER for event %s in %s millis",
flowAction, originalEventTimeMillis, status.getEventTimeMillis(),
trigger.getNextFireTime()));
}
+ /**
+ * Create suffix to add to end of flow name to differentiate reminder
triggers from the original job schedule trigger
+ * and ensure they are added to the scheduler.
+ * @param eventToRevisitMillis
+ * @return
+ */
+ public static String createSuffixForJobTrigger(long eventToRevisitMillis) {
+ return "reminder_for_" + (eventToRevisitMillis);
Review Comment:
nit: why the parens?
##########
gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java:
##########
@@ -583,12 +582,17 @@ public void close() throws IOException {
}
/**
- * Get a {@link org.quartz.Trigger} from the given job configuration
properties.
+ * Get a {@link org.quartz.Trigger} from the given job configuration
properties. If triggerSuffix is provided, appends
+ * it to the end of the flow name.
*/
- public static Trigger createTriggerForJob(JobKey jobKey, Properties
jobProps) {
- // Build a trigger for the job with the given cron-style schedule
+ public static Trigger createTriggerForJob(JobKey jobKey, Properties
jobProps, Optional<String> triggerSuffix) {
+ /*
+ Build a trigger for the job with the given cron-style schedule
+ Append a random integer to job name to be able to add multiple triggers
associated with the same job.
Review Comment:
out-of-date comment
##########
gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java:
##########
@@ -583,12 +582,17 @@ public void close() throws IOException {
}
/**
- * Get a {@link org.quartz.Trigger} from the given job configuration
properties.
+ * Get a {@link org.quartz.Trigger} from the given job configuration
properties. If triggerSuffix is provided, appends
+ * it to the end of the flow name.
*/
- public static Trigger createTriggerForJob(JobKey jobKey, Properties
jobProps) {
- // Build a trigger for the job with the given cron-style schedule
+ public static Trigger createTriggerForJob(JobKey jobKey, Properties
jobProps, Optional<String> triggerSuffix) {
+ /*
+ Build a trigger for the job with the given cron-style schedule
+ Append a random integer to job name to be able to add multiple triggers
associated with the same job.
+ */
return TriggerBuilder.newTrigger()
- .withIdentity(jobProps.getProperty(ConfigurationKeys.JOB_NAME_KEY),
+ .withIdentity(jobProps.getProperty(ConfigurationKeys.JOB_NAME_KEY)
+ + (triggerSuffix.isPresent() ? "_" + triggerSuffix.get() : ""),
Review Comment:
```
triggerSuffix.transform(s -> "_" + s).or("")
```
?
--
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]
