ZihanLi58 commented on code in PR #3677:
URL: https://github.com/apache/gobblin/pull/3677#discussion_r1164730248
##########
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixJobLauncher.java:
##########
@@ -447,11 +455,11 @@ public void launchJob(@Nullable JobListener jobListener)
throws JobException {
if (this.runningMap.replace(this.jobContext.getJobName(), false, true)) {
LOGGER.info("Job {} will be executed, add into running map.",
this.jobContext.getJobId());
isLaunched = true;
- super.launchJob(jobListener);
+ launchJobImpl(jobListener);
} else {
LOGGER.warn("Job {} will not be executed because other jobs are still
running.", this.jobContext.getJobId());
}
- // TODO: Better error handling
+ // TODO: Better error handling. The current impl swallows exceptions for
jobs that were started by this method call
} catch (Throwable t) {
Review Comment:
Add todo here to make the behavior of swallowing exception configurable
##########
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/HelixUtils.java:
##########
@@ -112,22 +117,11 @@ public static String getHelixInstanceName(
return namePrefix + "_" + instanceId;
}
- // We have switched from Helix JobQueue to WorkFlow based job execution.
- @Deprecated
- public static void submitJobToQueue(
- JobConfig.Builder jobConfigBuilder,
- String queueName,
- String jobName,
- TaskDriver helixTaskDriver,
- HelixManager helixManager,
- long jobQueueDeleteTimeoutSeconds) throws Exception {
- submitJobToWorkFlow(jobConfigBuilder, queueName, jobName, helixTaskDriver,
helixManager, jobQueueDeleteTimeoutSeconds);
- }
-
static void waitJobInitialization(
HelixManager helixManager,
String workFlowName,
- String jobName) throws Exception {
+ String jobName,
+ Duration timeout) throws Exception {
Review Comment:
Just curious here why do we choose to use duration here? Why not use long
directly to represent timeout mills?
--
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]
