You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "maheshrajus (via GitHub)" <gi...@apache.org> on 2023/03/30 14:39:45 UTC
[GitHub] [hive] maheshrajus opened a new pull request, #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
maheshrajus opened a new pull request, #4174:
URL: https://github.com/apache/hive/pull/4174
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
Delete directly aborted transactions instead of select and loading ids into memory.
### Why are the changes needed?
select needs to be eliminated and the delete should work with the where clause instead of the built in clause.
we can see no reason for loading the ids into memory and then generate a huge sql.
### Does this PR introduce _any_ user-facing change?
NO
### How was this patch tested?
CI run/UT tests
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1196387599
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -873,61 +878,35 @@ public void removeDuplicateCompletedTxnComponents() throws MetaException {
public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
LOG.info("Start to clean empty aborted or committed TXNS");
try {
- Connection dbConn = null;
- Statement stmt = null;
- ResultSet rs = null;
- try {
- //Aborted and committed are terminal states, so nothing about the txn can change
- //after that, so READ COMMITTED is sufficient.
- dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED, connPoolCompaction);
- stmt = dbConn.createStatement();
- /*
- * Only delete aborted / committed transaction in a way that guarantees two things:
- * 1. never deletes anything that is inside the TXN_OPENTXN_TIMEOUT window
- * 2. never deletes the maximum txnId even if it is before the TXN_OPENTXN_TIMEOUT window
- */
- long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
-
- String s = "SELECT \"TXN_ID\" FROM \"TXNS\" WHERE " +
- "\"TXN_ID\" NOT IN (SELECT \"TC_TXNID\" FROM \"TXN_COMPONENTS\") AND " +
- " (\"TXN_STATE\" = " + TxnStatus.ABORTED + " OR \"TXN_STATE\" = " + TxnStatus.COMMITTED + ") AND "
- + " \"TXN_ID\" < " + lowWaterMark;
- LOG.debug("Going to execute query <{}>", s);
- rs = stmt.executeQuery(s);
- List<Long> txnids = new ArrayList<>();
- while (rs.next()) txnids.add(rs.getLong(1));
- close(rs);
- if(txnids.size() <= 0) {
- return;
- }
- Collections.sort(txnids);//easier to read logs
-
- List<String> queries = new ArrayList<>();
- StringBuilder prefix = new StringBuilder();
- StringBuilder suffix = new StringBuilder();
-
- // Delete from TXNS.
- prefix.append("DELETE FROM \"TXNS\" WHERE ");
-
- TxnUtils.buildQueryWithINClause(conf, queries, prefix, suffix, txnids, "\"TXN_ID\"", false, false);
-
- for (String query : queries) {
- LOG.debug("Going to execute update <{}>", query);
- int rc = stmt.executeUpdate(query);
+ try (Connection dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED, connPoolCompaction)) {
+ try (PreparedStatement stmt = dbConn.prepareStatement(DELETE_FAILED_TXNS_DIRECTLY_SQL)) {
+ //Aborted and committed are terminal states, so nothing about the txn can change
+ //after that, so READ COMMITTED is sufficient.
+ /*
+ * Only delete aborted / committed transaction in a way that guarantees two things:
+ * 1. never deletes anything that is inside the TXN_OPENTXN_TIMEOUT window
+ * 2. never deletes the maximum txnId even if it is before the TXN_OPENTXN_TIMEOUT window
+ */
+ long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
+ stmt.setLong(1, lowWaterMark);
+ LOG.debug("Going to execute query <{}>", DELETE_FAILED_TXNS_DIRECTLY_SQL);
+ int rc = stmt.executeUpdate();
LOG.debug("Removed {} empty Aborted and Committed transactions from TXNS", rc);
+ LOG.debug("Going to commit");
+ dbConn.commit();
+ } catch (SQLException e) {
+ LOG.error("Unable to delete from txns table " + e.getMessage());
+ LOG.debug("Going to rollback");
+ rollbackDBConn(dbConn);
+ checkRetryable(e, "cleanEmptyAbortedTxns");
+ throw new MetaException("Unable to connect to transaction database " +
Review Comment:
error message is confusing, should be something like `Failed to delete` or something
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4174:
URL: https://github.com/apache/hive/pull/4174#issuecomment-1553350271
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4174)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=duplicated_lines_density&view=list) No Duplication information
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ merged pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ merged PR #4174:
URL: https://github.com/apache/hive/pull/4174
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4174:
URL: https://github.com/apache/hive/pull/4174#issuecomment-1490709732
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4174)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=duplicated_lines_density&view=list) No Duplication information
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1179137448
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -888,35 +887,13 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
*/
long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
- String s = "SELECT \"TXN_ID\" FROM \"TXNS\" WHERE " +
+ String s = "DELETE FROM \"TXNS\" WHERE " +
Review Comment:
Could we move sql to constants (see `TxnQueries`) and use prepared statement?
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1183331418
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -926,10 +900,8 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
checkRetryable(e, "cleanEmptyAbortedTxns");
throw new MetaException("Unable to connect to transaction database " +
e.getMessage());
- } finally {
- close(rs, stmt, dbConn);
}
- } catch (RetryException e) {
+ } catch (RetryException | SQLException e) {
Review Comment:
SQLException from getDbConn() is not checked for being Retryable
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4174:
URL: https://github.com/apache/hive/pull/4174#issuecomment-1550420268
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4174)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=duplicated_lines_density&view=list) No Duplication information
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4174:
URL: https://github.com/apache/hive/pull/4174#issuecomment-1531691029
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4174)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=duplicated_lines_density&view=list) No Duplication information
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] maheshrajus commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "maheshrajus (via GitHub)" <gi...@apache.org>.
maheshrajus commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1182612109
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -888,35 +887,13 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
*/
long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
- String s = "SELECT \"TXN_ID\" FROM \"TXNS\" WHERE " +
+ String s = "DELETE FROM \"TXNS\" WHERE " +
Review Comment:
done
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] scarlin-cloudera commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "scarlin-cloudera (via GitHub)" <gi...@apache.org>.
scarlin-cloudera commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1153811990
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -888,7 +888,7 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
*/
long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
- String s = "SELECT \"TXN_ID\" FROM \"TXNS\" WHERE " +
+ String s = "DELETE \"TXN_ID\" FROM \"TXNS\" WHERE " +
Review Comment:
Do the delete Ids get returned in the result set of executeQuery? Can we delete the rest of the code that collects these Ids and then appears to delete them ( a second time)?
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] maheshrajus commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "maheshrajus (via GitHub)" <gi...@apache.org>.
maheshrajus commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1182136243
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -888,35 +887,13 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
*/
long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
- String s = "SELECT \"TXN_ID\" FROM \"TXNS\" WHERE " +
+ String s = "DELETE FROM \"TXNS\" WHERE " +
Review Comment:
hi @deniskuzZ, the query is not hardcoded fully.
we have some txn state fields in middile of the query.
TxnStatus.ABORTED, and TxnStatus.COMMITTED. So better to keep like this for better readable purpose.
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1182176853
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -875,7 +875,6 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
try {
Connection dbConn = null;
Statement stmt = null;
- ResultSet rs = null;
Review Comment:
try-with-resource works fine with that construct
````
try(Connection con = getConnection()) {
try (PreparedStatement prep = con.prepareConnection("delete ...")) {
....
con.commit()
} catch (SQLException e) {
....
con.rollback();
....
}
}
````
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] maheshrajus commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "maheshrajus (via GitHub)" <gi...@apache.org>.
maheshrajus commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1182127027
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -875,7 +875,6 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
try {
Connection dbConn = null;
Statement stmt = null;
- ResultSet rs = null;
Review Comment:
Hi @deniskuzZ thx for review.
we are not just closing the db connection in finally block, we are doing rollback in catch block( rollbackDBConn(dbConn)).
I think try-with-resource may not work in this case. So i am keeping try block code only.
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4174:
URL: https://github.com/apache/hive/pull/4174#issuecomment-1552224716
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4174)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=duplicated_lines_density&view=list) No Duplication information
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4174:
URL: https://github.com/apache/hive/pull/4174#issuecomment-1520552035
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4174)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=duplicated_lines_density&view=list) No Duplication information
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1196388750
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -873,61 +878,35 @@ public void removeDuplicateCompletedTxnComponents() throws MetaException {
public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
LOG.info("Start to clean empty aborted or committed TXNS");
try {
- Connection dbConn = null;
- Statement stmt = null;
- ResultSet rs = null;
- try {
- //Aborted and committed are terminal states, so nothing about the txn can change
- //after that, so READ COMMITTED is sufficient.
- dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED, connPoolCompaction);
- stmt = dbConn.createStatement();
- /*
- * Only delete aborted / committed transaction in a way that guarantees two things:
- * 1. never deletes anything that is inside the TXN_OPENTXN_TIMEOUT window
- * 2. never deletes the maximum txnId even if it is before the TXN_OPENTXN_TIMEOUT window
- */
- long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
-
- String s = "SELECT \"TXN_ID\" FROM \"TXNS\" WHERE " +
- "\"TXN_ID\" NOT IN (SELECT \"TC_TXNID\" FROM \"TXN_COMPONENTS\") AND " +
- " (\"TXN_STATE\" = " + TxnStatus.ABORTED + " OR \"TXN_STATE\" = " + TxnStatus.COMMITTED + ") AND "
- + " \"TXN_ID\" < " + lowWaterMark;
- LOG.debug("Going to execute query <{}>", s);
- rs = stmt.executeQuery(s);
- List<Long> txnids = new ArrayList<>();
- while (rs.next()) txnids.add(rs.getLong(1));
- close(rs);
- if(txnids.size() <= 0) {
- return;
- }
- Collections.sort(txnids);//easier to read logs
-
- List<String> queries = new ArrayList<>();
- StringBuilder prefix = new StringBuilder();
- StringBuilder suffix = new StringBuilder();
-
- // Delete from TXNS.
- prefix.append("DELETE FROM \"TXNS\" WHERE ");
-
- TxnUtils.buildQueryWithINClause(conf, queries, prefix, suffix, txnids, "\"TXN_ID\"", false, false);
-
- for (String query : queries) {
- LOG.debug("Going to execute update <{}>", query);
- int rc = stmt.executeUpdate(query);
+ try (Connection dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED, connPoolCompaction)) {
+ try (PreparedStatement stmt = dbConn.prepareStatement(DELETE_FAILED_TXNS_DIRECTLY_SQL)) {
+ //Aborted and committed are terminal states, so nothing about the txn can change
+ //after that, so READ COMMITTED is sufficient.
+ /*
+ * Only delete aborted / committed transaction in a way that guarantees two things:
+ * 1. never deletes anything that is inside the TXN_OPENTXN_TIMEOUT window
+ * 2. never deletes the maximum txnId even if it is before the TXN_OPENTXN_TIMEOUT window
+ */
+ long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
+ stmt.setLong(1, lowWaterMark);
+ LOG.debug("Going to execute query <{}>", DELETE_FAILED_TXNS_DIRECTLY_SQL);
+ int rc = stmt.executeUpdate();
LOG.debug("Removed {} empty Aborted and Committed transactions from TXNS", rc);
+ LOG.debug("Going to commit");
+ dbConn.commit();
+ } catch (SQLException e) {
+ LOG.error("Unable to delete from txns table " + e.getMessage());
+ LOG.debug("Going to rollback");
+ rollbackDBConn(dbConn);
+ checkRetryable(e, "cleanEmptyAbortedTxns");
+ throw new MetaException("Unable to connect to transaction database " +
+ e.getMessage());
}
- LOG.info("Aborted and committed transactions removed from TXNS: {}", txnids);
- LOG.debug("Going to commit");
- dbConn.commit();
} catch (SQLException e) {
- LOG.error("Unable to delete from txns table " + e.getMessage());
- LOG.debug("Going to rollback");
- rollbackDBConn(dbConn);
+ LOG.error("Unable to connect to transaction database " + e.getMessage());
Review Comment:
can we move the string literals into the constants?
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -873,61 +878,35 @@ public void removeDuplicateCompletedTxnComponents() throws MetaException {
public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
LOG.info("Start to clean empty aborted or committed TXNS");
try {
- Connection dbConn = null;
- Statement stmt = null;
- ResultSet rs = null;
- try {
- //Aborted and committed are terminal states, so nothing about the txn can change
- //after that, so READ COMMITTED is sufficient.
- dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED, connPoolCompaction);
- stmt = dbConn.createStatement();
- /*
- * Only delete aborted / committed transaction in a way that guarantees two things:
- * 1. never deletes anything that is inside the TXN_OPENTXN_TIMEOUT window
- * 2. never deletes the maximum txnId even if it is before the TXN_OPENTXN_TIMEOUT window
- */
- long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
-
- String s = "SELECT \"TXN_ID\" FROM \"TXNS\" WHERE " +
- "\"TXN_ID\" NOT IN (SELECT \"TC_TXNID\" FROM \"TXN_COMPONENTS\") AND " +
- " (\"TXN_STATE\" = " + TxnStatus.ABORTED + " OR \"TXN_STATE\" = " + TxnStatus.COMMITTED + ") AND "
- + " \"TXN_ID\" < " + lowWaterMark;
- LOG.debug("Going to execute query <{}>", s);
- rs = stmt.executeQuery(s);
- List<Long> txnids = new ArrayList<>();
- while (rs.next()) txnids.add(rs.getLong(1));
- close(rs);
- if(txnids.size() <= 0) {
- return;
- }
- Collections.sort(txnids);//easier to read logs
-
- List<String> queries = new ArrayList<>();
- StringBuilder prefix = new StringBuilder();
- StringBuilder suffix = new StringBuilder();
-
- // Delete from TXNS.
- prefix.append("DELETE FROM \"TXNS\" WHERE ");
-
- TxnUtils.buildQueryWithINClause(conf, queries, prefix, suffix, txnids, "\"TXN_ID\"", false, false);
-
- for (String query : queries) {
- LOG.debug("Going to execute update <{}>", query);
- int rc = stmt.executeUpdate(query);
+ try (Connection dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED, connPoolCompaction)) {
+ try (PreparedStatement stmt = dbConn.prepareStatement(DELETE_FAILED_TXNS_DIRECTLY_SQL)) {
+ //Aborted and committed are terminal states, so nothing about the txn can change
+ //after that, so READ COMMITTED is sufficient.
+ /*
+ * Only delete aborted / committed transaction in a way that guarantees two things:
+ * 1. never deletes anything that is inside the TXN_OPENTXN_TIMEOUT window
+ * 2. never deletes the maximum txnId even if it is before the TXN_OPENTXN_TIMEOUT window
+ */
+ long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
+ stmt.setLong(1, lowWaterMark);
+ LOG.debug("Going to execute query <{}>", DELETE_FAILED_TXNS_DIRECTLY_SQL);
+ int rc = stmt.executeUpdate();
LOG.debug("Removed {} empty Aborted and Committed transactions from TXNS", rc);
+ LOG.debug("Going to commit");
+ dbConn.commit();
+ } catch (SQLException e) {
+ LOG.error("Unable to delete from txns table " + e.getMessage());
+ LOG.debug("Going to rollback");
+ rollbackDBConn(dbConn);
+ checkRetryable(e, "cleanEmptyAbortedTxns");
+ throw new MetaException("Unable to connect to transaction database " +
+ e.getMessage());
}
- LOG.info("Aborted and committed transactions removed from TXNS: {}", txnids);
- LOG.debug("Going to commit");
- dbConn.commit();
} catch (SQLException e) {
- LOG.error("Unable to delete from txns table " + e.getMessage());
- LOG.debug("Going to rollback");
- rollbackDBConn(dbConn);
+ LOG.error("Unable to connect to transaction database " + e.getMessage());
Review Comment:
could we move the string literals into the constants?
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4174:
URL: https://github.com/apache/hive/pull/4174#issuecomment-1551581024
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4174)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=duplicated_lines_density&view=list) No Duplication information
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] maheshrajus commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "maheshrajus (via GitHub)" <gi...@apache.org>.
maheshrajus commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1196415840
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -87,6 +87,11 @@ class CompactionTxnHandler extends TxnHandler {
"DELETE FROM \"COMPACTION_METRICS_CACHE\" WHERE \"CMC_DATABASE\" = ? AND \"CMC_TABLE\" = ? " +
"AND \"CMC_METRIC_TYPE\" = ?";
+ private static final String DELETE_FAILED_TXNS_DIRECTLY_SQL =
Review Comment:
as per our discussion this we will keep in CompactionTxnHandler file.
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] maheshrajus commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "maheshrajus (via GitHub)" <gi...@apache.org>.
maheshrajus commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1175188754
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -888,7 +888,7 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
*/
long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
- String s = "SELECT \"TXN_ID\" FROM \"TXNS\" WHERE " +
+ String s = "DELETE \"TXN_ID\" FROM \"TXNS\" WHERE " +
Review Comment:
yes, done. thank you for review.
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4174:
URL: https://github.com/apache/hive/pull/4174#issuecomment-1524846048
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4174)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=duplicated_lines_density&view=list) No Duplication information
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1179133565
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -875,7 +875,6 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
try {
Connection dbConn = null;
Statement stmt = null;
- ResultSet rs = null;
Review Comment:
please use `try-with-resources` instead of explicit resource management
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] maheshrajus commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "maheshrajus (via GitHub)" <gi...@apache.org>.
maheshrajus commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1182226180
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -875,7 +875,6 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
try {
Connection dbConn = null;
Statement stmt = null;
- ResultSet rs = null;
Review Comment:
ok got it. done
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1183324488
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -872,51 +877,20 @@ public void removeDuplicateCompletedTxnComponents() throws MetaException {
@RetrySemantics.SafeToRetry
public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
LOG.info("Start to clean empty aborted or committed TXNS");
- try {
- Connection dbConn = null;
- Statement stmt = null;
- ResultSet rs = null;
- try {
+ try (Connection dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED, connPoolCompaction)) {
+ try (Statement stmt = dbConn.createStatement()) {
Review Comment:
please use prepared statement
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] maheshrajus commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "maheshrajus (via GitHub)" <gi...@apache.org>.
maheshrajus commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1195315143
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -87,6 +87,11 @@ class CompactionTxnHandler extends TxnHandler {
"DELETE FROM \"COMPACTION_METRICS_CACHE\" WHERE \"CMC_DATABASE\" = ? AND \"CMC_TABLE\" = ? " +
"AND \"CMC_METRIC_TYPE\" = ?";
+ private static final String DELETE_FAILED_TXNS_DIRECTLY_SQL =
Review Comment:
moving to TxnQueries again need to add mvn dependency. As this is different module.
For simple we will keep in this file only as some existed SQL strings also present in this file.
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1196313656
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -87,6 +87,11 @@ class CompactionTxnHandler extends TxnHandler {
"DELETE FROM \"COMPACTION_METRICS_CACHE\" WHERE \"CMC_DATABASE\" = ? AND \"CMC_TABLE\" = ? " +
"AND \"CMC_METRIC_TYPE\" = ?";
+ private static final String DELETE_FAILED_TXNS_DIRECTLY_SQL =
Review Comment:
they are in the same module and package:
````
package org.apache.hadoop.hive.metastore.txn
class TxnQueries
class CompactionTxnHandler
````
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4174:
URL: https://github.com/apache/hive/pull/4174#issuecomment-1551124096
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4174)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=duplicated_lines_density&view=list) No Duplication information
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1196390298
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -873,61 +878,35 @@ public void removeDuplicateCompletedTxnComponents() throws MetaException {
public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
LOG.info("Start to clean empty aborted or committed TXNS");
try {
- Connection dbConn = null;
- Statement stmt = null;
- ResultSet rs = null;
- try {
- //Aborted and committed are terminal states, so nothing about the txn can change
- //after that, so READ COMMITTED is sufficient.
- dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED, connPoolCompaction);
- stmt = dbConn.createStatement();
- /*
- * Only delete aborted / committed transaction in a way that guarantees two things:
- * 1. never deletes anything that is inside the TXN_OPENTXN_TIMEOUT window
- * 2. never deletes the maximum txnId even if it is before the TXN_OPENTXN_TIMEOUT window
- */
- long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
-
- String s = "SELECT \"TXN_ID\" FROM \"TXNS\" WHERE " +
- "\"TXN_ID\" NOT IN (SELECT \"TC_TXNID\" FROM \"TXN_COMPONENTS\") AND " +
- " (\"TXN_STATE\" = " + TxnStatus.ABORTED + " OR \"TXN_STATE\" = " + TxnStatus.COMMITTED + ") AND "
- + " \"TXN_ID\" < " + lowWaterMark;
- LOG.debug("Going to execute query <{}>", s);
- rs = stmt.executeQuery(s);
- List<Long> txnids = new ArrayList<>();
- while (rs.next()) txnids.add(rs.getLong(1));
- close(rs);
- if(txnids.size() <= 0) {
- return;
- }
- Collections.sort(txnids);//easier to read logs
-
- List<String> queries = new ArrayList<>();
- StringBuilder prefix = new StringBuilder();
- StringBuilder suffix = new StringBuilder();
-
- // Delete from TXNS.
- prefix.append("DELETE FROM \"TXNS\" WHERE ");
-
- TxnUtils.buildQueryWithINClause(conf, queries, prefix, suffix, txnids, "\"TXN_ID\"", false, false);
-
- for (String query : queries) {
- LOG.debug("Going to execute update <{}>", query);
- int rc = stmt.executeUpdate(query);
+ try (Connection dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED, connPoolCompaction)) {
+ try (PreparedStatement stmt = dbConn.prepareStatement(DELETE_FAILED_TXNS_DIRECTLY_SQL)) {
+ //Aborted and committed are terminal states, so nothing about the txn can change
+ //after that, so READ COMMITTED is sufficient.
+ /*
+ * Only delete aborted / committed transaction in a way that guarantees two things:
+ * 1. never deletes anything that is inside the TXN_OPENTXN_TIMEOUT window
+ * 2. never deletes the maximum txnId even if it is before the TXN_OPENTXN_TIMEOUT window
+ */
+ long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
+ stmt.setLong(1, lowWaterMark);
+ LOG.debug("Going to execute query <{}>", DELETE_FAILED_TXNS_DIRECTLY_SQL);
+ int rc = stmt.executeUpdate();
LOG.debug("Removed {} empty Aborted and Committed transactions from TXNS", rc);
+ LOG.debug("Going to commit");
+ dbConn.commit();
+ } catch (SQLException e) {
+ LOG.error("Unable to delete from txns table " + e.getMessage());
+ LOG.debug("Going to rollback");
Review Comment:
what's the purpose of this log?
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4174:
URL: https://github.com/apache/hive/pull/4174#issuecomment-1554303562
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4174)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4174&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4174&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4174&metric=duplicated_lines_density&view=list) No Duplication information
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1183332209
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -87,6 +87,11 @@ class CompactionTxnHandler extends TxnHandler {
"DELETE FROM \"COMPACTION_METRICS_CACHE\" WHERE \"CMC_DATABASE\" = ? AND \"CMC_TABLE\" = ? " +
"AND \"CMC_METRIC_TYPE\" = ?";
+ private static final String DELETE_FAILED_TXNS_DIRECTLY_SQL =
Review Comment:
move it to TxnQueries
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4174: HIVE-27198: Delete aborted transactions directly instead of select and loading ids
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4174:
URL: https://github.com/apache/hive/pull/4174#discussion_r1182178182
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -888,35 +887,13 @@ public void cleanEmptyAbortedAndCommittedTxns() throws MetaException {
*/
long lowWaterMark = getOpenTxnTimeoutLowBoundaryTxnId(dbConn);
- String s = "SELECT \"TXN_ID\" FROM \"TXNS\" WHERE " +
+ String s = "DELETE FROM \"TXNS\" WHERE " +
Review Comment:
what do you mean? they only var here is `lowWaterMark` that should be passed to prepared statement
--
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-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org