[ 
https://issues.apache.org/jira/browse/HIVE-25346?focusedWorklogId=652245&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-652245
 ]

ASF GitHub Bot logged work on HIVE-25346:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 17/Sep/21 09:54
            Start Date: 17/Sep/21 09:54
    Worklog Time Spent: 10m 
      Work Description: deniskuzZ commented on a change in pull request #2547:
URL: https://github.com/apache/hive/pull/2547#discussion_r710919584



##########
File path: 
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1446,70 +1446,75 @@ public void commitTxn(CommitTxnRequest rqst)
                 OperationType.UPDATE + "," + OperationType.DELETE + ")";
 
         long tempCommitId = generateTemporaryId();
-        if (txnType.get() != TxnType.READ_ONLY
-                && !isReplayedReplTxn
-                && isUpdateOrDelete(stmt, conflictSQLSuffix)) {
-
-          isUpdateDelete = 'Y';
-          //if here it means currently committing txn performed update/delete 
and we should check WW conflict
-          /**
-           * "select distinct" is used below because
-           * 1. once we get to multi-statement txns, we only care to record 
that something was updated once
-           * 2. if {@link #addDynamicPartitions(AddDynamicPartitions)} is 
retried by caller it may create
-           *  duplicate entries in TXN_COMPONENTS
-           * but we want to add a PK on WRITE_SET which won't have unique rows 
w/o this distinct
-           * even if it includes all of its columns
-           *
-           * First insert into write_set using a temporary commitID, which 
will be updated in a separate call,
-           * see: {@link #updateWSCommitIdAndCleanUpMetadata(Statement, long, 
TxnType, Long, long)}}.
-           * This should decrease the scope of the S4U lock on the next_txn_id 
table.
-           */
-          Savepoint undoWriteSetForCurrentTxn = dbConn.setSavepoint();
-          stmt.executeUpdate("INSERT INTO \"WRITE_SET\" (\"WS_DATABASE\", 
\"WS_TABLE\", \"WS_PARTITION\", \"WS_TXNID\", \"WS_COMMIT_ID\", 
\"WS_OPERATION_TYPE\")" +
-                          " SELECT DISTINCT \"TC_DATABASE\", \"TC_TABLE\", 
\"TC_PARTITION\", \"TC_TXNID\", " + tempCommitId + ", \"TC_OPERATION_TYPE\" " + 
conflictSQLSuffix);
-
-          /**
-           * This S4U will mutex with other commitTxn() and openTxns().
-           * -1 below makes txn intervals look like [3,3] [4,4] if all txns 
are serial
-           * Note: it's possible to have several txns have the same commit id. 
 Suppose 3 txns start
-           * at the same time and no new txns start until all 3 commit.
-           * We could've incremented the sequence for commitId as well but it 
doesn't add anything functionally.
-           */
-          acquireTxnLock(stmt, false);
-          commitId = getHighWaterMark(stmt);
+        if (txnType.get() != TxnType.READ_ONLY && !isReplayedReplTxn && 
txnType.get() != TxnType.COMPACTION) {
+          if (isUpdateOrDelete(stmt, conflictSQLSuffix)) {
+            isUpdateDelete = 'Y';
+            //if here it means currently committing txn performed 
update/delete and we should check WW conflict
+            /**
+             * "select distinct" is used below because
+             * 1. once we get to multi-statement txns, we only care to record 
that something was updated once
+             * 2. if {@link #addDynamicPartitions(AddDynamicPartitions)} is 
retried by caller it may create
+             *  duplicate entries in TXN_COMPONENTS
+             * but we want to add a PK on WRITE_SET which won't have unique 
rows w/o this distinct
+             * even if it includes all of its columns
+             *
+             * First insert into write_set using a temporary commitID, which 
will be updated in a separate call,
+             * see: {@link #updateWSCommitIdAndCleanUpMetadata(Statement, 
long, TxnType, Long, long)}}.
+             * This should decrease the scope of the S4U lock on the 
next_txn_id table.
+             */
+            Savepoint undoWriteSetForCurrentTxn = dbConn.setSavepoint();
+            stmt.executeUpdate("INSERT INTO \"WRITE_SET\" (\"WS_DATABASE\", 
\"WS_TABLE\", \"WS_PARTITION\", \"WS_TXNID\", \"WS_COMMIT_ID\", 
\"WS_OPERATION_TYPE\")" +
+                            " SELECT DISTINCT \"TC_DATABASE\", \"TC_TABLE\", 
\"TC_PARTITION\", \"TC_TXNID\", " + tempCommitId + ", \"TC_OPERATION_TYPE\" " + 
conflictSQLSuffix);
 
-          if (!rqst.isExclWriteEnabled()) {
             /**
-             * see if there are any overlapping txns that wrote the same 
element, i.e. have a conflict
-             * Since entire commit operation is mutexed wrt other start/commit 
ops,
-             * committed.ws_commit_id <= current.ws_commit_id for all txns
-             * thus if committed.ws_commit_id < current.ws_txnid, transactions 
do NOT overlap
-             * For example, [17,20] is committed, [6,80] is being committed 
right now - these overlap
-             * [17,20] committed and [21,21] committing now - these do not 
overlap.
-             * [17,18] committed and [18,19] committing now - these overlap  
(here 18 started while 17 was still running)
+             * This S4U will mutex with other commitTxn() and openTxns().
+             * -1 below makes txn intervals look like [3,3] [4,4] if all txns 
are serial
+             * Note: it's possible to have several txns have the same commit 
id.  Suppose 3 txns start
+             * at the same time and no new txns start until all 3 commit.
+             * We could've incremented the sequence for commitId as well but 
it doesn't add anything functionally.
              */
-            try (ResultSet rs = checkForWriteConflict(stmt, txnid)) {
-              if (rs.next()) {
-                //found a conflict, so let's abort the txn
-                String committedTxn = "[" + 
JavaUtils.txnIdToString(rs.getLong(1)) + "," + rs.getLong(2) + "]";
-                StringBuilder resource = new 
StringBuilder(rs.getString(3)).append("/").append(rs.getString(4));
-                String partitionName = rs.getString(5);
-                if (partitionName != null) {
-                  resource.append('/').append(partitionName);
-                }
-                String msg = "Aborting [" + JavaUtils.txnIdToString(txnid) + 
"," + commitId + "]" + " due to a write conflict on " + resource +
-                        " committed by " + committedTxn + " " + 
rs.getString(7) + "/" + rs.getString(8);
-                //remove WRITE_SET info for current txn since it's about to 
abort
-                dbConn.rollback(undoWriteSetForCurrentTxn);
-                LOG.info(msg);
-                //todo: should make abortTxns() write something into 
TXNS.TXN_META_INFO about this
-                if (abortTxns(dbConn, Collections.singletonList(txnid), false, 
isReplayedReplTxn) != 1) {
-                  throw new IllegalStateException(msg + " FAILED!");
+            acquireTxnLock(stmt, false);
+            commitId = getHighWaterMark(stmt);
+
+            if (!rqst.isExclWriteEnabled()) {
+              /**
+               * see if there are any overlapping txns that wrote the same 
element, i.e. have a conflict
+               * Since entire commit operation is mutexed wrt other 
start/commit ops,
+               * committed.ws_commit_id <= current.ws_commit_id for all txns
+               * thus if committed.ws_commit_id < current.ws_txnid, 
transactions do NOT overlap
+               * For example, [17,20] is committed, [6,80] is being committed 
right now - these overlap
+               * [17,20] committed and [21,21] committing now - these do not 
overlap.
+               * [17,18] committed and [18,19] committing now - these overlap  
(here 18 started while 17 was still running)
+               */
+              try (ResultSet rs = checkForWriteConflict(stmt, txnid)) {
+                if (rs.next()) {
+                  //found a conflict, so let's abort the txn
+                  String committedTxn = "[" + 
JavaUtils.txnIdToString(rs.getLong(1)) + "," + rs.getLong(2) + "]";
+                  StringBuilder resource = new 
StringBuilder(rs.getString(3)).append("/").append(rs.getString(4));
+                  String partitionName = rs.getString(5);
+                  if (partitionName != null) {
+                    resource.append('/').append(partitionName);
+                  }
+                  String msg = "Aborting [" + JavaUtils.txnIdToString(txnid) + 
"," + commitId + "]" + " due to a write conflict on " + resource +
+                          " committed by " + committedTxn + " " + 
rs.getString(7) + "/" + rs.getString(8);
+                  //remove WRITE_SET info for current txn since it's about to 
abort
+                  dbConn.rollback(undoWriteSetForCurrentTxn);
+                  LOG.info(msg);
+                  //todo: should make abortTxns() write something into 
TXNS.TXN_META_INFO about this
+                  if (abortTxns(dbConn, Collections.singletonList(txnid), 
false, isReplayedReplTxn) != 1) {
+                    throw new IllegalStateException(msg + " FAILED!");
+                  }
+                  dbConn.commit();
+                  throw new TxnAbortedException(msg);
                 }
-                dbConn.commit();
-                throw new TxnAbortedException(msg);
               }
             }
+          } else if (isInsert(stmt, txnid)) {

Review comment:
       You don't have to check for `insert` as there is no other alternative.




-- 
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.

To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 652245)
    Time Spent: 7h 20m  (was: 7h 10m)

> cleanTxnToWriteIdTable breaks SNAPSHOT isolation
> ------------------------------------------------
>
>                 Key: HIVE-25346
>                 URL: https://issues.apache.org/jira/browse/HIVE-25346
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Zoltan Chovan
>            Assignee: Zoltan Chovan
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 7h 20m
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to