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




ql/src/java/org/apache/hadoop/hive/ql/Driver.java (line 200)
<https://reviews.apache.org/r/55479/#comment232765>

    I wonder if it might look cleaner if we have an inner class called 
DriverState similar to this LockedDriverState. But all driver state related 
stuffs such as enum, lock are encapsulated in this class. It provides the 
methods for state transition etc.



ql/src/java/org/apache/hadoop/hive/ql/Driver.java (line 1129)
<https://reviews.apache.org/r/55479/#comment232766>

    nit: need a space between userFromUGI,lDrvState



ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java (line 454)
<https://reviews.apache.org/r/55479/#comment232767>

    should be "Query was cancelled while acquiring locks on the underlying 
objects."?



ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java (line 184)
<https://reviews.apache.org/r/55479/#comment232772>

    Race condition here:
    if hiveLocks == null was caused by the interruption, but when the code 
executes this step, the state was just changed to be interrupted, then the 
exception msg will not be right.



ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java
 (line 189)
<https://reviews.apache.org/r/55479/#comment232768>

    as I commented before, the DriverState class might provide this method for 
inspecting this state, which looks better.


- Chaoyu Tang


On Jan. 12, 2017, 11:21 p.m., Yongzhi Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55479/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2017, 11:21 p.m.)
> 
> 
> Review request for hive, Aihua Xu and Chaoyu Tang.
> 
> 
> Bugs: HIVE-15572
>     https://issues.apache.org/jira/browse/HIVE-15572
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> 1. Add data structure to pass driverstate
> 2. Driver state check when acquire locks by zookeeper.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> fd6020b85591ea190aa33ae9f2dc925a38fc7471 
>   ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java 
> 721974db03f1f29bdb84f41db317e37a6a78ca32 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbLockManager.java 
> 45ead16560ce7514a1ab6f4ac2de6771582a8a73 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java 
> 24fbd9af5fb7be6b238c6ed246e360477d3c47de 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/EmbeddedLockManager.java 
> 20e114776f143715d5820e6a1acb794a9d6de02c 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockManager.java 
> b2eb99775c220e9ce347fa1cb918ebf4e738eac2 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 
> ce220a21de01a188da940e4511ee6876d0c15a4a 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java 
> ed022d9193f14436ed527f9cbd3df45d48857cf4 
>   
> ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java
>  14d0ef4e27e0518c1bafcbdcde12f09e101a3321 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDummyTxnManager.java 
> e189d383b6d090ce151b6ab30fb240c261430239 
> 
> Diff: https://reviews.apache.org/r/55479/diff/
> 
> 
> Testing
> -------
> 
> Unit test
> Manual test
> 
> 
> Thanks,
> 
> Yongzhi Chen
> 
>

Reply via email to