[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks

[GitBox <gi...@apache.org>]

jon-wei commented on a change in pull request #8236: Add TaskResourceCleaner; fix a couple of concurrency bugs in batch tasks
URL: https://github.com/apache/incubator-druid/pull/8236#discussion_r310367559
 
 

 ##########
 File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTask.java
 ##########
 @@ -935,7 +900,7 @@ private TaskStatus generateAndPublishSegments(
         final BatchAppenderatorDriver driver = newDriver(appenderator, toolbox, segmentAllocator);
         final Firehose firehose = firehoseFactory.connect(dataSchema.getParser(), firehoseTempDir)
     ) {
-      this.appenderator = appenderator;
+      registerResourceCloserOnAbnormalExit(config -> appenderator.closeNow());
 
 Review comment:
   hm, I just saw that Appenderator has the following javadoc:
   
   ```
   Concurrency: all methods defined in this class directly, including {@link #close()} and {@link #closeNow()}, i. e.
    * all methods of the data appending and indexing lifecycle except {@link #drop} must be called from a single thread.
   ```
   
   Maybe we should move the `appenderator.closeNow()` calls to an interrupt handler block in the main runTask methods and let interrupts through the resource closer trigger that.
   
   Some of the appenderators are created in try-with, but AppenderatorImpl close() checks if it's already closed, so calling close() after closeNow() should be fine.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org