Repository: hive Updated Branches: refs/heads/master ccc9bf3ea -> c3a9fa170
HIVE-12594 X lock on partition should not conflict with S lock on DB (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/c3a9fa17 Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/c3a9fa17 Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/c3a9fa17 Branch: refs/heads/master Commit: c3a9fa170377bf2d80dc6e6efa46187c6c5236b2 Parents: ccc9bf3 Author: Eugene Koifman <ekoif...@hortonworks.com> Authored: Thu Jan 5 13:28:46 2017 -0800 Committer: Eugene Koifman <ekoif...@hortonworks.com> Committed: Thu Jan 5 13:28:46 2017 -0800 ---------------------------------------------------------------------- .../hadoop/hive/metastore/txn/TxnHandler.java | 17 ++- .../hive/ql/lockmgr/TestDbTxnManager2.java | 151 ++++++++----------- 2 files changed, 80 insertions(+), 88 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/c3a9fa17/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 ea46d84..75a58c6 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 @@ -2114,6 +2114,9 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI { private boolean isTableLock() { return db != null && table != null && partition == null; } + private boolean isPartitionLock() { + return !(isDbLock() || isTableLock()); + } } private static class LockInfoComparator implements Comparator<LockInfo> { @@ -2603,15 +2606,25 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI { * the {@link #jumpTable} only deals with LockState/LockType. In some cases it's not * sufficient. For example, an EXCLUSIVE lock on partition should prevent SHARED_READ * on the table, but there is no reason for EXCLUSIVE on a table to prevent SHARED_READ - * on a database. + * on a database. Similarly, EXCLUSIVE on a partition should not conflict with SHARED_READ on + * a database. (SHARED_READ is usually acquired on a database to make sure it's not dropped + * while some operation is performed on that db (e.g. show tables, created table, etc) + * EXCLUSIVE on an object may mean it's being dropped or overwritten (for non-acid tables, + * an Insert uses EXCLUSIVE as well)). */ private boolean ignoreConflict(LockInfo desiredLock, LockInfo existingLock) { return ((desiredLock.isDbLock() && desiredLock.type == LockType.SHARED_READ && existingLock.isTableLock() && existingLock.type == LockType.EXCLUSIVE) || (existingLock.isDbLock() && existingLock.type == LockType.SHARED_READ && - desiredLock.isTableLock() && desiredLock.type == LockType.EXCLUSIVE)) + desiredLock.isTableLock() && desiredLock.type == LockType.EXCLUSIVE) || + + (desiredLock.isDbLock() && desiredLock.type == LockType.SHARED_READ && + existingLock.isPartitionLock() && existingLock.type == LockType.EXCLUSIVE) || + (existingLock.isDbLock() && existingLock.type == LockType.SHARED_READ && + desiredLock.isPartitionLock() && desiredLock.type == LockType.EXCLUSIVE)) || + //different locks from same txn should not conflict with each other (desiredLock.txnId != 0 && desiredLock.txnId == existingLock.txnId) || //txnId=0 means it's a select or IUD which does not write to ACID table, e.g http://git-wip-us.apache.org/repos/asf/hive/blob/c3a9fa17/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java ---------------------------------------------------------------------- diff --git a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java index 725558f..384f6eb 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java @@ -665,7 +665,7 @@ public class TestDbTxnManager2 { txnMgr.openTxn(ctx, "T1"); checkCmdOnDriver(driver.compileAndRespond("select * from tab_acid inner join tab_not_acid on a = na")); txnMgr.acquireLocks(driver.getPlan(), ctx, "T1"); - List<ShowLocksResponseElement> locks = getLocks(txnMgr, true); + List<ShowLocksResponseElement> locks = getLocks(txnMgr); Assert.assertEquals("Unexpected lock count", 6, locks.size()); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "tab_acid", null, locks); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "tab_acid", "p=bar", locks); @@ -678,7 +678,7 @@ public class TestDbTxnManager2 { txnMgr2.openTxn(ctx, "T2"); checkCmdOnDriver(driver.compileAndRespond("insert into tab_not_acid partition(np='doh') values(5,6)")); LockState ls = ((DbTxnManager)txnMgr2).acquireLocks(driver.getPlan(), ctx, "T2", false); - locks = getLocks(txnMgr2, true); + locks = getLocks(txnMgr2); Assert.assertEquals("Unexpected lock count", 7, locks.size()); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "tab_acid", null, locks); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "tab_acid", "p=bar", locks); @@ -809,85 +809,13 @@ public class TestDbTxnManager2 { return s == null ? null : s.toLowerCase(); } - /** - * @deprecated use {@link #getLocks(boolean)} - */ private List<ShowLocksResponseElement> getLocks() throws Exception { - return getLocks(false); - } - private List<ShowLocksResponseElement> getLocks(boolean sorted) throws Exception { - return getLocks(this.txnMgr, sorted); + return getLocks(txnMgr); } - /** - * @deprecated use {@link #getLocks(HiveTxnManager, boolean)} - */ private List<ShowLocksResponseElement> getLocks(HiveTxnManager txnMgr) throws Exception { - return getLocks(txnMgr, false); - } - - /** - * for some reason sort order of locks changes across different branches... - */ - private List<ShowLocksResponseElement> getLocks(HiveTxnManager txnMgr, boolean sorted) throws Exception { ShowLocksResponse rsp = ((DbLockManager)txnMgr.getLockManager()).getLocks(); - if(sorted) { - Collections.sort(rsp.getLocks(), new LockComparator()); - } return rsp.getLocks(); } - private static class LockComparator implements Comparator<ShowLocksResponseElement> { - @Override - public boolean equals(Object other) { - return other instanceof LockComparator; - } - //sort is not important except that it's consistent - @Override - public int compare(ShowLocksResponseElement p1, ShowLocksResponseElement p2) { - if(p1 == null && p2 == null) { - return 0; - } - if(p1 == null) { - return -1; - } - if(p2 == null) { - return 1; - } - int v = 0; - if((v = compare(p1.getDbname(), p2.getDbname())) != 0) { - return v; - } - if((v = compare(p1.getTablename(), p2.getTablename())) != 0) { - return v; - } - if((v = compare(p1.getPartname(), p2.getPartname())) != 0) { - return v; - } - if((v = p1.getType().getValue() - p2.getType().getValue()) != 0) { - return v; - } - if((v = p1.getState().getValue() - p2.getState().getValue()) != 0) { - return v; - } - //we should never get here (given current code) - if(p1.getLockid() == p2.getLockid()) { - return (int)(p1.getLockIdInternal() - p2.getLockIdInternal()); - } - return (int)(p1.getLockid() - p2.getLockid()); - } - private static int compare(String s1, String s2) { - if(s1 == null && s2 == null) { - return 0; - } - if(s1 == null) { - return -1; - } - if(s2 == null) { - return 1; - } - return s1.compareTo(s2); - } - } - /** * txns update same resource but do not overlap in time - no conflict */ @@ -1604,7 +1532,7 @@ public class TestDbTxnManager2 { "when matched and t.a in (3,7) then delete " + //deletes from p=1/q=2, p=2/q=2 "when not matched and t.a >= 8 then insert values(s.a, s.b, s.p, s.q)"));//insert p=1/q=2, p=1/q=3 and new part 1/1 txnMgr.acquireLocks(driver.getPlan(), ctx, "T1"); - List<ShowLocksResponseElement> locks = getLocks(txnMgr, true); + List<ShowLocksResponseElement> locks = getLocks(txnMgr); Assert.assertEquals("Unexpected lock count", 5, locks.size()); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "target", null, locks); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "source", null, locks); @@ -1620,7 +1548,7 @@ public class TestDbTxnManager2 { "when matched and t.a in (3,7) then delete " + //deletes from p=1/q=2, p=2/q=2 "when not matched and t.a >= 8 then insert values(s.a, s.b, s.p, s.q)"));//insert p=1/q=2, p=1/q=3 and new part 1/1 txnMgr2.acquireLocks(driver.getPlan(), ctx, "T1", false); - locks = getLocks(txnMgr2, true); + locks = getLocks(txnMgr2); Assert.assertEquals("Unexpected lock count", 10, locks.size()); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "target", null, locks); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "source", null, locks); @@ -1691,7 +1619,7 @@ public class TestDbTxnManager2 { //re-check locks which were in Waiting state - should now be Acquired ((DbLockManager)txnMgr2.getLockManager()).checkLock(extLockId); - locks = getLocks(txnMgr2, true); + locks = getLocks(txnMgr2); Assert.assertEquals("Unexpected lock count", 5, locks.size()); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "target", null, locks); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "source2", null, locks); @@ -1820,7 +1748,7 @@ public class TestDbTxnManager2 { 1,//no DP, so it's populated from lock info TxnDbUtil.countQueryAgent("select count(*) from TXN_COMPONENTS where tc_txnid=" + txnid1)); - List<ShowLocksResponseElement> locks = getLocks(txnMgr, true); + List<ShowLocksResponseElement> locks = getLocks(txnMgr); Assert.assertEquals("Unexpected lock count", 1, locks.size()); checkLock(causeConflict ? LockType.SHARED_WRITE : LockType.SHARED_READ, LockState.ACQUIRED, "default", "target", null, locks); @@ -1833,7 +1761,7 @@ public class TestDbTxnManager2 { "when matched then delete " + "when not matched then insert values(s.a,s.b)")); txnMgr2.acquireLocks(driver.getPlan(), ctx, "T2", false); - locks = getLocks(txnMgr, true); + locks = getLocks(txnMgr); Assert.assertEquals("Unexpected lock count", 3, locks.size()); checkLock(LockType.SHARED_WRITE, LockState.ACQUIRED, "default", "target", null, locks); @@ -1853,7 +1781,7 @@ public class TestDbTxnManager2 { //re-check locks which were in Waiting state - should now be Acquired ((DbLockManager)txnMgr2.getLockManager()).checkLock(extLockId); - locks = getLocks(txnMgr, true); + locks = getLocks(txnMgr); Assert.assertEquals("Unexpected lock count", 2, locks.size()); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "source", null, locks); checkLock(LockType.SHARED_WRITE, LockState.ACQUIRED, "default", "target", null, locks); @@ -1906,7 +1834,7 @@ public class TestDbTxnManager2 { long txnid1 = txnMgr.openTxn(ctx, "T1"); checkCmdOnDriver(driver.compileAndRespond("insert into target partition(p=1,q) values (1,2,2), (3,4,2), (5,6,3), (7,8,2)")); txnMgr.acquireLocks(driver.getPlan(), ctx, "T1"); - List<ShowLocksResponseElement> locks = getLocks(txnMgr, true); + List<ShowLocksResponseElement> locks = getLocks(txnMgr); Assert.assertEquals("Unexpected lock count", 1, locks.size()); //table is empty, so can only lock the table checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "target", null, locks); @@ -1938,7 +1866,7 @@ public class TestDbTxnManager2 { long txnid2 = txnMgr.openTxn(ctx, "T1"); checkCmdOnDriver(driver.compileAndRespond("insert into target partition(p=1,q) values (10,2,2), (30,4,2), (50,6,3), (70,8,2)")); txnMgr.acquireLocks(driver.getPlan(), ctx, "T1"); - locks = getLocks(txnMgr, true); + locks = getLocks(txnMgr); Assert.assertEquals("Unexpected lock count", 1, locks.size()); //Plan is using DummyPartition, so can only lock the table... unfortunately checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "target", null, locks); @@ -1977,7 +1905,7 @@ public class TestDbTxnManager2 { long txnId1 = txnMgr.openTxn(ctx, "T1"); checkCmdOnDriver(driver.compileAndRespond("update target set b = 2 where p=1")); txnMgr.acquireLocks(driver.getPlan(), ctx, "T1"); - List<ShowLocksResponseElement> locks = getLocks(txnMgr, true); + List<ShowLocksResponseElement> locks = getLocks(txnMgr); Assert.assertEquals("Unexpected lock count", 2, locks.size()); checkLock(LockType.SHARED_WRITE, LockState.ACQUIRED, "default", "target", "p=1/q=2", locks); checkLock(LockType.SHARED_WRITE, LockState.ACQUIRED, "default", "target", "p=1/q=3", locks); @@ -1990,7 +1918,7 @@ public class TestDbTxnManager2 { "when matched then update set b = 11 " + "when not matched then insert values(a1,b1,p1,q1)")); txnMgr2.acquireLocks(driver.getPlan(), ctx, "T2", false); - locks = getLocks(txnMgr, true); + locks = getLocks(txnMgr); Assert.assertEquals("Unexpected lock count", 7, locks.size()); /** * W locks from T1 are still there, so all locks from T2 block. @@ -2034,7 +1962,7 @@ public class TestDbTxnManager2 { //re-check locks which were in Waiting state - should now be Acquired ((DbLockManager)txnMgr2.getLockManager()).checkLock(extLockId); - locks = getLocks(txnMgr, true); + locks = getLocks(txnMgr); Assert.assertEquals("Unexpected lock count", 5, locks.size()); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "source", null, locks); checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "target", null, locks); @@ -2098,4 +2026,55 @@ public class TestDbTxnManager2 { TxnDbUtil.countQueryAgent("select count(*) from WRITE_SET where ws_txnid=" + txnid2)); } } + @Test + public void testShowTablesLock() throws Exception { + dropTable(new String[] {"T, T2"}); + CommandProcessorResponse cpr = driver.run( + "create table if not exists T (a int, b int)"); + checkCmdOnDriver(cpr); + + long txnid1 = txnMgr.openTxn(ctx, "Fifer"); + checkCmdOnDriver(driver.compileAndRespond("insert into T values(1,3)")); + txnMgr.acquireLocks(driver.getPlan(), ctx, "Fifer"); + List<ShowLocksResponseElement> locks = getLocks(); + Assert.assertEquals("Unexpected lock count", 1, locks.size()); + checkLock(LockType.EXCLUSIVE, LockState.ACQUIRED, "default", "t", null, locks); + + DbTxnManager txnMgr2 = (DbTxnManager) TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf); + checkCmdOnDriver(driver.compileAndRespond("show tables")); + txnMgr2.acquireLocks(driver.getPlan(), ctx, "Fidler"); + locks = getLocks(); + Assert.assertEquals("Unexpected lock count", 2, locks.size()); + checkLock(LockType.EXCLUSIVE, LockState.ACQUIRED, "default", "t", null, locks); + checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", null, null, locks); + txnMgr.commitTxn(); + txnMgr2.releaseLocks(txnMgr2.getLockManager().getLocks(false, false)); + Assert.assertEquals("Lock remained", 0, getLocks().size()); + Assert.assertEquals("Lock remained", 0, getLocks(txnMgr2).size()); + + + cpr = driver.run( + "create table if not exists T2 (a int, b int) partitioned by (p int) clustered by (a) " + + "into 2 buckets stored as orc TBLPROPERTIES ('transactional'='false')"); + checkCmdOnDriver(cpr); + + txnid1 = txnMgr.openTxn(ctx, "Fifer"); + checkCmdOnDriver(driver.compileAndRespond("insert into T2 partition(p=1) values(1,3)")); + txnMgr.acquireLocks(driver.getPlan(), ctx, "Fifer"); + locks = getLocks(); + Assert.assertEquals("Unexpected lock count", 1, locks.size()); + checkLock(LockType.EXCLUSIVE, LockState.ACQUIRED, "default", "t2", "p=1", locks); + + txnMgr2 = (DbTxnManager) TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf); + checkCmdOnDriver(driver.compileAndRespond("show tables")); + ((DbTxnManager)txnMgr2).acquireLocks(driver.getPlan(), ctx, "Fidler", false); + locks = getLocks(); + Assert.assertEquals("Unexpected lock count", 2, locks.size()); + checkLock(LockType.EXCLUSIVE, LockState.ACQUIRED, "default", "t2", "p=1", locks); + checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", null, null, locks); + txnMgr.commitTxn(); + txnMgr2.releaseLocks(txnMgr2.getLockManager().getLocks(false, false)); + Assert.assertEquals("Lock remained", 0, getLocks().size()); + Assert.assertEquals("Lock remained", 0, getLocks(txnMgr2).size()); + } }