This is an automated email from the ASF dual-hosted git repository. dkuzmenko pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push: new 83b4a0887cc HIVE-26165: Remove READ locks for ACID tables (Denys Kuzmenko, reviewed by Karen Coppage) 83b4a0887cc is described below commit 83b4a0887cc1c081af64f353d3e66d3d977c861d Author: Denys Kuzmenko <dkuzme...@cloudera.com> AuthorDate: Mon Jun 13 09:14:09 2022 +0200 HIVE-26165: Remove READ locks for ACID tables (Denys Kuzmenko, reviewed by Karen Coppage) Closes #3235 --- .../org/apache/hadoop/hive/ql/io/AcidUtils.java | 60 ++++++++++------------ .../hadoop/hive/ql/lockmgr/TestDbTxnManager2.java | 54 +++++++++++-------- 2 files changed, 60 insertions(+), 54 deletions(-) diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java index 45684df17e6..5ade4dabb80 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java @@ -2846,36 +2846,32 @@ public class AcidUtils { } private static boolean needsLock(Entity entity, boolean isExternalEnabled) { - switch (entity.getType()) { - case TABLE: - return isLockableTable(entity.getTable(), isExternalEnabled); - case PARTITION: - return isLockableTable(entity.getPartition().getTable(), isExternalEnabled); - default: - return true; - } + return needsLock(entity, isExternalEnabled, false); } - private static Table getTable(WriteEntity we) { - Table t = we.getTable(); - if (t == null) { - throw new IllegalStateException("No table info for " + we); + private static boolean needsLock(Entity entity, boolean isExternalEnabled, boolean isLocklessReads) { + switch (entity.getType()) { + case TABLE: + return isLockableTable(entity.getTable(), isExternalEnabled, isLocklessReads); + case PARTITION: + return isLockableTable(entity.getPartition().getTable(), isExternalEnabled, isLocklessReads); + default: + return true; } - return t; } - private static boolean isLockableTable(Table t, boolean isExternalEnabled) { + private static boolean isLockableTable(Table t, boolean isExternalEnabled, boolean isLocklessReads) { if (t.isTemporary()) { return false; } switch (t.getTableType()) { - case MANAGED_TABLE: - case MATERIALIZED_VIEW: - return true; - case EXTERNAL_TABLE: - return isExternalEnabled; - default: - return false; + case MANAGED_TABLE: + case MATERIALIZED_VIEW: + return !(isLocklessReads && isTransactionalTable(t)); + case EXTERNAL_TABLE: + return isExternalEnabled; + default: + return false; } } @@ -2890,8 +2886,10 @@ public class AcidUtils { Context.Operation operation, HiveConf conf) { List<LockComponent> lockComponents = new ArrayList<>(); + boolean isLocklessReadsEnabled = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED); boolean skipReadLock = !conf.getBoolVar(ConfVars.HIVE_TXN_READ_LOCKS); boolean skipNonAcidReadLock = !conf.getBoolVar(ConfVars.HIVE_TXN_NONACID_READ_LOCKS); + boolean sharedWrite = !conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK); boolean isExternalEnabled = conf.getBoolVar(HiveConf.ConfVars.HIVE_TXN_EXT_LOCKING_ENABLED); boolean isMerge = operation == Context.Operation.MERGE; @@ -2902,7 +2900,7 @@ public class AcidUtils { .filter(input -> !input.isDummy() && input.needsLock() && !input.isUpdateOrDelete() - && AcidUtils.needsLock(input, isExternalEnabled) + && AcidUtils.needsLock(input, isExternalEnabled, isLocklessReadsEnabled) && !skipReadLock) .collect(Collectors.toList()); @@ -2961,9 +2959,8 @@ public class AcidUtils { // overwrite) than we need a shared. If it's update or delete then we // need a SHARED_WRITE. for (WriteEntity output : outputs) { - LOG.debug("output is null " + (output == null)); - if (output.getType() == Entity.Type.DFS_DIR || output.getType() == Entity.Type.LOCAL_DIR || !AcidUtils - .needsLock(output, isExternalEnabled)) { + if (output.getType() == Entity.Type.DFS_DIR || output.getType() == Entity.Type.LOCAL_DIR + || !AcidUtils.needsLock(output, isExternalEnabled)) { // We don't lock files or directories. We also skip locking temp tables. continue; } @@ -3015,7 +3012,8 @@ public class AcidUtils { case INSERT_OVERWRITE: assert t != null; if (AcidUtils.isTransactionalTable(t)) { - if (conf.getBoolVar(HiveConf.ConfVars.TXN_OVERWRITE_X_LOCK) && !sharedWrite) { + if (conf.getBoolVar(HiveConf.ConfVars.TXN_OVERWRITE_X_LOCK) && !sharedWrite + && !isLocklessReadsEnabled) { compBuilder.setExclusive(); } else { compBuilder.setExclWrite(); @@ -3030,18 +3028,16 @@ public class AcidUtils { assert t != null; if (AcidUtils.isTransactionalTable(t)) { boolean isExclMergeInsert = conf.getBoolVar(ConfVars.TXN_MERGE_INSERT_X_LOCK) && isMerge; + compBuilder.setSharedRead(); if (sharedWrite) { compBuilder.setSharedWrite(); } else { if (isExclMergeInsert) { compBuilder.setExclWrite(); - } else { - if (AcidUtils.isLocklessReadsEnabled(t, conf)) { - compBuilder.setSharedWrite(); - } else { - compBuilder.setSharedRead(); - } + + } else if (isLocklessReadsEnabled) { + compBuilder.setSharedWrite(); } } if (isExclMergeInsert) { diff --git a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java index a076dec9b7d..51408ef681a 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java @@ -3591,10 +3591,12 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ driver.lockAndRespond(); List<ShowLocksResponseElement> locks = getLocks(); - Assert.assertEquals("Unexpected lock count", 1, locks.size()); + Assert.assertEquals("Unexpected lock count", blocking ? 1 : 0, locks.size()); - checkLock(LockType.SHARED_READ, - LockState.ACQUIRED, "default", "tab_acid", null, locks); + if (blocking) { + checkLock(LockType.SHARED_READ, + LockState.ACQUIRED, "default", "tab_acid", null, locks); + } DbTxnManager txnMgr2 = (DbTxnManager) TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf); swapTxnManager(txnMgr2); @@ -3619,7 +3621,7 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ } driver2.lockAndRespond(); locks = getLocks(); - Assert.assertEquals("Unexpected lock count", blocking ? 1 : 2, locks.size()); + Assert.assertEquals("Unexpected lock count", 1, locks.size()); checkLock(blocking ? LockType.EXCLUSIVE : LockType.EXCL_WRITE, LockState.ACQUIRED, "default", "tab_acid", null, locks); @@ -3722,10 +3724,12 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ driver.lockAndRespond(); List<ShowLocksResponseElement> locks = getLocks(); - Assert.assertEquals("Unexpected lock count", 1, locks.size()); + Assert.assertEquals("Unexpected lock count", blocking ? 1 : 0, locks.size()); - checkLock(LockType.SHARED_READ, - LockState.ACQUIRED, "default", "tab_acid", null, locks); + if (blocking) { + checkLock(LockType.SHARED_READ, + LockState.ACQUIRED, "default", "tab_acid", null, locks); + } DbTxnManager txnMgr2 = (DbTxnManager) TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf); swapTxnManager(txnMgr2); @@ -3750,7 +3754,7 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ } driver2.lockAndRespond(); locks = getLocks(); - Assert.assertEquals("Unexpected lock count", blocking ? 1 : 2, locks.size()); + Assert.assertEquals("Unexpected lock count", 1, locks.size()); checkLock(blocking ? LockType.EXCLUSIVE : LockType.EXCL_WRITE, LockState.ACQUIRED, "default", "tab_acid", null, locks); @@ -3859,12 +3863,14 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ driver.lockAndRespond(); List<ShowLocksResponseElement> locks = getLocks(); - Assert.assertEquals("Unexpected lock count", 2, locks.size()); + Assert.assertEquals("Unexpected lock count", blocking ? 2 : 0, locks.size()); - checkLock(LockType.SHARED_READ, - LockState.ACQUIRED, "default", "tab_acid", null, locks); - checkLock(LockType.SHARED_READ, - LockState.ACQUIRED, "default", "mv_tab_acid", null, locks); + if (blocking) { + checkLock(LockType.SHARED_READ, + LockState.ACQUIRED, "default", "tab_acid", null, locks); + checkLock(LockType.SHARED_READ, + LockState.ACQUIRED, "default", "mv_tab_acid", null, locks); + } DbTxnManager txnMgr2 = (DbTxnManager) TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf); swapTxnManager(txnMgr2); @@ -3889,7 +3895,7 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ } driver2.lockAndRespond(); locks = getLocks(); - Assert.assertEquals("Unexpected lock count", blocking ? 1 : 3, locks.size()); + Assert.assertEquals("Unexpected lock count", 1, locks.size()); checkLock(blocking ? LockType.EXCLUSIVE : LockType.EXCL_WRITE, LockState.ACQUIRED, "default", "mv_tab_acid", null, locks); @@ -4223,10 +4229,12 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ driver.lockAndRespond(); List<ShowLocksResponseElement> locks = getLocks(); - Assert.assertEquals("Unexpected lock count", 1, locks.size()); + Assert.assertEquals("Unexpected lock count", blocking ? 1 : 0, locks.size()); - checkLock(LockType.SHARED_READ, - LockState.ACQUIRED, "default", "tab_acid", null, locks); + if (blocking) { + checkLock(LockType.SHARED_READ, + LockState.ACQUIRED, "default", "tab_acid", null, locks); + } DbTxnManager txnMgr2 = (DbTxnManager) TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf); swapTxnManager(txnMgr2); @@ -4251,7 +4259,7 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ } driver2.lockAndRespond(); locks = getLocks(); - Assert.assertEquals("Unexpected lock count", blocking ? 1 : 2, locks.size()); + Assert.assertEquals("Unexpected lock count", 1, locks.size()); checkLock(blocking ? LockType.EXCLUSIVE : LockType.EXCL_WRITE, LockState.ACQUIRED, "default", "tab_acid", null, locks); @@ -4318,10 +4326,12 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ driver.lockAndRespond(); List<ShowLocksResponseElement> locks = getLocks(); - Assert.assertEquals("Unexpected lock count", 1, locks.size()); + Assert.assertEquals("Unexpected lock count", blocking ? 1 : 0, locks.size()); - checkLock(LockType.SHARED_READ, - LockState.ACQUIRED, "default", "tab_acid", null, locks); + if (blocking) { + checkLock(LockType.SHARED_READ, + LockState.ACQUIRED, "default", "tab_acid", null, locks); + } DbTxnManager txnMgr2 = (DbTxnManager) TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf); swapTxnManager(txnMgr2); @@ -4346,7 +4356,7 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ } driver2.lockAndRespond(); locks = getLocks(); - Assert.assertEquals("Unexpected lock count", blocking ? 1 : 2, locks.size()); + Assert.assertEquals("Unexpected lock count", 1, locks.size()); checkLock(blocking ? LockType.EXCLUSIVE : LockType.EXCL_WRITE, LockState.ACQUIRED, "default", "tab_acid", null, locks);