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 <[email protected]>
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 <[email protected]>
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());
+ }
+
}
