autumnust commented on a change in pull request #3250:
URL: https://github.com/apache/gobblin/pull/3250#discussion_r598020976
##########
File path:
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/GobblinMultiTaskAttempt.java
##########
@@ -384,10 +394,14 @@ private boolean taskSuccessfulInPriorAttempt(String
taskId) {
* @return a list of {@link Task}s from the {@link WorkUnit}s, as well as if
there's a failure in task creation
* which should be handled separately to avoid silently starving on certain
workunit.
*/
- private Pair<List<Task>, Boolean> runWorkUnits(CountUpAndDownLatch
countDownLatch) {
-
+ private synchronized Pair<List<Task>, Boolean>
runWorkUnits(CountUpAndDownLatch countDownLatch) {
List<Task> tasks = Lists.newArrayList();
-
+ //Has the task-attempt already been cancelled? This can happen for
instance when a cancellation has been invoked on
+ // the GobblinMultiTaskAttempt instance (e.g. in the case of Helix task
cancellation) before the Gobblin tasks
+ // have been submitted to the underlying task executor.
+ if (this.stopped.get()) {
Review comment:
It doesn't seem to be enough for synchronization purpose. cancel could
happen after this line and between line 407 and 408.
--
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]
