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 @@ else if (output.getTyp() == WriteEntity.Type.DUMMYPARTITION) { 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 hiveLocks = lockMgr.lock(lockObjects, false); 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 HiveLockObjectData(String queryId, 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()); } /** 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 @@ private String getLockName(String parent, HiveLockMode mode) { 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 conflictingLocks = new HashSet(); 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 expectedLocks = new ArrayList(); + 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 lockObjsCaptor = ArgumentCaptor.forClass(List.class); + + when(mockQueryPlan.getInputs()).thenReturn(createReadEntities()); + when(mockQueryPlan.getOutputs()).thenReturn(new HashSet()); + + // Execute + txnMgr.acquireLocks(mockQueryPlan, ctx, "fred"); + + // Verify + Assert.assertEquals("db1", SessionState.get().getCurrentDatabase()); + List 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)lockObjsCaptor.capture(), eq(false)); + List lockObjs = (List)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 int compare(HiveLockObj lock1, HiveLockObj lock2) { Assert.assertEquals(name2, lockObj.getName()); Assert.assertEquals(HiveLockMode.SHARED, lockObj.getMode()); } + + private HashSet createReadEntities() { + HashSet readEntities = new HashSet(); + 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 partCols = new ArrayList(1); + partCols.add(fs); + t.setPartCols(partCols); + } + return t; + } } 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; 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.