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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
