hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ct...@apache.org
Subject hive git commit: HIVE-10972: DummyTxnManager always locks the current database in shared mode, which is incorrect (Aihua Xu via Chaoyu Tang, reviewd by Alan Gates)
Date Sat, 20 Jun 2015 20:28:54 GMT
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.ma@gmail.com>
Authored: Sat Jun 20 16:23:00 2015 -0400
Committer: ctang <ctang.ma@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.


Mime
View raw message