You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/06/15 14:27:01 UTC
[GitHub] [hive] hmangla98 opened a new pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
hmangla98 opened a new pull request #2396:
URL: https://github.com/apache/hive/pull/2396
Fix the clean up of open repl created transactions
--
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:
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655105296
##########
File path: ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
##########
@@ -244,8 +245,16 @@ public void testAbortTxns() throws Exception {
List<Long> txnList = openedTxns.getTxn_ids();
txnHandler.abortTxns(new AbortTxnsRequest(txnList));
+ OpenTxnRequest replRqst = new OpenTxnRequest(2, "me", "localhost");
+ replRqst.setReplPolicy("default.*");
+ replRqst.setReplSrcTxnIds(Arrays.asList(1L, 2L));
+ List<Long> targetTxns = txnHandler.openTxns(replRqst).getTxn_ids();
+ txnHandler.abortTxns(new AbortTxnsRequest(targetTxns));
+
+ assertFalse(targetTxnsPresentInReplTxnMap(targetTxns));
Review comment:
This test is included in TestDbTxnManager.
--
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:
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r668449689
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java
##########
@@ -124,6 +124,13 @@ public int execute() {
try {
long loadTaskStartTime = System.currentTimeMillis();
SecurityUtils.reloginExpiringKeytabUser();
+ //Don't proceed if target db is replication incompatible.
+ if (work.dbNameToLoadIn != null) {
Review comment:
Do we know when could that be?
--
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655133158
##########
File path: ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
##########
@@ -244,8 +245,16 @@ public void testAbortTxns() throws Exception {
List<Long> txnList = openedTxns.getTxn_ids();
txnHandler.abortTxns(new AbortTxnsRequest(txnList));
+ OpenTxnRequest replRqst = new OpenTxnRequest(2, "me", "localhost");
+ replRqst.setReplPolicy("default.*");
+ replRqst.setReplSrcTxnIds(Arrays.asList(1L, 2L));
+ List<Long> targetTxns = txnHandler.openTxns(replRqst).getTxn_ids();
+ txnHandler.abortTxns(new AbortTxnsRequest(targetTxns));
+
+ assertFalse(targetTxnsPresentInReplTxnMap(targetTxns));
Review comment:
This test is included in TestDbTxnManager.
--
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:
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655604672
##########
File path: ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
##########
@@ -1690,6 +1701,19 @@ private void checkReplTxnForTest(Long startTxnId, Long endTxnId, String replPoli
}
}
+ private boolean targetTxnsPresentInReplTxnMap(Long startTxnId, Long endTxnId, List<Long> targetTxnId) throws Exception {
+ String[] output = TestTxnDbUtil.queryToString(conf, "SELECT \"RTM_TARGET_TXN_ID\" FROM \"REPL_TXN_MAP\" WHERE " +
+ " \"RTM_SRC_TXN_ID\" >= " + startTxnId + "AND \"RTM_SRC_TXN_ID\" <= " + endTxnId).split("\n");
+ List<Long> replayedTxns = new ArrayList<>();
+ for (int idx = 1; idx < output.length; idx++) {
+ Long txnId = Long.parseLong(output[idx].trim());
+ if (targetTxnId.contains(txnId)) {
Review comment:
Do you really need this check?
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
Review comment:
This isn't an info level log. We can remove it actually as the exception is being thrown and that would appear any way
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager.java
##########
@@ -516,10 +516,17 @@ public void testHeartbeaterReplicationTxn() throws Exception {
} catch (LockException e) {
exception = e;
}
- Assert.assertNotNull("Txn should have been aborted", exception);
- Assert.assertEquals(ErrorMsg.TXN_ABORTED, exception.getCanonicalErrorMsg());
+ Assert.assertNotNull("Source transaction with txnId: 1, missing from REPL_TXN_MAP", exception);
Review comment:
The message you get if the assertion fails. If this entry is missing, the exception object would be null. Is that expected?
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
Add target txn id as well.
--
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:
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655760894
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager.java
##########
@@ -516,10 +516,17 @@ public void testHeartbeaterReplicationTxn() throws Exception {
} catch (LockException e) {
exception = e;
}
- Assert.assertNotNull("Txn should have been aborted", exception);
- Assert.assertEquals(ErrorMsg.TXN_ABORTED, exception.getCanonicalErrorMsg());
+ Assert.assertNotNull("Source transaction with txnId: 1, missing from REPL_TXN_MAP", exception);
Review comment:
If this entry is missing, the exception would be thrown which will be caught up in line 517. So, e would be not null.
--
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:
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655105296
##########
File path: ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
##########
@@ -244,8 +245,16 @@ public void testAbortTxns() throws Exception {
List<Long> txnList = openedTxns.getTxn_ids();
txnHandler.abortTxns(new AbortTxnsRequest(txnList));
+ OpenTxnRequest replRqst = new OpenTxnRequest(2, "me", "localhost");
+ replRqst.setReplPolicy("default.*");
+ replRqst.setReplSrcTxnIds(Arrays.asList(1L, 2L));
+ List<Long> targetTxns = txnHandler.openTxns(replRqst).getTxn_ids();
+ txnHandler.abortTxns(new AbortTxnsRequest(targetTxns));
+
+ assertFalse(targetTxnsPresentInReplTxnMap(targetTxns));
Review comment:
This test is included in TestDbTxnManager.
##########
File path: ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
##########
@@ -244,8 +245,16 @@ public void testAbortTxns() throws Exception {
List<Long> txnList = openedTxns.getTxn_ids();
txnHandler.abortTxns(new AbortTxnsRequest(txnList));
+ OpenTxnRequest replRqst = new OpenTxnRequest(2, "me", "localhost");
+ replRqst.setReplPolicy("default.*");
+ replRqst.setReplSrcTxnIds(Arrays.asList(1L, 2L));
+ List<Long> targetTxns = txnHandler.openTxns(replRqst).getTxn_ids();
+ txnHandler.abortTxns(new AbortTxnsRequest(targetTxns));
+
+ assertFalse(targetTxnsPresentInReplTxnMap(targetTxns));
Review comment:
This test is included in TestDbTxnManager.
##########
File path: ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
##########
@@ -244,8 +245,16 @@ public void testAbortTxns() throws Exception {
List<Long> txnList = openedTxns.getTxn_ids();
txnHandler.abortTxns(new AbortTxnsRequest(txnList));
+ OpenTxnRequest replRqst = new OpenTxnRequest(2, "me", "localhost");
+ replRqst.setReplPolicy("default.*");
+ replRqst.setReplSrcTxnIds(Arrays.asList(1L, 2L));
+ List<Long> targetTxns = txnHandler.openTxns(replRqst).getTxn_ids();
+ txnHandler.abortTxns(new AbortTxnsRequest(targetTxns));
+
+ assertFalse(targetTxnsPresentInReplTxnMap(targetTxns));
Review comment:
This test is included in TestDbTxnManager.
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
target txn id is not present. that's the reason this exception is being thrown.
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager.java
##########
@@ -516,10 +516,17 @@ public void testHeartbeaterReplicationTxn() throws Exception {
} catch (LockException e) {
exception = e;
}
- Assert.assertNotNull("Txn should have been aborted", exception);
- Assert.assertEquals(ErrorMsg.TXN_ABORTED, exception.getCanonicalErrorMsg());
+ Assert.assertNotNull("Source transaction with txnId: 1, missing from REPL_TXN_MAP", exception);
Review comment:
If this entry is missing, the exception would be thrown which will be caught up in line 517. So, e would be not null.
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
no, we have only source txn id and the replPolicy. Using this info, we query the REPL_TXN_MAP table and get corresponding target txn id and then commit this txn.
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
abort txn txnId would not have replPolicy set. So, its execution would not come here.
--
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:
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r669272559
##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcrossInstances.java
##########
@@ -692,8 +693,9 @@ public void testBootStrapDumpOfWarehouse() throws Throwable {
.run("alter database default set dbproperties ('hive.repl.ckpt.key'='', 'repl.last.id'='')");
try {
replica.load("", "`*`");
- } catch (SemanticException e) {
- assertEquals("REPL LOAD * is not supported", e.getMessage());
+ assertTrue(false);
Review comment:
This will always fail. Why do you need to have such assertion?
--
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655838555
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
Aren't we aborting based on target txn id so we know which target txn id we are looking for?
--
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:
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] pkumarsinha merged pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha merged pull request #2396:
URL: https://github.com/apache/hive/pull/2396
--
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r668391907
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java
##########
@@ -124,6 +124,13 @@ public int execute() {
try {
long loadTaskStartTime = System.currentTimeMillis();
SecurityUtils.reloginExpiringKeytabUser();
+ //Don't proceed if target db is replication incompatible.
+ if (work.dbNameToLoadIn != null) {
+ Database targetDb = getHive().getDatabase(work.dbNameToLoadIn);
+ if (targetDb != null && MetaStoreUtils.isDbReplIncompatible(targetDb)) {
Review comment:
When would targetDb be null? When a wrong work.dbNameToLoadIn is specified ?
--
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655759104
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
target txn id is not present. that's the reason this exception is being thrown.
--
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:
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r669272781
##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcrossInstances.java
##########
@@ -692,8 +693,9 @@ public void testBootStrapDumpOfWarehouse() throws Throwable {
.run("alter database default set dbproperties ('hive.repl.ckpt.key'='', 'repl.last.id'='')");
try {
replica.load("", "`*`");
- } catch (SemanticException e) {
- assertEquals("REPL LOAD * is not supported", e.getMessage());
+ assertTrue(false);
Review comment:
assertEquals("REPL LOAD * is not supported", e.getMessage()); -
Do we not throw this error anymore
--
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r668421719
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java
##########
@@ -124,6 +124,13 @@ public int execute() {
try {
long loadTaskStartTime = System.currentTimeMillis();
SecurityUtils.reloginExpiringKeytabUser();
+ //Don't proceed if target db is replication incompatible.
+ if (work.dbNameToLoadIn != null) {
+ Database targetDb = getHive().getDatabase(work.dbNameToLoadIn);
+ if (targetDb != null && MetaStoreUtils.isDbReplIncompatible(targetDb)) {
Review comment:
for the first load operation (bootstrap load), targetDb would be null.
--
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r657888141
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1122,6 +1154,49 @@ public void abortTxns(AbortTxnsRequest rqst) throws MetaException {
}
}
+ private void markDbAsReplIncompatible(Connection dbConn, String database) throws SQLException, MetaException {
Review comment:
We needn't to have almost a copy of updateReplId(). If you need similar code for both externalize that part. I was also wondering if we would ever need and update.
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java
##########
@@ -124,6 +124,11 @@ public int execute() {
try {
long loadTaskStartTime = System.currentTimeMillis();
SecurityUtils.reloginExpiringKeytabUser();
+ //Don't proceed if target db is replication incompatible.
+ Database targetDb = getHive().getDatabase(work.dbNameToLoadIn);
+ if (targetDb != null && MetaStoreUtils.isDbReplIncompatible(targetDb)) {
+ throw new SemanticException(ErrorMsg.REPL_INCOMPATIBLE_EXCEPTION.getMsg());
Review comment:
Add DB name as well
##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcidTables.java
##########
@@ -139,6 +142,40 @@ public void tearDown() throws Throwable {
primary.run("drop database if exists " + primaryDbName + "_extra cascade");
}
+ @Test
+ public void testTargetDbReplIncompatible() throws Throwable {
+ HiveConf primaryConf = primary.getConf();
+ TxnStore txnHandler = TxnUtils.getTxnStore(primary.getConf());
+
+ primary.run("use " + primaryDbName)
+ .run("CREATE TABLE t1(a string) STORED AS TEXTFILE")
+ .dump(primaryDbName);
+ replica.load(replicatedDbName, primaryDbName);
+
+ assertFalse(MetaStoreUtils.isDbReplIncompatible(replica.getDatabase(replicatedDbName)));
+
+ Long sourceTxnId = openTxns(1, txnHandler, primaryConf).get(0);
+ txnHandler.abortTxn(new AbortTxnRequest(sourceTxnId));
+
+ sourceTxnId = openTxns(1, txnHandler, primaryConf).get(0);
+
+ primary.dump(primaryDbName);
+ replica.load(replicatedDbName, primaryDbName);
+ assertFalse(MetaStoreUtils.isDbReplIncompatible(replica.getDatabase(replicatedDbName)));
+
+ Long targetTxnId = txnHandler.getTargetTxnId(HiveUtils.getReplPolicy(replicatedDbName), sourceTxnId);
+ txnHandler.abortTxn(new AbortTxnRequest(targetTxnId));
+ assertTrue(MetaStoreUtils.isDbReplIncompatible(replica.getDatabase(replicatedDbName)));
+
+ WarehouseInstance.Tuple dumpData = primary.dump(primaryDbName);
+
+ assertFalse(ReplUtils.failedWithNonRecoverableError(new Path(dumpData.dumpLocation), conf));
+ replica.loadFailure(replicatedDbName, primaryDbName);
+ assertTrue(ReplUtils.failedWithNonRecoverableError(new Path(dumpData.dumpLocation), conf));
+
+ primary.dumpFailure(primaryDbName);
Review comment:
Check for this : assertTrue(ReplUtils.failedWithNonRecoverableError(new Path(dumpData.dumpLocation), conf));
event after dump failure
--
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:
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655842392
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
Didn't get this. Don't we do "abort txn <txnId>" here the txn id is of target and not of source, right?
--
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:
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r662736099
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1122,12 +1154,76 @@ public void abortTxns(AbortTxnsRequest rqst) throws MetaException {
}
}
+ private long getDatabaseId(Connection dbConn, String database, String catalog) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ try {
+ String query = "select \"DB_ID\" from \"DBS\" where \"NAME\" = ? and \"CTLG_NAME\" = ?";
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(database, catalog));
+ LOG.debug("Going to execute query <" + query.replaceAll("\\?", "{}") + ">",
+ quoteString(database), quoteString(catalog));
+ rs = pst.executeQuery();
+ if (!rs.next()) {
+ LOG.error("Database: " + database + " does not exist in catalog " + catalog);
+ return -1;
+ }
+ return rs.getLong(1);
+ } finally {
+ close(rs);
+ closeStmt(pst);
+ }
+ }
+
+ private void updateDatabaseProp(Connection dbConn, Statement stmt,
+ long dbId, String prop, String propValue) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ String query;
+ try {
+ rs = stmt.executeQuery("SELECT \"PARAM_VALUE\" FROM \"DATABASE_PARAMS\" WHERE \"PARAM_KEY\" = " +
+ "'" + prop + "' AND \"DB_ID\" = " + dbId);
+ if (!rs.next()) {
+ query = "INSERT INTO \"DATABASE_PARAMS\" VALUES ( " + dbId + " , '" + prop + "' , ? )";
+ } else {
+ query = "UPDATE \"DATABASE_PARAMS\" SET \"PARAM_VALUE\" = ? WHERE \"DB_ID\" = " + dbId +
+ " AND \"PARAM_KEY\" = '" + prop + "'";
+ }
+ close(rs);
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(propValue));
+ LOG.debug("Updating " + prop + " for db <" + query.replaceAll("\\?", "{}") + ">", propValue);
+ if (pst.executeUpdate() != 1) {
+ //only one row insert or update should happen
+ throw new RuntimeException("DATABASE_PARAMS is corrupted for databaseID: " + dbId);
+ }
+ } finally {
+ close(rs);
+ closeStmt(pst);
+ }
+ }
+
+ private void markDbAsReplIncompatible(Connection dbConn, String database) throws SQLException {
+ Statement stmt = dbConn.createStatement();
+ String catalog = MetaStoreUtils.getDefaultCatalog(conf);
+ String s = sqlGenerator.getDbProduct().getPrepareTxnStmt();
+ if (s != null) {
+ stmt.execute(s);
+ }
+ long dbId = getDatabaseId(dbConn, database, catalog);
+ if (dbId == -1) {
+ return;
+ }
+ updateDatabaseProp(dbConn, stmt, dbId, ReplConst.REPL_INCOMPATIBLE, ReplConst.TRUE);
+ closeStmt(stmt);
Review comment:
This is still prone to statement object leak.
--
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r668391491
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java
##########
@@ -124,6 +124,13 @@ public int execute() {
try {
long loadTaskStartTime = System.currentTimeMillis();
SecurityUtils.reloginExpiringKeytabUser();
+ //Don't proceed if target db is replication incompatible.
+ if (work.dbNameToLoadIn != null) {
Review comment:
When would this be null?
--
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r662738933
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1122,12 +1154,76 @@ public void abortTxns(AbortTxnsRequest rqst) throws MetaException {
}
}
+ private long getDatabaseId(Connection dbConn, String database, String catalog) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ try {
+ String query = "select \"DB_ID\" from \"DBS\" where \"NAME\" = ? and \"CTLG_NAME\" = ?";
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(database, catalog));
+ LOG.debug("Going to execute query <" + query.replaceAll("\\?", "{}") + ">",
+ quoteString(database), quoteString(catalog));
+ rs = pst.executeQuery();
+ if (!rs.next()) {
+ LOG.error("Database: " + database + " does not exist in catalog " + catalog);
+ return -1;
+ }
+ return rs.getLong(1);
+ } finally {
+ close(rs);
+ closeStmt(pst);
+ }
+ }
+
+ private void updateDatabaseProp(Connection dbConn, Statement stmt,
+ long dbId, String prop, String propValue) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ String query;
+ try {
+ rs = stmt.executeQuery("SELECT \"PARAM_VALUE\" FROM \"DATABASE_PARAMS\" WHERE \"PARAM_KEY\" = " +
+ "'" + prop + "' AND \"DB_ID\" = " + dbId);
+ if (!rs.next()) {
+ query = "INSERT INTO \"DATABASE_PARAMS\" VALUES ( " + dbId + " , '" + prop + "' , ? )";
+ } else {
+ query = "UPDATE \"DATABASE_PARAMS\" SET \"PARAM_VALUE\" = ? WHERE \"DB_ID\" = " + dbId +
Review comment:
If the value to be updated is same as the current one, we might skip the updation
--
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655604672
##########
File path: ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
##########
@@ -1690,6 +1701,19 @@ private void checkReplTxnForTest(Long startTxnId, Long endTxnId, String replPoli
}
}
+ private boolean targetTxnsPresentInReplTxnMap(Long startTxnId, Long endTxnId, List<Long> targetTxnId) throws Exception {
+ String[] output = TestTxnDbUtil.queryToString(conf, "SELECT \"RTM_TARGET_TXN_ID\" FROM \"REPL_TXN_MAP\" WHERE " +
+ " \"RTM_SRC_TXN_ID\" >= " + startTxnId + "AND \"RTM_SRC_TXN_ID\" <= " + endTxnId).split("\n");
+ List<Long> replayedTxns = new ArrayList<>();
+ for (int idx = 1; idx < output.length; idx++) {
+ Long txnId = Long.parseLong(output[idx].trim());
+ if (targetTxnId.contains(txnId)) {
Review comment:
Do you really need this check?
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
Review comment:
This isn't an info level log. We can remove it actually as the exception is being thrown and that would appear any way
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager.java
##########
@@ -516,10 +516,17 @@ public void testHeartbeaterReplicationTxn() throws Exception {
} catch (LockException e) {
exception = e;
}
- Assert.assertNotNull("Txn should have been aborted", exception);
- Assert.assertEquals(ErrorMsg.TXN_ABORTED, exception.getCanonicalErrorMsg());
+ Assert.assertNotNull("Source transaction with txnId: 1, missing from REPL_TXN_MAP", exception);
Review comment:
The message you get if the assertion fails. If this entry is missing, the exception object would be null. Is that expected?
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
Add target txn id as well.
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
Aren't we aborting based on target txn id so we know which target txn id we are looking for?
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
Didn't get this. Don't we do "abort txn <txnId>" here the txn id is of target and not of source, right?
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
Didn't get this. Don't we do "abort txn txnId" here the txn id is of target and not of source, right?
--
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:
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655842392
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
Didn't get this. Don't we do "abort txn txnId" here the txn id is of target and not of source, right?
--
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:
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r669277084
##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcrossInstances.java
##########
@@ -692,8 +693,9 @@ public void testBootStrapDumpOfWarehouse() throws Throwable {
.run("alter database default set dbproperties ('hive.repl.ckpt.key'='', 'repl.last.id'='')");
try {
replica.load("", "`*`");
- } catch (SemanticException e) {
- assertEquals("REPL LOAD * is not supported", e.getMessage());
+ assertTrue(false);
Review comment:
This particular error is outdated. Earlier, repl load did not raise any exception and hence catch clause was of no use. In this change, Repl load will throw an exception stating that dbname can't be null.
--
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655839436
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
no, we have only source txn id and the replPolicy. Using this info, we query the REPL_TXN_MAP table and get corresponding target txn id and then commit this txn.
--
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:
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r668454856
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java
##########
@@ -124,6 +124,13 @@ public int execute() {
try {
long loadTaskStartTime = System.currentTimeMillis();
SecurityUtils.reloginExpiringKeytabUser();
+ //Don't proceed if target db is replication incompatible.
+ if (work.dbNameToLoadIn != null) {
Review comment:
we have test cases to test REPL LOAD * scenarios.
--
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655847667
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1339,7 +1340,8 @@ public void commitTxn(CommitTxnRequest rqst)
// corresponding open txn event.
LOG.info("Target txn id is missing for source txn id : " + sourceTxnId +
" and repl policy " + rqst.getReplPolicy());
- return;
+ throw new NoSuchTxnException("Source transaction: " + JavaUtils.txnIdToString(sourceTxnId)
Review comment:
abort txn txnId would not have replPolicy set. So, its execution would not come here.
--
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:
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r663019158
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1122,12 +1154,79 @@ public void abortTxns(AbortTxnsRequest rqst) throws MetaException {
}
}
+ private long getDatabaseId(Connection dbConn, String database, String catalog) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ try {
+ String query = "select \"DB_ID\" from \"DBS\" where \"NAME\" = ? and \"CTLG_NAME\" = ?";
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(database, catalog));
+ LOG.debug("Going to execute query <" + query.replaceAll("\\?", "{}") + ">",
+ quoteString(database), quoteString(catalog));
+ rs = pst.executeQuery();
+ if (!rs.next()) {
+ LOG.error("Database: " + database + " does not exist in catalog " + catalog);
+ return -1;
+ }
+ return rs.getLong(1);
+ } finally {
+ close(rs);
+ closeStmt(pst);
+ }
+ }
+
+ private void updateDatabaseProp(Connection dbConn, long dbId, String prop, String propValue) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ try {
+ String query = "SELECT \"PARAM_VALUE\" FROM \"DATABASE_PARAMS\" WHERE \"PARAM_KEY\" = " +
+ "'" + prop + "' AND \"DB_ID\" = " + dbId;
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, null);
+ rs = pst.executeQuery();
+ query = null;
+ if (!rs.next()) {
+ query = "INSERT INTO \"DATABASE_PARAMS\" VALUES ( " + dbId + " , '" + prop + "' , ? )";
+ } else if (!rs.getString(1).equals(propValue)) {
+ query = "UPDATE \"DATABASE_PARAMS\" SET \"PARAM_VALUE\" = ? WHERE \"DB_ID\" = " + dbId +
+ " AND \"PARAM_KEY\" = '" + prop + "'";
+ }
+ closeStmt(pst);
+ if (query != null) {
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(propValue));
+ LOG.debug("Updating " + prop + " for db <" + query.replaceAll("\\?", "{}") + ">", propValue);
Review comment:
Db name would be helpful.
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1122,12 +1154,79 @@ public void abortTxns(AbortTxnsRequest rqst) throws MetaException {
}
}
+ private long getDatabaseId(Connection dbConn, String database, String catalog) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ try {
+ String query = "select \"DB_ID\" from \"DBS\" where \"NAME\" = ? and \"CTLG_NAME\" = ?";
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(database, catalog));
+ LOG.debug("Going to execute query <" + query.replaceAll("\\?", "{}") + ">",
+ quoteString(database), quoteString(catalog));
+ rs = pst.executeQuery();
+ if (!rs.next()) {
+ LOG.error("Database: " + database + " does not exist in catalog " + catalog);
+ return -1;
+ }
+ return rs.getLong(1);
+ } finally {
+ close(rs);
+ closeStmt(pst);
+ }
+ }
+
+ private void updateDatabaseProp(Connection dbConn, long dbId, String prop, String propValue) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ try {
+ String query = "SELECT \"PARAM_VALUE\" FROM \"DATABASE_PARAMS\" WHERE \"PARAM_KEY\" = " +
+ "'" + prop + "' AND \"DB_ID\" = " + dbId;
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, null);
+ rs = pst.executeQuery();
+ query = null;
+ if (!rs.next()) {
+ query = "INSERT INTO \"DATABASE_PARAMS\" VALUES ( " + dbId + " , '" + prop + "' , ? )";
+ } else if (!rs.getString(1).equals(propValue)) {
+ query = "UPDATE \"DATABASE_PARAMS\" SET \"PARAM_VALUE\" = ? WHERE \"DB_ID\" = " + dbId +
+ " AND \"PARAM_KEY\" = '" + prop + "'";
+ }
+ closeStmt(pst);
+ if (query != null) {
Review comment:
if query == null, having a log message should be useful.
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1143,37 +1242,13 @@ private void updateReplId(Connection dbConn, ReplLastIdInfo replLastIdInfo) thro
stmt.execute(s);
}
- String query = "select \"DB_ID\" from \"DBS\" where \"NAME\" = ? and \"CTLG_NAME\" = ?";
- List<String> params = Arrays.asList(db, catalog);
- pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, params);
- LOG.debug("Going to execute query <" + query.replaceAll("\\?", "{}") + ">",
- quoteString(db), quoteString(catalog));
- rs = pst.executeQuery();
- if (!rs.next()) {
- throw new MetaException("DB with name " + db + " does not exist in catalog " + catalog);
+ long dbId = getDatabaseId(dbConn, db, catalog);
+ if (dbId == -1) {
Review comment:
Why is this not an error?
--
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r668422316
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java
##########
@@ -124,6 +124,13 @@ public int execute() {
try {
long loadTaskStartTime = System.currentTimeMillis();
SecurityUtils.reloginExpiringKeytabUser();
+ //Don't proceed if target db is replication incompatible.
+ if (work.dbNameToLoadIn != null) {
Review comment:
we can have some situation in which work.dbNameToLoadIn is not set.
--
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655067789
##########
File path: ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
##########
@@ -244,8 +245,16 @@ public void testAbortTxns() throws Exception {
List<Long> txnList = openedTxns.getTxn_ids();
txnHandler.abortTxns(new AbortTxnsRequest(txnList));
+ OpenTxnRequest replRqst = new OpenTxnRequest(2, "me", "localhost");
+ replRqst.setReplPolicy("default.*");
+ replRqst.setReplSrcTxnIds(Arrays.asList(1L, 2L));
+ List<Long> targetTxns = txnHandler.openTxns(replRqst).getTxn_ids();
+ txnHandler.abortTxns(new AbortTxnsRequest(targetTxns));
+
+ assertFalse(targetTxnsPresentInReplTxnMap(targetTxns));
Review comment:
Test REPL_TXN_TIMEOUT case as well
##########
File path: ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
##########
@@ -244,8 +245,16 @@ public void testAbortTxns() throws Exception {
List<Long> txnList = openedTxns.getTxn_ids();
txnHandler.abortTxns(new AbortTxnsRequest(txnList));
+ OpenTxnRequest replRqst = new OpenTxnRequest(2, "me", "localhost");
+ replRqst.setReplPolicy("default.*");
+ replRqst.setReplSrcTxnIds(Arrays.asList(1L, 2L));
+ List<Long> targetTxns = txnHandler.openTxns(replRqst).getTxn_ids();
Review comment:
Assert that these are REPLAYED txns.
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -4714,6 +4714,12 @@ private int abortTxns(Connection dbConn, List<Long> txnids, boolean checkHeartbe
prefix.append("DELETE FROM \"HIVE_LOCKS\" WHERE ");
TxnUtils.buildQueryWithINClause(conf, queries, prefix, suffix, txnids, "\"HL_TXNID\"", false, false);
+ // Delete mapping from REPL_TXN_MAP if it exists.
+ prefix.setLength(0);
+ suffix.setLength(0);
+ prefix.append("DELETE FROM \"REPL_TXN_MAP\" WHERE ");
Review comment:
This is applicable only for the replayed transactions and not all.
--
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:
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] pkumarsinha commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r662736099
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1122,12 +1154,76 @@ public void abortTxns(AbortTxnsRequest rqst) throws MetaException {
}
}
+ private long getDatabaseId(Connection dbConn, String database, String catalog) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ try {
+ String query = "select \"DB_ID\" from \"DBS\" where \"NAME\" = ? and \"CTLG_NAME\" = ?";
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(database, catalog));
+ LOG.debug("Going to execute query <" + query.replaceAll("\\?", "{}") + ">",
+ quoteString(database), quoteString(catalog));
+ rs = pst.executeQuery();
+ if (!rs.next()) {
+ LOG.error("Database: " + database + " does not exist in catalog " + catalog);
+ return -1;
+ }
+ return rs.getLong(1);
+ } finally {
+ close(rs);
+ closeStmt(pst);
+ }
+ }
+
+ private void updateDatabaseProp(Connection dbConn, Statement stmt,
+ long dbId, String prop, String propValue) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ String query;
+ try {
+ rs = stmt.executeQuery("SELECT \"PARAM_VALUE\" FROM \"DATABASE_PARAMS\" WHERE \"PARAM_KEY\" = " +
+ "'" + prop + "' AND \"DB_ID\" = " + dbId);
+ if (!rs.next()) {
+ query = "INSERT INTO \"DATABASE_PARAMS\" VALUES ( " + dbId + " , '" + prop + "' , ? )";
+ } else {
+ query = "UPDATE \"DATABASE_PARAMS\" SET \"PARAM_VALUE\" = ? WHERE \"DB_ID\" = " + dbId +
+ " AND \"PARAM_KEY\" = '" + prop + "'";
+ }
+ close(rs);
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(propValue));
+ LOG.debug("Updating " + prop + " for db <" + query.replaceAll("\\?", "{}") + ">", propValue);
+ if (pst.executeUpdate() != 1) {
+ //only one row insert or update should happen
+ throw new RuntimeException("DATABASE_PARAMS is corrupted for databaseID: " + dbId);
+ }
+ } finally {
+ close(rs);
+ closeStmt(pst);
+ }
+ }
+
+ private void markDbAsReplIncompatible(Connection dbConn, String database) throws SQLException {
+ Statement stmt = dbConn.createStatement();
+ String catalog = MetaStoreUtils.getDefaultCatalog(conf);
+ String s = sqlGenerator.getDbProduct().getPrepareTxnStmt();
+ if (s != null) {
+ stmt.execute(s);
+ }
+ long dbId = getDatabaseId(dbConn, database, catalog);
+ if (dbId == -1) {
+ return;
+ }
+ updateDatabaseProp(dbConn, stmt, dbId, ReplConst.REPL_INCOMPATIBLE, ReplConst.TRUE);
+ closeStmt(stmt);
Review comment:
There is still prone to statement object leak.
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1122,12 +1154,76 @@ public void abortTxns(AbortTxnsRequest rqst) throws MetaException {
}
}
+ private long getDatabaseId(Connection dbConn, String database, String catalog) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ try {
+ String query = "select \"DB_ID\" from \"DBS\" where \"NAME\" = ? and \"CTLG_NAME\" = ?";
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(database, catalog));
+ LOG.debug("Going to execute query <" + query.replaceAll("\\?", "{}") + ">",
+ quoteString(database), quoteString(catalog));
+ rs = pst.executeQuery();
+ if (!rs.next()) {
+ LOG.error("Database: " + database + " does not exist in catalog " + catalog);
+ return -1;
+ }
+ return rs.getLong(1);
+ } finally {
+ close(rs);
+ closeStmt(pst);
+ }
+ }
+
+ private void updateDatabaseProp(Connection dbConn, Statement stmt,
+ long dbId, String prop, String propValue) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ String query;
+ try {
+ rs = stmt.executeQuery("SELECT \"PARAM_VALUE\" FROM \"DATABASE_PARAMS\" WHERE \"PARAM_KEY\" = " +
+ "'" + prop + "' AND \"DB_ID\" = " + dbId);
+ if (!rs.next()) {
+ query = "INSERT INTO \"DATABASE_PARAMS\" VALUES ( " + dbId + " , '" + prop + "' , ? )";
+ } else {
+ query = "UPDATE \"DATABASE_PARAMS\" SET \"PARAM_VALUE\" = ? WHERE \"DB_ID\" = " + dbId +
Review comment:
If the value to be updated is same as the current value, we might skip the updation
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -1122,12 +1154,76 @@ public void abortTxns(AbortTxnsRequest rqst) throws MetaException {
}
}
+ private long getDatabaseId(Connection dbConn, String database, String catalog) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ try {
+ String query = "select \"DB_ID\" from \"DBS\" where \"NAME\" = ? and \"CTLG_NAME\" = ?";
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(database, catalog));
+ LOG.debug("Going to execute query <" + query.replaceAll("\\?", "{}") + ">",
+ quoteString(database), quoteString(catalog));
+ rs = pst.executeQuery();
+ if (!rs.next()) {
+ LOG.error("Database: " + database + " does not exist in catalog " + catalog);
+ return -1;
+ }
+ return rs.getLong(1);
+ } finally {
+ close(rs);
+ closeStmt(pst);
+ }
+ }
+
+ private void updateDatabaseProp(Connection dbConn, Statement stmt,
+ long dbId, String prop, String propValue) throws SQLException {
+ ResultSet rs = null;
+ PreparedStatement pst = null;
+ String query;
+ try {
+ rs = stmt.executeQuery("SELECT \"PARAM_VALUE\" FROM \"DATABASE_PARAMS\" WHERE \"PARAM_KEY\" = " +
+ "'" + prop + "' AND \"DB_ID\" = " + dbId);
+ if (!rs.next()) {
+ query = "INSERT INTO \"DATABASE_PARAMS\" VALUES ( " + dbId + " , '" + prop + "' , ? )";
+ } else {
+ query = "UPDATE \"DATABASE_PARAMS\" SET \"PARAM_VALUE\" = ? WHERE \"DB_ID\" = " + dbId +
+ " AND \"PARAM_KEY\" = '" + prop + "'";
+ }
+ close(rs);
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, query, Arrays.asList(propValue));
+ LOG.debug("Updating " + prop + " for db <" + query.replaceAll("\\?", "{}") + ">", propValue);
+ if (pst.executeUpdate() != 1) {
+ //only one row insert or update should happen
+ throw new RuntimeException("DATABASE_PARAMS is corrupted for databaseID: " + dbId);
+ }
+ } finally {
+ close(rs);
+ closeStmt(pst);
+ }
+ }
+
+ private void markDbAsReplIncompatible(Connection dbConn, String database) throws SQLException {
+ Statement stmt = dbConn.createStatement();
+ String catalog = MetaStoreUtils.getDefaultCatalog(conf);
+ String s = sqlGenerator.getDbProduct().getPrepareTxnStmt();
+ if (s != null) {
+ stmt.execute(s);
+ }
+ long dbId = getDatabaseId(dbConn, database, catalog);
+ if (dbId == -1) {
Review comment:
If DB is not present, wouldn't that be an error case?
--
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r655105296
##########
File path: ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
##########
@@ -244,8 +245,16 @@ public void testAbortTxns() throws Exception {
List<Long> txnList = openedTxns.getTxn_ids();
txnHandler.abortTxns(new AbortTxnsRequest(txnList));
+ OpenTxnRequest replRqst = new OpenTxnRequest(2, "me", "localhost");
+ replRqst.setReplPolicy("default.*");
+ replRqst.setReplSrcTxnIds(Arrays.asList(1L, 2L));
+ List<Long> targetTxns = txnHandler.openTxns(replRqst).getTxn_ids();
+ txnHandler.abortTxns(new AbortTxnsRequest(targetTxns));
+
+ assertFalse(targetTxnsPresentInReplTxnMap(targetTxns));
Review comment:
This test is included in TestDbTxnManager.
--
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:
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] hmangla98 commented on a change in pull request #2396: HIVE-25246: Fix the clean up of open repl created transactions
Posted by GitBox <gi...@apache.org>.
hmangla98 commented on a change in pull request #2396:
URL: https://github.com/apache/hive/pull/2396#discussion_r668454856
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java
##########
@@ -124,6 +124,13 @@ public int execute() {
try {
long loadTaskStartTime = System.currentTimeMillis();
SecurityUtils.reloginExpiringKeytabUser();
+ //Don't proceed if target db is replication incompatible.
+ if (work.dbNameToLoadIn != null) {
Review comment:
we have some cases which to test REPL LOAD * scenarios.
--
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