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



Some nits, did not checked all the unit tests changes. Anything I have to check 
particularly?


common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 2727-2730 (original), 2727-2730 (patched)
<https://reviews.apache.org/r/72387/#comment308732>

    Is this config still needed in this case?



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 2732 (patched)
<https://reviews.apache.org/r/72387/#comment308730>

    Maybe rephrase?
    Manages concurrency levels for ACID resoruces. Enables users enable 
parallel queries by enabling write-write conflict resolution happen only at 
commit phase 
    - If true - no commit phase conflict resolution: 
       - INSERT OVERWRITE requests EXCLUSIVE locks
       - UPDATE/DELETE requests EXCL_WRITE lock
       - INSERT requests SHARED_READ lock
    - If false - write might fail when committed on conflict check: 
       - INSERT OVERWRITE requests EXCL_WRITE locks
       - UPDATE/DELETE/INSERT requests SHARED_READ lock



ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
Line 3027 (original), 3027-3028 (patched)
<https://reviews.apache.org/r/72387/#comment308731>

    Maybe get once, and store?



ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
Lines 758 (patched)
<https://reviews.apache.org/r/72387/#comment308733>

    nit: remove spaces



ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java
Line 1142 (original), 1265 (patched)
<https://reviews.apache.org/r/72387/#comment308734>

    nit: space too


- Peter Vary


On ápr. 20, 2020, 7:23 de, Denys Kuzmenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72387/
> -----------------------------------------------------------
> 
> (Updated ápr. 20, 2020, 7:23 de)
> 
> 
> Review request for hive, Marton Bod and Peter Vary.
> 
> 
> Bugs: HIVE-19369
>     https://issues.apache.org/jira/browse/HIVE-19369
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Hive Locking with Micro-managed and full-ACID tables needs a better locking 
> implementation which allows for no-wait readers always.
> 
> EXCL_DROP
> EXCL_WRITE
> SHARED_WRITE
> SHARED_READ
> 
> Short write-up
> 
> EXCL_DROP is a "drop partition" or "drop table" and waits for all others to 
> exit
> EXCL_WRITE excludes all writes and will wait for all existing SHARED_WRITE to 
> exit.
> SHARED_WRITE allows all SHARED_WRITES to go through, but will wait for an 
> EXCL_WRITE & EXCL_DROP (waiting so that you can do drop + insert in different 
> threads).
> 
> SHARED_READ does not wait for any lock - it fails fast for a pending 
> EXCL_DROP, because even if there is an EXCL_WRITE or SHARED_WRITE pending, 
> there's no semantic reason to wait for them to succeed before going ahead 
> with a SHARED_WRITE.
> 
> a select * => SHARED_READ
> an insert into => SHARED_WRITE
> an insert overwrite or MERGE => EXCL_WRITE
> a drop table => EXCL_DROP
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7b3acad511 
>   ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 77878ca40b 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   ql/src/test/queries/clientpositive/explain_locks.q 3c11560c5f 
>   ql/src/test/results/clientpositive/explain_locks.q.out 3183533478 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/LockRequestBuilder.java
>  22902a9c20 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/LockTypeComparator.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  331fd4cc8d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/LockTypeUtil.java
>  f928bf781b 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/LockTypeUtilTest.java
>  3d69a6e7dc 
> 
> 
> Diff: https://reviews.apache.org/r/72387/diff/1/
> 
> 
> Testing
> -------
> 
> Added number of tests under TestDbTxnManager2, TestTxnHandler & extended 
> explain_locks.q test.
> 
> 
> Thanks,
> 
> Denys Kuzmenko
> 
>

Reply via email to