leventov 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_r313083513
 
 

 ##########
 File path: 
indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java
 ##########
 @@ -164,7 +164,10 @@ default int getPriority()
    * Asks a task to arrange for its "run" method to exit promptly. Tasks that 
take too long to stop gracefully will be
    * terminated with extreme prejudice.
    *
-   * This method can be called at any time while {@link #run} is being called 
when the task is killed.
+   * This method can be called at any time while {@link #run} is being called 
when the task is killed. If this task
+   * is not started yet, that is {@link #run} is not called yet, this method 
will be never called.
 
 Review comment:
   > I think the contract now should say "this method can be called at any time 
no matter when run() is executed".
   
   I agree.
   
   > I don't know what it should be to address [your 
comment](https://github.com/apache/incubator-druid/pull/8236#discussion_r312195277).
 What is your suggestion?
   
   I would just put the phrase "Regardless when `stopGracefully()` is called w. 
r. t. `run()`, the implementation must not allow a resource leak or lingering 
executions (local or remote)."

----------------------------------------------------------------
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]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to