[
https://issues.apache.org/jira/browse/HIVE-24333?focusedWorklogId=509805&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-509805
]
ASF GitHub Bot logged work on HIVE-24333:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 10/Nov/20 16:31
Start Date: 10/Nov/20 16:31
Worklog Time Spent: 10m
Work Description: belugabehr commented on a change in pull request #1629:
URL: https://github.com/apache/hive/pull/1629#discussion_r520699765
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/Driver.java
##########
@@ -359,6 +216,101 @@ private void runInternal(String command, boolean
alreadyCompiled) throws Command
SessionState.getPerfLogger().cleanupPerfLogMetrics();
}
+ /**
+ * @return If the perfLogger should be reseted.
+ */
+ private boolean validateTxnList() throws CommandProcessorException {
+ int retryShapshotCount = 0;
+ int maxRetrySnapshotCount = HiveConf.getIntVar(driverContext.getConf(),
+ HiveConf.ConfVars.HIVE_TXN_MAX_RETRYSNAPSHOT_COUNT);
+
+ try {
+ do {
+ driverContext.setOutdatedTxn(false);
+ // Inserts will not invalidate the snapshot, that could cause
duplicates.
+ if (!driverTxnHandler.isValidTxnListState()) {
+ LOG.info("Re-compiling after acquiring locks, attempt #" +
retryShapshotCount);
+ // Snapshot was outdated when locks were acquired, hence regenerate
context, txn list and retry.
+ // TODO: Lock acquisition should be moved before analyze, this is a
bit hackish.
+ // Currently, we acquire a snapshot, compile the query with that
snapshot, and then - acquire locks.
+ // If snapshot is still valid, we continue as usual.
+ // But if snapshot is not valid, we recompile the query.
+ if (driverContext.isOutdatedTxn()) {
+ // Later transaction invalidated the snapshot, a new transaction
is required
+ LOG.info("Snapshot is outdated, re-initiating transaction ...");
+ driverContext.getTxnManager().rollbackTxn();
+
+ String userFromUGI = DriverUtils.getUserFromUGI(driverContext);
+ driverContext.getTxnManager().openTxn(context, userFromUGI,
driverContext.getTxnType());
+ lockAndRespond();
+ }
+ driverContext.setRetrial(true);
+ driverContext.getBackupContext().addSubContext(context);
+
driverContext.getBackupContext().setHiveLocks(context.getHiveLocks());
+ context = driverContext.getBackupContext();
+
+ driverContext.getConf().set(ValidTxnList.VALID_TXNS_KEY,
+ driverContext.getTxnManager().getValidTxns().toString());
+
+ if (driverContext.getPlan().hasAcidResourcesInQuery()) {
+ compileInternal(context.getCmd(), true);
+ driverTxnHandler.recordValidWriteIds();
+ driverTxnHandler.setWriteIdForAcidFileSinks();
+ }
+ // Since we're reusing the compiled plan, we need to update its
start time for current run
+
driverContext.getPlan().setQueryStartTime(driverContext.getQueryDisplay().getQueryStartTime());
+ }
+ // Re-check snapshot only in case we had to release locks and open a
new transaction,
+ // otherwise exclusive locks should protect output tables/partitions
in snapshot from concurrent writes.
+ } while (driverContext.isOutdatedTxn() && ++retryShapshotCount <=
maxRetrySnapshotCount);
+
+ if (retryShapshotCount > maxRetrySnapshotCount) {
+ // Throw exception
+ HiveException e = new HiveException(
+ "Operation could not be executed, " +
SNAPSHOT_WAS_OUTDATED_WHEN_LOCKS_WERE_ACQUIRED + ".");
+ DriverUtils.handleHiveException(driverContext, e, 14, null);
+ } else if (retryShapshotCount != 0) {
+ // the reason that we set the txn manager for the cxt here is because
each query has its own ctx object.
+ // The txn mgr is shared across the same instance of Driver, which can
run multiple queries.
+ context.setHiveTxnManager(driverContext.getTxnManager());
+ }
Review comment:
Since you are slicing and dicing things, you may want to look at
breaking this out a bit.
It is bad form to throw and exception, and then handle it, within the same
method.
I think this should be moved out of this `try` block.
```
if (retryShapshotCount > maxRetrySnapshotCount) {
// Throw exception
HiveException e = new HiveException(
"Operation could not be executed, " +
SNAPSHOT_WAS_OUTDATED_WHEN_LOCKS_WERE_ACQUIRED + ".");
DriverUtils.handleHiveException(driverContext, e, 14, null);
}
if (retryShapshotCount != 0) {
// the reason that we set the txn manager for the cxt here is
because each query has its own ctx object.
// The txn mgr is shared across the same instance of Driver, which
can run multiple queries.
context.setHiveTxnManager(driverContext.getTxnManager());
return true;
}
return false;
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 509805)
Time Spent: 50m (was: 40m)
> Cut long methods in Driver to smaller, more manageable pieces
> -------------------------------------------------------------
>
> Key: HIVE-24333
> URL: https://issues.apache.org/jira/browse/HIVE-24333
> Project: Hive
> Issue Type: Sub-task
> Components: Hive
> Reporter: Miklos Gergely
> Assignee: Miklos Gergely
> Priority: Major
> Labels: pull-request-available
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Some methods in Driver are too long to be easily understandable. They should
> be cut into pieces to make them easier to understand.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
