HIVE-12504 TxnHandler.abortTxn() should check if already aborted to improve message (Eugene Koifman, reviewed by Wei Zheng)
Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/2a24612b Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/2a24612b Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/2a24612b Branch: refs/heads/master Commit: 2a24612befbba2849f429653b6bf8de7461d2874 Parents: 252dd7e Author: Eugene Koifman <[email protected]> Authored: Tue Dec 6 16:28:16 2016 -0800 Committer: Eugene Koifman <[email protected]> Committed: Tue Dec 6 16:28:16 2016 -0800 ---------------------------------------------------------------------- .../hadoop/hive/metastore/txn/TxnHandler.java | 9 ++++-- .../hadoop/hive/metastore/txn/TxnStore.java | 2 +- .../hadoop/hive/ql/lockmgr/DbTxnManager.java | 2 ++ .../hive/metastore/txn/TestTxnHandler.java | 31 ++++++++++++++++++++ .../hive/ql/lockmgr/TestDbTxnManager.java | 2 +- 5 files changed, 41 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/2a24612b/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java ---------------------------------------------------------------------- diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java b/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java index 0c495ef..ea46d84 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java @@ -33,6 +33,7 @@ import org.apache.hadoop.hive.common.classification.InterfaceStability; import org.apache.hadoop.hive.metastore.DatabaseProduct; import org.apache.hadoop.hive.metastore.HouseKeeperService; import org.apache.hadoop.hive.metastore.Warehouse; +import org.apache.thrift.TException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.commons.dbcp.PoolingDataSource; @@ -515,17 +516,19 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI { } } - public void abortTxn(AbortTxnRequest rqst) throws NoSuchTxnException, MetaException { + public void abortTxn(AbortTxnRequest rqst) throws NoSuchTxnException, MetaException, TxnAbortedException { long txnid = rqst.getTxnid(); try { Connection dbConn = null; + Statement stmt = null; try { lockInternal(); dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED); if (abortTxns(dbConn, Collections.singletonList(txnid), true) != 1) { LOG.debug("Going to rollback"); dbConn.rollback(); - throw new NoSuchTxnException("No such transaction " + JavaUtils.txnIdToString(txnid)); + stmt = dbConn.createStatement(); + ensureValidTxn(dbConn, txnid, stmt); } LOG.debug("Going to commit"); @@ -537,7 +540,7 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI { throw new MetaException("Unable to update transaction database " + StringUtils.stringifyException(e)); } finally { - closeDbConn(dbConn); + close(null, stmt, dbConn); unlockInternal(); } } catch (RetryException e) { http://git-wip-us.apache.org/repos/asf/hive/blob/2a24612b/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java ---------------------------------------------------------------------- diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java index 170280e..879ae55 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java @@ -97,7 +97,7 @@ public interface TxnStore { * @throws NoSuchTxnException * @throws MetaException */ - public void abortTxn(AbortTxnRequest rqst) throws NoSuchTxnException, MetaException; + public void abortTxn(AbortTxnRequest rqst) throws NoSuchTxnException, MetaException, TxnAbortedException; /** * Abort (rollback) a list of transactions in one request. http://git-wip-us.apache.org/repos/asf/hive/blob/2a24612b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java index 867e445..203eae5 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java @@ -406,6 +406,8 @@ public class DbTxnManager extends HiveTxnManagerImpl { } catch (NoSuchTxnException e) { LOG.error("Metastore could not find " + JavaUtils.txnIdToString(txnId)); throw new LockException(e, ErrorMsg.TXN_NO_SUCH_TRANSACTION, JavaUtils.txnIdToString(txnId)); + } catch(TxnAbortedException e) { + throw new LockException(e, ErrorMsg.TXN_ABORTED, JavaUtils.txnIdToString(txnId)); } catch (TException e) { throw new LockException(ErrorMsg.METASTORE_COMMUNICATION_FAILED.getMsg(), e); http://git-wip-us.apache.org/repos/asf/hive/blob/2a24612b/ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java ---------------------------------------------------------------------- diff --git a/ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java b/ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java index dbe1ce8..11cedb9 100644 --- a/ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java +++ b/ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.hive.metastore.txn; +import org.apache.hadoop.hive.common.JavaUtils; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.api.AbortTxnRequest; import org.apache.hadoop.hive.metastore.api.CheckLockRequest; @@ -162,6 +163,36 @@ public class TestTxnHandler { saw[tid.intValue()] = true; } for (int i = 1; i < saw.length; i++) assertTrue(saw[i]); + txnHandler.commitTxn(new CommitTxnRequest(2)); + boolean gotException = false; + try { + txnHandler.abortTxn(new AbortTxnRequest(1)); + } + catch(TxnAbortedException ex) { + gotException = true; + Assert.assertEquals("Transaction " + JavaUtils.txnIdToString(1) + " already aborted", ex.getMessage()); + } + Assert.assertTrue(gotException); + gotException = false; + try { + txnHandler.abortTxn(new AbortTxnRequest(2)); + } + catch(NoSuchTxnException ex) { + gotException = true; + //if this wasn't an empty txn, we'd get a better msg + //Assert.assertEquals("Transaction " + JavaUtils.txnIdToString(2) + " already committed.", ex.getMessage()); + Assert.assertEquals("No such transaction " + JavaUtils.txnIdToString(2), ex.getMessage()); + } + Assert.assertTrue(gotException); + gotException = false; + try { + txnHandler.abortTxn(new AbortTxnRequest(3)); + } + catch(NoSuchTxnException ex) { + gotException = true; + Assert.assertEquals("No such transaction " + JavaUtils.txnIdToString(3), ex.getMessage()); + } + Assert.assertTrue(gotException); } @Test http://git-wip-us.apache.org/repos/asf/hive/blob/2a24612b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager.java ---------------------------------------------------------------------- diff --git a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager.java b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager.java index 30d7b94..460bad5 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager.java @@ -237,7 +237,7 @@ public class TestDbTxnManager { exception = ex; } Assert.assertNotNull("Expected exception2", exception); - Assert.assertEquals("Wrong Exception2", ErrorMsg.TXN_NO_SUCH_TRANSACTION, exception.getCanonicalErrorMsg()); + Assert.assertEquals("Wrong Exception2", ErrorMsg.TXN_ABORTED, exception.getCanonicalErrorMsg()); } @Test
