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