phet commented on code in PR #3779:
URL: https://github.com/apache/gobblin/pull/3779#discussion_r1330654344
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -466,9 +466,8 @@ public synchronized void scheduleJob(Properties jobProps,
JobListener jobListene
@Override
protected void logNewlyScheduledJob(JobDetail job, Trigger trigger) {
Properties jobProps = (Properties) job.getJobDataMap().get(PROPERTIES_KEY);
- log.info(jobSchedulerTracePrefixBuilder(jobProps) + "nextTriggerTime: {}
localTZNextTriggerTime:{} - Job newly "
- + "scheduled", utcDateAsUTCEpochMillis(trigger.getNextFireTime()),
- systemDefaultZoneDateAsUTCEpochMillis(trigger.getNextFireTime()));
+ log.info(jobSchedulerTracePrefixBuilder(jobProps) + "nextTriggerTime (in
UTC): {} - Job newly "
Review Comment:
rather than sprinkling " (in UTC)" so many places, what if we modified
`jobSchedulerTracePrefixBuilder` so it read something like:
```
"Scheduler trigger tracing (in epoch-ms UTC): [flowName:..."
```
?
(so just once in the log line's prefix)
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/FlowTriggerHandler.java:
##########
@@ -211,8 +211,7 @@ protected Trigger createAndScheduleReminder(JobKey
origJobKey, MultiActiveLeaseA
triggerEventTimeMillis);
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"
+ log.debug("Flow Trigger Handler - [{}, eventTimestamp: {}] - attempting
to schedule reminder for event {} with"
Review Comment:
looks like we'd want a space after "with"
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -754,25 +753,24 @@ public void executeImpl(JobExecutionContext context)
throws JobExecutionExceptio
Trigger trigger = context.getTrigger();
// THIS current event has already fired if this method is called, so
it now exists in <previousFireTime>
long triggerTimeMillis =
utcDateAsUTCEpochMillis(trigger.getPreviousFireTime());
- long localTZTriggerTimeMillis =
systemDefaultZoneDateAsUTCEpochMillis(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")) {
String preservedConsensusEventTime = jobProps.getProperty(
ConfigurationKeys.SCHEDULER_PRESERVED_CONSENSUS_EVENT_TIME_MILLIS_KEY, "0");
String expectedReminderTime = 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);
+ _log.info(jobSchedulerTracePrefixBuilder(jobProps) + "triggerTime
(in UTC): {} expectedReminderTime (in "
+ + "UTC): {} - Reminder job triggered by scheduler at {}",
preservedConsensusEventTime,
+ expectedReminderTime, triggerTimeMillis);
// TODO: add a metric if expected reminder time far exceeds system
time
jobProps.setProperty(ConfigurationKeys.ORCHESTRATOR_TRIGGER_EVENT_TIME_MILLIS_KEY,
preservedConsensusEventTime);
} else {
jobProps.setProperty(ConfigurationKeys.ORCHESTRATOR_TRIGGER_EVENT_TIME_MILLIS_KEY,
String.valueOf(triggerTimeMillis));
- _log.info(jobSchedulerTracePrefixBuilder(jobProps) + "triggerTime:
{} nextTriggerTime: {} "
- + "localTZTriggerTime: {} localTZNextTriggerTime: {}- Job
triggered by scheduler", triggerTimeMillis,
- utcDateAsUTCEpochMillis(trigger.getNextFireTime()),
localTZTriggerTimeMillis,
-
systemDefaultZoneDateAsUTCEpochMillis(trigger.getNextFireTime()));
+ _log.info(jobSchedulerTracePrefixBuilder(jobProps) + "triggerTime
(in UTC): {} nextTriggerTime (in UTC): {} -"
Review Comment:
add a space before '-'?
--
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]
