abhishekmjain commented on code in PR #4097:
URL: https://github.com/apache/gobblin/pull/4097#discussion_r1955674239
##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/joblauncher/GobblinJobLauncher.java:
##########
@@ -137,7 +137,9 @@ public GobblinJobLauncher(Properties jobProps, Path
appWorkDir,
@Override
public void close() throws IOException {
try {
- executeCancellation();
+ cancelJob(jobListener);
+ } catch (Exception e) {
+ log.error("Exception occurred while cancelling job", e);
} finally {
try {
cleanupWorkingDirectory();
Review Comment:
Even with `executeCancellation` full job gets cancelled. `cancelJob` is just
a synchronized way to cancel - cancelJob waits on `cancellationRequest` whereas
`executeCancellation` directly calls cancel. `executeCancellation` may execute
in two threads parallelly if we don't change this.
`MRJobLauncher` is also calling `killJob` directly which is not something
which should be followed. (I believe this might have been translated to calling
`executeCancellation` directly in Temporal when the code was first written)
--
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: dev-unsubscr...@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
