ZihanLi58 commented on code in PR #3737:
URL: https://github.com/apache/gobblin/pull/3737#discussion_r1294061342
##########
gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java:
##########
@@ -586,9 +587,13 @@ public void close() throws IOException {
* Get a {@link org.quartz.Trigger} from the given job configuration
properties.
*/
public static Trigger createTriggerForJob(JobKey jobKey, Properties
jobProps) {
- // Build a trigger for the job with the given cron-style schedule
+ Random random = new Random();
+ /*
+ 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) +
random.nextInt(1000),
Review Comment:
Random number will make it super hard to debug if something went wrong..
does it make sense to change the trigger name to be normal flow name -> normal
trigger that tigger the execution; {flowName + “reminder job” +
executionid/timestamp} -> reminder job for a specific execution?
##########
gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java:
##########
@@ -586,9 +587,13 @@ public void close() throws IOException {
* Get a {@link org.quartz.Trigger} from the given job configuration
properties.
*/
public static Trigger createTriggerForJob(JobKey jobKey, Properties
jobProps) {
- // Build a trigger for the job with the given cron-style schedule
+ Random random = new Random();
+ /*
+ 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) +
random.nextInt(1000),
Review Comment:
Random number will make it super hard to debug if something went wrong..
does it make sense to change the trigger name to be normal flow name -> normal
trigger that tigger the execution; {flowName + “reminder job” +
executionid/timestamp} -> reminder job for a specific execution?
--
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]
