sv2000 commented on a change in pull request #3092:
URL: https://github.com/apache/incubator-gobblin/pull/3092#discussion_r477621504
##########
File path:
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixTaskStateTracker.java
##########
@@ -59,7 +58,11 @@ public void registerNewTask(Task task) {
try {
this.scheduledReporters.put(task.getTaskId(),
scheduleTaskMetricsUpdater(new TaskMetricsUpdater(task), task));
} catch (RejectedExecutionException ree) {
+ // Propagate the exception to caller that has full control of the
life-cycle of a helix task.
LOGGER.error(String.format("Scheduling of task state reporter for task
%s was rejected", task.getTaskId()));
+ Throwables.propagate(ree);
Review comment:
One question worth considering: Do we want to make the failure in
scheduling TaskMetricsUpdater a fatal failure? or can it be made configurable?
##########
File path:
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/GobblinMultiTaskAttempt.java
##########
@@ -426,7 +431,7 @@ private boolean taskSuccessfulInPriorAttempt(String taskId)
{
if (task == null) {
if (e instanceof RetryException) {
// Indicating task being null due to failure in creation even
after retrying.
- isTaskCreatedSuccessfully = false;
+ areAllTaskSubmitted = false;
Review comment:
areAllTaskSubmitted -> areAllTasksSubmitted.
##########
File path:
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/GobblinMultiTaskAttempt.java
##########
@@ -385,7 +387,10 @@ private boolean taskSuccessfulInPriorAttempt(String
taskId) {
private Pair<List<Task>, Boolean> runWorkUnits(CountUpAndDownLatch
countDownLatch) {
List<Task> tasks = Lists.newArrayList();
- boolean isTaskCreatedSuccessfully = true;
+
+ // A flag indicating if there are any tasks not submitted successfully.
+ // Caller of this method should handler un-submitted task accordingly.
Review comment:
handler -> handle.
un-submitted task -> tasks with submission failures
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
