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 7a7ba31 HIVE-25011: Concurrency: Do not acquire locks for EXPLAIN and initiate implicit transaction for SHOW commands (Denys Kuzmenko, reviewed by Peter Vary, Aasha Medhi) 7a7ba31 is described below commit 7a7ba31fa4f76121497bd2d09fcd2f934cae452a Author: Denys Kuzmenko <dkuzme...@cloudera.com> AuthorDate: Wed Oct 6 08:05:14 2021 +0300 HIVE-25011: Concurrency: Do not acquire locks for EXPLAIN and initiate implicit transaction for SHOW commands (Denys Kuzmenko, reviewed by Peter Vary, Aasha Medhi) EXPLAIN UPDATE ... should not be in conflict with another active ongoing UPDATE operation. Co-authored-by: Gopal V <gop...@apache.org> Closes #2660 --- .../java/org/apache/hadoop/hive/ql/Compiler.java | 2 ++ ql/src/java/org/apache/hadoop/hive/ql/Context.java | 2 +- .../apache/hadoop/hive/ql/DriverTxnHandler.java | 11 +++++--- .../org/apache/hadoop/hive/ql/io/AcidUtils.java | 30 ++++++++-------------- .../hadoop/hive/ql/lockmgr/DbTxnManager.java | 9 ++++++- .../hadoop/hive/ql/lockmgr/HiveTxnManager.java | 3 +++ .../hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java | 6 +++++ .../hadoop/hive/ql/parse/SemanticAnalyzer.java | 18 ++++++------- .../hadoop/hive/ql/lockmgr/TestDbTxnManager2.java | 15 +++++++++++ 9 files changed, 62 insertions(+), 34 deletions(-) diff --git a/ql/src/java/org/apache/hadoop/hive/ql/Compiler.java b/ql/src/java/org/apache/hadoop/hive/ql/Compiler.java index 70bc031..8620fd0 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/Compiler.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/Compiler.java @@ -290,6 +290,8 @@ public class Compiler { */ case SHOWDATABASES: case SHOWTABLES: + case SHOW_TABLESTATUS: + case SHOW_TBLPROPERTIES: case SHOWCOLUMNS: case SHOWFUNCTIONS: case SHOWPARTITIONS: diff --git a/ql/src/java/org/apache/hadoop/hive/ql/Context.java b/ql/src/java/org/apache/hadoop/hive/ql/Context.java index 5c82cce..91330dd 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/Context.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/Context.java @@ -435,7 +435,7 @@ public class Context { /** * Find whether we should execute the current query due to explain - * @return true if the query needs to be executed, false if not + * @return true if the query skips execution, false if does execute */ public boolean isExplainSkipExecution() { return (explainConfig != null && explainConfig.getAnalyze() != AnalyzeState.RUNNING); diff --git a/ql/src/java/org/apache/hadoop/hive/ql/DriverTxnHandler.java b/ql/src/java/org/apache/hadoop/hive/ql/DriverTxnHandler.java index 44bcd20..a6d3481 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/DriverTxnHandler.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/DriverTxnHandler.java @@ -138,7 +138,7 @@ class DriverTxnHandler { void cleanupTxnList() { driverContext.getConf().unset(ValidTxnList.VALID_TXNS_KEY); } - + void acquireLocksIfNeeded() throws CommandProcessorException { if (requiresLock()) { acquireLocks(); @@ -156,6 +156,11 @@ class DriverTxnHandler { return false; } + // no execution is going to be attempted, skip acquiring locks + if (context.isExplainSkipExecution()) { + return false; + } + if (!HiveConf.getBoolVar(driverContext.getConf(), ConfVars.HIVE_LOCK_MAPRED_ONLY)) { return true; } @@ -501,7 +506,7 @@ class DriverTxnHandler { void handleTransactionAfterExecution() throws CommandProcessorException { try { //since set autocommit starts an implicit txn, close it - if (driverContext.getTxnManager().isImplicitTransactionOpen() || + if (driverContext.getTxnManager().isImplicitTransactionOpen(context) || driverContext.getPlan().getOperation() == HiveOperation.COMMIT) { endTransactionAndCleanup(true); } else if (driverContext.getPlan().getOperation() == HiveOperation.ROLLBACK) { @@ -565,7 +570,7 @@ class DriverTxnHandler { void endTransactionAndCleanup(boolean commit, HiveTxnManager txnManager) throws LockException { PerfLogger perfLogger = SessionState.getPerfLogger(); perfLogger.perfLogBegin(CLASS_NAME, PerfLogger.RELEASE_LOCKS); - + // If we've opened a transaction we need to commit or rollback rather than explicitly releasing the locks. driverContext.getConf().unset(ValidTxnList.VALID_TXNS_KEY); driverContext.getConf().unset(ValidTxnWriteIdList.VALID_TABLES_WRITEIDS_KEY); 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 6034d78..1870f98 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 @@ -65,7 +65,6 @@ import org.apache.hadoop.hive.common.ValidReaderWriteIdList; import org.apache.hadoop.hive.common.ValidTxnWriteIdList; import org.apache.hadoop.hive.common.ValidWriteIdList; import org.apache.hadoop.hive.common.TableName; -import org.apache.hadoop.hive.conf.Constants; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.conf.HiveConf.ConfVars; import org.apache.hadoop.hive.metastore.LockComponentBuilder; @@ -184,19 +183,17 @@ public class AcidUtils { public static final int MAX_STATEMENTS_PER_TXN = 10000; public static final Pattern LEGACY_BUCKET_DIGIT_PATTERN = Pattern.compile("^[0-9]{6}"); public static final Pattern BUCKET_PATTERN = Pattern.compile("bucket_([0-9]+)(_[0-9]+)?$"); - private static final Set<Integer> READ_TXN_TOKENS = new HashSet<Integer>(); + private static final Set<Integer> READ_TXN_TOKENS = new HashSet<>(); private static Cache<String, DirInfoValue> dirCache; private static AtomicBoolean dirCacheInited = new AtomicBoolean(); static { READ_TXN_TOKENS.addAll(Arrays.asList( - HiveParser.TOK_DESCDATABASE, - HiveParser.TOK_DESCTABLE, - HiveParser.TOK_SHOWTABLES, - HiveParser.TOK_SHOW_TABLESTATUS, - HiveParser.TOK_SHOW_TBLPROPERTIES, - HiveParser.TOK_EXPLAIN + HiveParser.TOK_DESCDATABASE, + HiveParser.TOK_DESCTABLE, + HiveParser.TOK_EXPLAIN, + HiveParser.TOK_EXPLAIN_SQ_REWRITE )); } @@ -2724,7 +2721,7 @@ public class AcidUtils { return (name.equals(baseDirName) && dpSpecs.contains(partitionSpec)); } else { - return name.equals(baseDirName) + return name.equals(baseDirName) || (isDeltaPrefix && (name.startsWith(deltaDirName) || name.startsWith(deleteDeltaDirName))) || (!isDeltaPrefix && (name.equals(deltaDirName) || name.equals(deleteDeltaDirName))); } @@ -3101,12 +3098,10 @@ public class AcidUtils { * @param tree AST */ public static TxnType getTxnType(Configuration conf, ASTNode tree) { - int tp = tree.getToken().getType(); // check if read-only txn if (HiveConf.getBoolVar(conf, ConfVars.HIVE_TXN_READONLY_ENABLED) && isReadOnlyTxn(tree)) { return TxnType.READ_ONLY; } - // check if txn has a materialized view rebuild if (tree.getToken().getType() == HiveParser.TOK_ALTER_MATERIALIZED_VIEW_REBUILD) { return TxnType.MATER_VIEW_REBUILD; @@ -3118,18 +3113,15 @@ public class AcidUtils { return TxnType.DEFAULT; } - public static boolean isReadOnlyTxn(ASTNode tree) { final ASTSearcher astSearcher = new ASTSearcher(); - return READ_TXN_TOKENS.contains(tree.getToken().getType()) || (tree.getToken().getType() == HiveParser.TOK_QUERY && - Stream.of( - new int[]{HiveParser.TOK_INSERT_INTO}, - new int[]{HiveParser.TOK_INSERT, HiveParser.TOK_TAB}) - .noneMatch(pattern -> astSearcher.simpleBreadthFirstSearch(tree, pattern) != null)); - + return READ_TXN_TOKENS.contains(tree.getToken().getType()) + || (tree.getToken().getType() == HiveParser.TOK_QUERY && Stream.of( + new int[]{HiveParser.TOK_INSERT_INTO}, + new int[]{HiveParser.TOK_INSERT, HiveParser.TOK_TAB}) + .noneMatch(pattern -> astSearcher.simpleBreadthFirstSearch(tree, pattern) != null)); } - private static void initDirCache(int durationInMts) { if (dirCacheInited.get()) { LOG.debug("DirCache got initialized already"); diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java index 37ea517..300b52b 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java @@ -869,12 +869,19 @@ public final class DbTxnManager extends HiveTxnManagerImpl { @Override public boolean isImplicitTransactionOpen() { + return isImplicitTransactionOpen(null); + } + + @Override + public boolean isImplicitTransactionOpen(Context ctx) { if(!isTxnOpen()) { //some commands like "show databases" don't start implicit transactions return false; } if(!isExplicitTransaction) { - assert numStatements == 1 : "numStatements=" + numStatements; + if (ctx == null || !ctx.isExplainSkipExecution()) { + assert numStatements == 1 : "numStatements=" + numStatements; + } return true; } return false; diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java index 23fb433..88b00f7 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java @@ -291,8 +291,11 @@ public interface HiveTxnManager { */ boolean recordSnapshot(QueryPlan queryPlan); + @Deprecated boolean isImplicitTransactionOpen(); + boolean isImplicitTransactionOpen(Context ctx); + boolean isTxnOpen(); /** * if {@code isTxnOpen()}, returns the currently active transaction ID. diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java index 480dcc1..9d5f9aa 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java @@ -210,12 +210,18 @@ abstract class HiveTxnManagerImpl implements HiveTxnManager, Configurable { public boolean recordSnapshot(QueryPlan queryPlan) { return false; } + @Override public boolean isImplicitTransactionOpen() { return true; } @Override + public boolean isImplicitTransactionOpen(Context ctx) { + return true; + } + + @Override public LockResponse acquireMaterializationRebuildLock(String dbName, String tableName, long txnId) throws LockException { // This is default implementation. Locking only works for incremental maintenance diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java b/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java index a8caf26..b210b94 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java @@ -7498,7 +7498,7 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer { } try { if (ctx.getExplainConfig() != null) { - writeId = 0L; // For explain plan, txn won't be opened and doesn't make sense to allocate write id + writeId = null; // For explain plan, txn won't be opened and doesn't make sense to allocate write id } else { if (isMmTable) { writeId = txnMgr.getTableWriteId(destinationTable.getDbName(), destinationTable.getTableName()); @@ -15096,7 +15096,7 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer { * Some initial checks for a query to see if we can look this query up in the results cache. */ private boolean queryTypeCanUseCache() { - if(this.qb == null || this.qb.getParseInfo() == null) { + if (this.qb == null || this.qb.getParseInfo() == null) { return false; } if (this instanceof ColumnStatsSemanticAnalyzer) { @@ -15108,14 +15108,12 @@ public class SemanticAnalyzer extends BaseSemanticAnalyzer { if (queryState.getHiveOperation() != HiveOperation.QUERY) { return false; } - - if (qb.getParseInfo().isAnalyzeCommand()) { - return false; - } - - if (qb.getParseInfo().hasInsertTables()) { - return false; - } + if (qb.getParseInfo().isAnalyzeCommand()) { + return false; + } + if (qb.getParseInfo().hasInsertTables()) { + return false; + } // HIVE-19096 - disable for explain analyze return ctx.getExplainAnalyze() == null; 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 753c1a7..acd9786 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 @@ -3284,4 +3284,19 @@ public class TestDbTxnManager2 extends DbTxnManagerEndToEndTestBase{ "where \"CTC_PARTITION\"='p=foo' and \"CTC_TXNID\" IN (5,7)")); } + @Test + public void testSkipAcquireLocksForExplain() throws Exception { + dropTable(new String[] {"tab_acid"}); + + driver.run("create table if not exists tab_acid (a int) partitioned by (p string) " + + "stored as orc TBLPROPERTIES ('transactional'='true')"); + driver.run("insert into tab_acid values(1,'foo'),(3,'bar')"); + + driver.compileAndRespond("explain update tab_acid set a = a+2 where a > 2", true); + driver.lockAndRespond(); + + List<ShowLocksResponseElement> locks = getLocks(); + Assert.assertEquals("Unexpected lock count", 0, locks.size()); + } + }