-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15873/#review35625
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java
<https://reviews.apache.org/r/15873/#comment66425>

    should check shutdown thrown a HiveException that results in a message like 
 "FAILED: Operation cancelled" ?
    It seems very possible that launching() is called after cancellation. I 
think better to give the more informative error message.
    



ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java
<https://reviews.apache.org/r/15873/#comment66438>

    When pollFinished is running, this shutdown() function will not be able to 
make progress. Which means that the query cancellation will happen only after a 
task (could be an MR task) is complete.
    
    It seems synchronizing around shutdown should be sufficient, either by 
making it volatile or having synchronized methods around it.
    
    Since thread safe concurrent collection classes are being used here, I 
don't see other concurrency issues that would make it necessary to make all 
these functions synchronized. 
    
    
    



ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java
<https://reviews.apache.org/r/15873/#comment66260>

    This second addToRunnable(tsk) is redundant.
    


- Thejas Nair


On Feb. 27, 2014, 7:06 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2014, 7:06 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 6705ec4 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>

Reply via email to