-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-29291: Use minHistoryWriteId by default #6154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
140f810 to
15c61e6
Compare
| } | ||
| }); | ||
|
|
||
| Long minTxnId = jdbcResource.execute(new MinUncommittedTxnIdHandler()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
existing MinUncommittedTxnIdHandler wasn't used for unknown reason
| if (txnType == TxnType.SOFT_DELETE || txnType == TxnType.COMPACTION) { | ||
| new AcquireTxnLockFunction(false).execute(jdbcResource); | ||
| if (!ConfVars.useMinHistoryWriteId()) { | ||
| new AcquireTxnLockFunction(false).execute(jdbcResource); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nextTxnId hwm is not used with MinHistoryWriteId, so no need to lock in oder to get accurate value
| "FROM \"TXN_COMPONENTS\" WHERE \"TC_TXNID\"= :txnId AND \"TC_OPERATION_TYPE\" <> :type"), | ||
| writeSetInsertSql + (ConfVars.useMinHistoryLevel() ? conflictSQLSuffix : | ||
| "FROM \"TXN_COMPONENTS\" WHERE \"TC_TXNID\"= :txnId" + ( | ||
| (txnType != TxnType.REBALANCE_COMPACTION) ? "" : " AND \"TC_OPERATION_TYPE\" <> :type")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
REBALANCE_COMPACTION uses UPDATE operation semantic with EXCL_WRITE
| queryBatch.add("DELETE FROM \"MATERIALIZATION_REBUILD_LOCKS\" WHERE \"MRL_TXN_ID\" = " + txnid); | ||
| } | ||
| if (txnType == TxnType.SOFT_DELETE || txnType == TxnType.COMPACTION) { | ||
| if (txnType == TxnType.SOFT_DELETE || MetaStoreServerUtils.isCompactionTxn(txnType)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no idea why REBALANCE_COMPACTION was excluded, looks like a bug
| " \"CUR\".\"TC_PARTITION\" IS NULL) " + | ||
| // txns overlap | ||
| " WHERE \"CUR\".\"TC_TXNID\" <= \"COMMITTED\".\"WS_COMMIT_ID\""; | ||
| " WHERE \"CUR\".\"TC_TXNID\" <= \"COMMITTED\".\"WS_COMMIT_ID\"" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just moved the predicate under the WHERE clause
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this fix and sorry for the late response. I didn't get any notification about the review request.
Anyway I went trough the patch and it looks good to me, I have only one question.
...ver/src/main/java/org/apache/hadoop/hive/metastore/txn/jdbc/functions/CommitTxnFunction.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM
64ab2ad to
3aad9d0
Compare
|



What changes were proposed in this pull request?
Use minHistoryWriteId by default
Why are the changes needed?
Overcomes scalability constraints imposed by global min open txn concept
Does this PR introduce any user-facing change?
No
How was this patch tested?
Jenkins