[
https://issues.apache.org/jira/browse/GOBBLIN-1846?focusedWorklogId=866839&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-866839
]
ASF GitHub Bot logged work on GOBBLIN-1846:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 21/Jun/23 20:07
Start Date: 21/Jun/23 20:07
Worklog Time Spent: 10m
Work Description: phet commented on code in PR #3707:
URL: https://github.com/apache/gobblin/pull/3707#discussion_r1237605433
##########
gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java:
##########
@@ -407,6 +407,10 @@ public void scheduleJob(Properties jobProps, JobListener
jobListener, Map<String
this.scheduledJobs.put(jobName, job.getKey());
}
+ public void scheduleJobLogUtil(JobDetail job, Trigger trigger) {
Review Comment:
firstly, does this need to be `public`? I'd expect `protected`...
secondly, if the name here were read as a noun (about a util), it would be
appropriate to a class. a method should be named by a verb. reading it that
way, it suggests to do scheduling of a "JobLogUtil".
how about `logJobSchedulingTrace` or `logJobSchedulingInfo`?
or since it's so specific, `logNewlyScheduledJob`?
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -442,6 +446,14 @@ public synchronized void scheduleJob(Properties jobProps,
JobListener jobListene
}
}
+ @Override
+ public void scheduleJobLogUtil(JobDetail job, Trigger trigger) {
+ Properties jobProps = (Properties)
job.getJobDataMap().get(JobScheduler.JOB_SCHEDULER_KEY);
+ log.info("Scheduler trigger tracing: [flowName: {} flowGroup: {}] -
nextTriggerTime: {} - Job newly scheduled",
+ jobProps.getProperty(ConfigurationKeys.FLOW_NAME_KEY, ""),
Review Comment:
let's fallback to something clearer for those reading the logs. e.g. `"<<no
group>>"` or `"<<none>>"`
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -666,13 +687,24 @@ public static class GobblinServiceJob extends
BaseGobblinJob implements Interrup
@Override
public void executeImpl(JobExecutionContext context) throws
JobExecutionException {
- _log.info("Starting FlowSpec " + context.getJobDetail().getKey());
+ JobDetail jobDetail = context.getJobDetail();
+ _log.info("Starting FlowSpec " + jobDetail.getKey());
- JobDataMap dataMap = context.getJobDetail().getJobDataMap();
+ JobDataMap dataMap = jobDetail.getJobDataMap();
JobScheduler jobScheduler = (JobScheduler)
dataMap.get(JOB_SCHEDULER_KEY);
Properties jobProps = (Properties) dataMap.get(PROPERTIES_KEY);
JobListener jobListener = (JobListener) dataMap.get(JOB_LISTENER_KEY);
+ // 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 = trigger.getPreviousFireTime().getTime();
+
jobProps.setProperty(ConfigurationKeys.SCHEDULER_EVENT_TO_TRIGGER_TIMESTAMP_MILLIS_KEY,
+ String.valueOf(triggerTimestampMillis));
+ _log.info("Scheduler trigger tracing: [flowName: {} flowGroup: {}] -
triggerTime: {} nextTriggerTime: {} - "
+ + "Job triggered by scheduler",
+ jobProps.getProperty(ConfigurationKeys.JOB_NAME_KEY),
jobProps.getProperty(ConfigurationKeys.JOB_GROUP_KEY),
+ triggerTimestampMillis, trigger.getNextFireTime().getTime());
Review Comment:
somewhat surprised to see you work again from first principles rather than
leverage the new method abstraction you just added as `scheduleJobLogUtil`...
but not a big deal (I see how the method would need to be more general for
it to work here as well)
Issue Time Tracking
-------------------
Worklog Id: (was: 866839)
Time Spent: 1h 10m (was: 1h)
> Add logs to validate scheduled job triggers
> -------------------------------------------
>
> Key: GOBBLIN-1846
> URL: https://issues.apache.org/jira/browse/GOBBLIN-1846
> Project: Apache Gobblin
> Issue Type: Bug
> Components: gobblin-service
> Reporter: Urmi Mustafi
> Assignee: Abhishek Tiwari
> Priority: Major
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> Add logging in 3 critical places: newly scheduled jobs (or updated ones),
> each trigger of a new job, and when jobs are unscheduled so we can track
> every expected trigger of a scheduled job occurs. We will use the three types
> of logs mentioned to ensure there are no missed scheduling events due to host
> downtime with a simple analysis of the logs.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
