Repository: hive Updated Branches: refs/heads/master b97303cdd -> e09e14a48
HIVE-10972: DummyTxnManager always locks the current database in shared mode, which is incorrect (Aihua Xu via Chaoyu Tang, reviewd by Alan Gates) Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/e09e14a4 Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/e09e14a4 Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/e09e14a4 Branch: refs/heads/master Commit: e09e14a484150947a4286276b2855293be6fb2fa Parents: b97303c Author: ctang <ctang...@gmail.com> Authored: Sat Jun 20 16:23:00 2015 -0400 Committer: ctang <ctang...@gmail.com> Committed: Sat Jun 20 16:23:00 2015 -0400 ---------------------------------------------------------------------- .../hadoop/hive/ql/lockmgr/DummyTxnManager.java | 15 --- .../hadoop/hive/ql/lockmgr/HiveLockObject.java | 2 +- .../zookeeper/ZooKeeperHiveLockManager.java | 2 + .../hive/ql/lockmgr/TestDummyTxnManager.java | 119 ++++++++++++++++++- .../clientnegative/lockneg_try_lock_db_in_use.q | 2 +- .../lockneg_try_lock_db_in_use.q.out | 9 +- 6 files changed, 126 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/e09e14a4/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java index ea04415..21ab8ee 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java @@ -157,21 +157,6 @@ class DummyTxnManager extends HiveTxnManagerImpl { return; } - HiveLockObject.HiveLockObjectData lockData = - new HiveLockObject.HiveLockObjectData(plan.getQueryId(), - String.valueOf(System.currentTimeMillis()), - "IMPLICIT", - plan.getQueryStr()); - - // Lock the database also - String currentDb = SessionState.get().getCurrentDatabase(); - lockObjects.add( - new HiveLockObj( - new HiveLockObject(currentDb, lockData), - HiveLockMode.SHARED - ) - ); - dedupLockObjects(lockObjects); List<HiveLock> hiveLocks = lockMgr.lock(lockObjects, false); http://git-wip-us.apache.org/repos/asf/hive/blob/e09e14a4/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockObject.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockObject.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockObject.java index e75a90a..7e93387 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockObject.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockObject.java @@ -51,7 +51,7 @@ public class HiveLockObject { this.queryId = removeDelimiter(queryId); this.lockTime = removeDelimiter(lockTime); this.lockMode = removeDelimiter(lockMode); - this.queryStr = removeDelimiter(queryStr.trim()); + this.queryStr = removeDelimiter(queryStr == null ? null : queryStr.trim()); } /** http://git-wip-us.apache.org/repos/asf/hive/blob/e09e14a4/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java index 4f86dd9..fb954d8 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java @@ -255,6 +255,8 @@ public class ZooKeeperHiveLockManager implements HiveLockManager { private ZooKeeperHiveLock lock (HiveLockObject key, HiveLockMode mode, boolean keepAlive, boolean parentCreated) throws LockException { + LOG.info("Acquiring lock for " + key.getName() + " with mode " + key.getData().getLockMode()); + int tryNum = 0; ZooKeeperHiveLock ret = null; Set<String> conflictingLocks = new HashSet<String>(); http://git-wip-us.apache.org/repos/asf/hive/blob/e09e14a4/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDummyTxnManager.java ---------------------------------------------------------------------- diff --git a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDummyTxnManager.java b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDummyTxnManager.java index 5abb729..19f82ad 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDummyTxnManager.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDummyTxnManager.java @@ -18,16 +18,112 @@ package org.apache.hadoop.hive.ql.lockmgr; -import junit.framework.Assert; +import static org.mockito.Mockito.*; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.metastore.api.FieldSchema; +import org.apache.hadoop.hive.ql.Context; +import org.apache.hadoop.hive.ql.QueryPlan; +import org.apache.hadoop.hive.ql.hooks.ReadEntity; +import org.apache.hadoop.hive.ql.hooks.WriteEntity; import org.apache.hadoop.hive.ql.lockmgr.HiveLockObject.HiveLockObjectData; +import org.apache.hadoop.hive.ql.lockmgr.zookeeper.ZooKeeperHiveLock; +import org.apache.hadoop.hive.ql.metadata.Table; +import org.apache.hadoop.hive.ql.session.SessionState; +import org.apache.log4j.Level; +import org.apache.log4j.LogManager; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; import java.util.List; +@RunWith(MockitoJUnitRunner.class) public class TestDummyTxnManager { + private HiveConf conf = new HiveConf(); + private HiveTxnManager txnMgr; + private Context ctx; + private int nextInput = 1; + + @Mock + HiveLockManager mockLockManager; + + @Mock + QueryPlan mockQueryPlan; + + @Before + public void setUp() throws Exception { + conf.setBoolVar(HiveConf.ConfVars.HIVE_SUPPORT_CONCURRENCY, true); + conf.setVar(HiveConf.ConfVars.HIVE_TXN_MANAGER, DummyTxnManager.class.getName()); + SessionState.start(conf); + ctx = new Context(conf); + LogManager.getRootLogger().setLevel(Level.DEBUG); + + txnMgr = TxnManagerFactory.getTxnManagerFactory().getTxnManager(conf); + Assert.assertTrue(txnMgr instanceof DummyTxnManager); + // Use reflection to set LockManager since creating the object using the + // relection in DummyTxnManager won't take Mocked object + Field field = DummyTxnManager.class.getDeclaredField("lockMgr"); + field.setAccessible(true); + field.set(txnMgr, mockLockManager); + } + + @After + public void tearDown() throws Exception { + if (txnMgr != null) txnMgr.closeTxnManager(); + } + + /** + * Verifies the current database object is not locked if the table read is against different database + * @throws Exception + */ + @Test + public void testSingleReadTable() throws Exception { + // Setup + SessionState.get().setCurrentDatabase("db1"); + + List<HiveLock> expectedLocks = new ArrayList<HiveLock>(); + expectedLocks.add(new ZooKeeperHiveLock("default", new HiveLockObject(), HiveLockMode.SHARED)); + expectedLocks.add(new ZooKeeperHiveLock("default.table1", new HiveLockObject(), HiveLockMode.SHARED)); + + when(mockLockManager.lock(anyListOf(HiveLockObj.class), eq(false))).thenReturn(expectedLocks); + doNothing().when(mockLockManager).setContext(any(HiveLockManagerCtx.class)); + doNothing().when(mockLockManager).close(); + ArgumentCaptor<List> lockObjsCaptor = ArgumentCaptor.forClass(List.class); + + when(mockQueryPlan.getInputs()).thenReturn(createReadEntities()); + when(mockQueryPlan.getOutputs()).thenReturn(new HashSet<WriteEntity>()); + + // Execute + txnMgr.acquireLocks(mockQueryPlan, ctx, "fred"); + + // Verify + Assert.assertEquals("db1", SessionState.get().getCurrentDatabase()); + List<HiveLock> resultLocks = ctx.getHiveLocks(); + Assert.assertEquals(expectedLocks.size(), resultLocks.size()); + Assert.assertEquals(expectedLocks.get(0).getHiveLockMode(), resultLocks.get(0).getHiveLockMode()); + Assert.assertEquals(expectedLocks.get(0).getHiveLockObject().getName(), resultLocks.get(0).getHiveLockObject().getName()); + Assert.assertEquals(expectedLocks.get(1).getHiveLockMode(), resultLocks.get(1).getHiveLockMode()); + Assert.assertEquals(expectedLocks.get(0).getHiveLockObject().getName(), resultLocks.get(0).getHiveLockObject().getName()); + + verify(mockLockManager).lock((List<HiveLockObj>)lockObjsCaptor.capture(), eq(false)); + List<HiveLockObj> lockObjs = (List<HiveLockObj>)lockObjsCaptor.getValue(); + Assert.assertEquals(2, lockObjs.size()); + Assert.assertEquals("default", lockObjs.get(0).getName()); + Assert.assertEquals(HiveLockMode.SHARED, lockObjs.get(0).mode); + Assert.assertEquals("default/table1", lockObjs.get(1).getName()); + Assert.assertEquals(HiveLockMode.SHARED, lockObjs.get(1).mode); + } @Test public void testDedupLockObjects() { @@ -75,4 +171,25 @@ public class TestDummyTxnManager { Assert.assertEquals(name2, lockObj.getName()); Assert.assertEquals(HiveLockMode.SHARED, lockObj.getMode()); } + + private HashSet<ReadEntity> createReadEntities() { + HashSet<ReadEntity> readEntities = new HashSet<ReadEntity>(); + ReadEntity re = new ReadEntity(newTable(false)); + readEntities.add(re); + + return readEntities; + } + + private Table newTable(boolean isPartitioned) { + Table t = new Table("default", "table" + Integer.toString(nextInput++)); + if (isPartitioned) { + FieldSchema fs = new FieldSchema(); + fs.setName("version"); + fs.setType("String"); + List<FieldSchema> partCols = new ArrayList<FieldSchema>(1); + partCols.add(fs); + t.setPartCols(partCols); + } + return t; + } } http://git-wip-us.apache.org/repos/asf/hive/blob/e09e14a4/ql/src/test/queries/clientnegative/lockneg_try_lock_db_in_use.q ---------------------------------------------------------------------- diff --git a/ql/src/test/queries/clientnegative/lockneg_try_lock_db_in_use.q b/ql/src/test/queries/clientnegative/lockneg_try_lock_db_in_use.q index 4127a6f..85bd425 100644 --- a/ql/src/test/queries/clientnegative/lockneg_try_lock_db_in_use.q +++ b/ql/src/test/queries/clientnegative/lockneg_try_lock_db_in_use.q @@ -8,7 +8,7 @@ create table tstsrcpart like default.srcpart; insert overwrite table tstsrcpart partition (ds='2008-04-08', hr='11') select key, value from default.srcpart where ds='2008-04-08' and hr='11'; -lock table tstsrcpart shared; +lock database lockneg2 shared; show locks; lock database lockneg2 exclusive; http://git-wip-us.apache.org/repos/asf/hive/blob/e09e14a4/ql/src/test/results/clientnegative/lockneg_try_lock_db_in_use.q.out ---------------------------------------------------------------------- diff --git a/ql/src/test/results/clientnegative/lockneg_try_lock_db_in_use.q.out b/ql/src/test/results/clientnegative/lockneg_try_lock_db_in_use.q.out index 97ab37a..5486151 100644 --- a/ql/src/test/results/clientnegative/lockneg_try_lock_db_in_use.q.out +++ b/ql/src/test/results/clientnegative/lockneg_try_lock_db_in_use.q.out @@ -32,15 +32,14 @@ POSTHOOK: Input: default@srcpart@ds=2008-04-08/hr=11 POSTHOOK: Output: lockneg2@tstsrcpart@ds=2008-04-08/hr=11 POSTHOOK: Lineage: tstsrcpart PARTITION(ds=2008-04-08,hr=11).key SIMPLE [(srcpart)srcpart.FieldSchema(name:key, type:string, comment:default), ] POSTHOOK: Lineage: tstsrcpart PARTITION(ds=2008-04-08,hr=11).value SIMPLE [(srcpart)srcpart.FieldSchema(name:value, type:string, comment:default), ] -PREHOOK: query: lock table tstsrcpart shared -PREHOOK: type: LOCKTABLE -POSTHOOK: query: lock table tstsrcpart shared -POSTHOOK: type: LOCKTABLE +PREHOOK: query: lock database lockneg2 shared +PREHOOK: type: LOCKDATABASE +POSTHOOK: query: lock database lockneg2 shared +POSTHOOK: type: LOCKDATABASE PREHOOK: query: show locks PREHOOK: type: SHOWLOCKS POSTHOOK: query: show locks POSTHOOK: type: SHOWLOCKS -lockneg2@tstsrcpart SHARED PREHOOK: query: lock database lockneg2 exclusive PREHOOK: type: LOCKDATABASE Unable to acquire EXPLICIT, EXCLUSIVE lock lockneg2 after 1 attempts.
