[
https://issues.apache.org/jira/browse/HIVE-23725?focusedWorklogId=449642&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-449642
]
ASF GitHub Bot logged work on HIVE-23725:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 23/Jun/20 05:26
Start Date: 23/Jun/20 05:26
Worklog Time Spent: 10m
Work Description: jcamachor commented on a change in pull request #1151:
URL: https://github.com/apache/hive/pull/1151#discussion_r443969420
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/Driver.java
##########
@@ -675,50 +678,18 @@ private void runInternal(String command, boolean
alreadyCompiled) throws Command
try {
if (!validTxnManager.isValidTxnListState()) {
- LOG.info("Compiling after acquiring locks");
+ LOG.info("Reexecuting after acquiring locks, since snapshot was
outdated.");
// 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, we compile the query wrt that
snapshot,
- // and then, we acquire locks. If snapshot is still valid, we
continue as usual.
- // But if snapshot is not valid, we recompile the query.
- if (driverContext.isOutdatedTxn()) {
- 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()) {
- validTxnManager.recordValidWriteIds();
- }
-
- if (!alreadyCompiled) {
- // compile internal will automatically reset the perf logger
- compileInternal(command, true);
- } else {
- // Since we're reusing the compiled plan, we need to update its
start time for current run
-
driverContext.getPlan().setQueryStartTime(driverContext.getQueryDisplay().getQueryStartTime());
- }
-
- if (!validTxnManager.isValidTxnListState()) {
- // Throw exception
- throw handleHiveException(new HiveException("Operation could not
be executed"), 14);
+ // txn list and retry (see ReExecutionRetryLockPlugin)
+ try {
+ releaseLocksAndCommitOrRollback(false);
+ } catch (LockException e) {
+ handleHiveException(e, 12);
}
-
- //Reset the PerfLogger
- perfLogger = SessionState.getPerfLogger(true);
-
- // 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());
+ throw handleHiveException(
Review comment:
```
In the original logic, if another commit invalidated the snaphsot a second
time, the query also failed with HiveExection.
```
Iiuc that should not happen because we were holding the locks that we had
already acquired; however, now we are releasing them. Hence, the logic is
slightly different?
In any case, it is straightforward to add a config property such as
`HIVE_QUERY_MAX_REEXECUTION_COUNT` for this specific retry, then retrieve it in
`shouldReExecute` method in `ReExecutionRetryLockPlugin`: You have both the
number of retries as well as the conf (`getConf` method) to retrieve the max
number of retries for the config. The check on
`HIVE_QUERY_MAX_REEXECUTION_COUNT` for the rest of the plugins will need to be
moved into `shouldReExecute` method in those plugins too (currently it is done
within the `run` method in the `ReExecDriver` itself).
----------------------------------------------------------------
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: 449642)
Time Spent: 1h 40m (was: 1.5h)
> ValidTxnManager snapshot outdating causing partial reads in merge insert
> ------------------------------------------------------------------------
>
> Key: HIVE-23725
> URL: https://issues.apache.org/jira/browse/HIVE-23725
> Project: Hive
> Issue Type: Bug
> Reporter: Peter Varga
> Assignee: Peter Varga
> Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> When the ValidTxnManager invalidates the snapshot during merge insert and
> starts to read committed transactions that were not committed when the query
> compilation happened, it can cause partial read problems if the committed
> transaction created new partition in the source or target table.
> The solution should be not only fix the snapshot but also recompile the query
> and acquire the locks again.
> You could construct an example like this:
> 1. open and compile transaction 1 that merge inserts data from a partitioned
> source table that has a few partition.
> 2. Open, run and commit transaction 2 that inserts data to an old and a new
> partition to the source table.
> 3. Open, run and commit transaction 3 that inserts data to the target table
> of the merge statement, that will retrigger a snapshot generation in
> transaction 1.
> 4. Run transaction 1, the snapshot will be regenerated, and it will read
> partial data from transaction 2 breaking the ACID properties.
> Different setup.
> Switch the transaction order:
> 1. compile transaction 1 that inserts data to an old and a new partition of
> the source table.
> 2. compile transaction 2 that insert data to the target table
> 2. compile transaction 3 that merge inserts data from the source table to the
> target table
> 3. run and commit transaction 1
> 4. run and commit transaction 2
> 5. run transaction 3, since it cointains 1 and 2 in its snaphot the
> isValidTxnListState will be triggered and we do a partial read of the
> transaction 1 for the same reasons.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
