You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by ek...@apache.org on 2016/03/30 21:17:28 UTC

hive git commit: HIVE-10249 ACID: show locks should show who the lock is waiting for (Eugene Koifman, reviewed by Wei Zheng)

Repository: hive
Updated Branches:
  refs/heads/master 51efcb80e -> 4e9f95a1b


HIVE-10249 ACID: show locks should show who the lock is waiting for (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/4e9f95a1
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/4e9f95a1
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/4e9f95a1

Branch: refs/heads/master
Commit: 4e9f95a1bad89ac4ea0cefc65eeba7a1e56a948d
Parents: 51efcb8
Author: Eugene Koifman <ek...@hortonworks.com>
Authored: Wed Mar 30 12:17:06 2016 -0700
Committer: Eugene Koifman <ek...@hortonworks.com>
Committed: Wed Mar 30 12:17:06 2016 -0700

----------------------------------------------------------------------
 .../hadoop/hive/metastore/txn/TxnDbUtil.java    |  6 ++-
 .../hadoop/hive/metastore/txn/TxnHandler.java   | 46 ++++++++++++++++----
 .../hive/metastore/txn/TestTxnHandler.java      |  2 +-
 .../org/apache/hadoop/hive/ql/exec/DDLTask.java | 16 ++++++-
 .../hive/ql/lockmgr/TestDbTxnManager2.java      | 28 ++++++++++++
 .../clientpositive/dbtxnmgr_showlocks.q.out     |  6 +--
 6 files changed, 89 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/4e9f95a1/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
----------------------------------------------------------------------
diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java b/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
index df480ea..c82d23a 100644
--- a/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
+++ b/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
@@ -103,7 +103,11 @@ public final class TxnDbUtil {
           " HL_ACQUIRED_AT bigint," +
           " HL_USER varchar(128) NOT NULL," +
           " HL_HOST varchar(128) NOT NULL," +
-          " PRIMARY KEY(HL_LOCK_EXT_ID, HL_LOCK_INT_ID))");
+          " HL_HEARTBEAT_COUNT integer," +
+          " HL_AGENT_INFO varchar(128)," +
+          " HL_BLOCKEDBY_EXT_ID bigint," +
+          " HL_BLOCKEDBY_INT_ID bigint," +
+        " PRIMARY KEY(HL_LOCK_EXT_ID, HL_LOCK_INT_ID))");
       stmt.execute("CREATE INDEX HL_TXNID_INDEX ON HIVE_LOCKS (HL_TXNID)");
 
       stmt.execute("CREATE TABLE NEXT_LOCK_ID (" + " NL_NEXT bigint NOT NULL)");

http://git-wip-us.apache.org/repos/asf/hive/blob/4e9f95a1/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 21faff4..be3c6de 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
@@ -847,8 +847,8 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI {
    */
   private static class LockInfoExt extends LockInfo {
     private final ShowLocksResponseElement e;
-    LockInfoExt(ShowLocksResponseElement e, long intLockId) {
-      super(e, intLockId);
+    LockInfoExt(ShowLocksResponseElement e) {
+      super(e);
       this.e = e;
     }
   }
@@ -864,7 +864,8 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI {
         stmt = dbConn.createStatement();
 
         String s = "select hl_lock_ext_id, hl_txnid, hl_db, hl_table, hl_partition, hl_lock_state, " +
-          "hl_lock_type, hl_last_heartbeat, hl_acquired_at, hl_user, hl_host, hl_lock_int_id from HIVE_LOCKS";
+          "hl_lock_type, hl_last_heartbeat, hl_acquired_at, hl_user, hl_host, hl_lock_int_id," +
+          "hl_blockedby_ext_id, hl_blockedby_int_id from HIVE_LOCKS";
         LOG.debug("Doing to execute query <" + s + ">");
         ResultSet rs = stmt.executeQuery(s);
         while (rs.next()) {
@@ -892,7 +893,16 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI {
           if (!rs.wasNull()) e.setAcquiredat(acquiredAt);
           e.setUser(rs.getString(10));
           e.setHostname(rs.getString(11));
-          sortedList.add(new LockInfoExt(e, rs.getLong(12)));
+          e.setLockIdInternal(rs.getLong(12));
+          long id = rs.getLong(13);
+          if(!rs.wasNull()) {
+            e.setBlockedByExtId(id);
+          }
+          id = rs.getLong(14);
+          if(!rs.wasNull()) {
+            e.setBlockedByIntId(id);
+          }
+          sortedList.add(new LockInfoExt(e));
         }
         LOG.debug("Going to rollback");
         dbConn.rollback();
@@ -1142,6 +1152,10 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI {
   private static void shouldNeverHappen(long txnid) {
     throw new RuntimeException("This should never happen: " + JavaUtils.txnIdToString(txnid));
   }
+  private static void shouldNeverHappen(long txnid, long extLockId, long intLockId) {
+    throw new RuntimeException("This should never happen: " + JavaUtils.txnIdToString(txnid) + " "
+      + JavaUtils.lockIdToString(extLockId) + " " + intLockId);
+  }
   public void addDynamicPartitions(AddDynamicPartitions rqst)
       throws NoSuchTxnException,  TxnAbortedException, MetaException {
     Connection dbConn = null;
@@ -1673,15 +1687,15 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI {
       }
       txnId = rs.getLong("hl_txnid");//returns 0 if value is NULL
     }
-    LockInfo(ShowLocksResponseElement e, long intLockId) {
+    LockInfo(ShowLocksResponseElement e) {
       extLockId = e.getLockid();
-      this.intLockId = intLockId;
+      intLockId = e.getLockIdInternal();
+      txnId = e.getTxnid();
       db = e.getDbname();
       table = e.getTablename();
       partition = e.getPartname();
       state = e.getState();
       type = e.getType();
-      txnId = e.getTxnid();
     }
 
     public boolean equals(Object other) {
@@ -1980,9 +1994,10 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI {
 
     LOG.debug("Going to execute query <" + query.toString() + ">");
     Statement stmt = null;
+    ResultSet rs = null;
     try {
       stmt = dbConn.createStatement();
-      ResultSet rs = stmt.executeQuery(query.toString());
+      rs = stmt.executeQuery(query.toString());
       SortedSet<LockInfo> lockSet = new TreeSet<LockInfo>(new LockInfoComparator());
       while (rs.next()) {
         lockSet.add(new LockInfo(rs));
@@ -2053,7 +2068,20 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI {
           switch (lockAction) {
             case WAIT:
               if(!ignoreConflict(info, locks[i])) {
+                /*we acquire all locks for a given query atomically; if 1 blocks, all go into (remain) in
+                * Waiting state.  wait() will undo any 'acquire()' which may have happened as part of
+                * this (metastore db) transaction and then we record which lock blocked the lock
+                * we were testing ('info').*/
                 wait(dbConn, save);
+                String sqlText = "update HIVE_LOCKS" +
+                  " set HL_BLOCKEDBY_EXT_ID=" + locks[i].extLockId +
+                  ", HL_BLOCKEDBY_INT_ID=" + locks[i].intLockId +
+                  " where HL_LOCK_EXT_ID=" + info.extLockId + " and HL_LOCK_INT_ID=" + info.intLockId;
+                LOG.debug("Executing sql: " + sqlText);
+                int updCnt = stmt.executeUpdate(sqlText);
+                if(updCnt != 1) {
+                  shouldNeverHappen(info.txnId, info.extLockId, info.intLockId);
+                }
                 LOG.debug("Going to commit");
                 dbConn.commit();
                 response.setState(LockState.WAITING);
@@ -2082,7 +2110,7 @@ abstract class TxnHandler implements TxnStore, TxnStore.MutexAPI {
       dbConn.commit();
       response.setState(LockState.ACQUIRED);
     } finally {
-      closeStmt(stmt);
+      close(rs, stmt, null);
     }
     return response;
   }

http://git-wip-us.apache.org/repos/asf/hive/blob/4e9f95a1/metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
----------------------------------------------------------------------
diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java b/metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
index 26a660a..37eacde 100644
--- a/metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
+++ b/metastore/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java
@@ -1153,7 +1153,7 @@ public class TestTxnHandler {
   }
 
   @Test
-  @Ignore
+  @Ignore("Wedges Derby")
   public void deadlockDetected() throws Exception {
     LOG.debug("Starting deadlock test");
     if (txnHandler instanceof TxnHandler) {

http://git-wip-us.apache.org/repos/asf/hive/blob/4e9f95a1/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
index 56eecf6..b26f09d 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
@@ -2541,6 +2541,8 @@ public class DDLTask extends Task<DDLWork> implements Serializable {
     os.write(separator);
     os.writeBytes("State");
     os.write(separator);
+    os.writeBytes("Blocked By");
+    os.write(separator);
     os.writeBytes("Type");
     os.write(separator);
     os.writeBytes("Transaction ID");
@@ -2557,7 +2559,12 @@ public class DDLTask extends Task<DDLWork> implements Serializable {
     List<ShowLocksResponseElement> locks = rsp.getLocks();
     if (locks != null) {
       for (ShowLocksResponseElement lock : locks) {
-        os.writeBytes(Long.toString(lock.getLockid()));
+        if(lock.isSetLockIdInternal()) {
+          os.writeBytes(Long.toString(lock.getLockid()) + "." + Long.toString(lock.getLockIdInternal()));
+        }
+        else {
+          os.writeBytes(Long.toString(lock.getLockid()));
+        }
         os.write(separator);
         os.writeBytes(lock.getDbname());
         os.write(separator);
@@ -2567,6 +2574,13 @@ public class DDLTask extends Task<DDLWork> implements Serializable {
         os.write(separator);
         os.writeBytes(lock.getState().toString());
         os.write(separator);
+        if(lock.isSetBlockedByExtId()) {//both "blockedby" are either there or not
+          os.writeBytes(Long.toString(lock.getBlockedByExtId()) + "." + Long.toString(lock.getBlockedByIntId()));
+        }
+        else {
+          os.writeBytes("            ");//12 chars - try to keep cols aligned
+        }
+        os.write(separator);
         os.writeBytes(lock.getType().toString());
         os.write(separator);
         os.writeBytes((lock.getTxnid() == 0) ? "NULL" : Long.toString(lock.getTxnid()));

http://git-wip-us.apache.org/repos/asf/hive/blob/4e9f95a1/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 d1b370e..836b507 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
@@ -240,6 +240,34 @@ public class TestDbTxnManager2 {
     otherTxnMgr.closeTxnManager();
   }
 
+  /**
+   * check that locks in Waiting state show what they are waiting on
+   * This test is somewhat abusive in that it make DbLockManager retain locks for 2
+   * different queries (which are not part of the same transaction) which can never
+   * happen in real use cases... but it makes testing convenient.
+   * @throws Exception
+   */
+  @Test
+  public void testLockBlockedBy() throws Exception {
+    CommandProcessorResponse cpr = driver.run("create table TAB_BLOCKED (a int, b int) clustered by (a) into 2  buckets stored as orc TBLPROPERTIES ('transactional'='true')");
+    checkCmdOnDriver(cpr);
+    cpr = driver.compileAndRespond("select * from TAB_BLOCKED");
+    checkCmdOnDriver(cpr);
+    txnMgr.acquireLocks(driver.getPlan(), ctx, "I AM SAM");
+    List<ShowLocksResponseElement> locks = getLocks(txnMgr);
+    Assert.assertEquals("Unexpected lock count", 1, locks.size());
+    checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "TAB_BLOCKED", null, locks.get(0));
+    cpr = driver.compileAndRespond("drop table TAB_BLOCKED");
+    checkCmdOnDriver(cpr);
+    ((DbTxnManager)txnMgr).acquireLocks(driver.getPlan(), ctx, "SAM I AM", false);//make non-blocking
+    locks = getLocks(txnMgr);
+    Assert.assertEquals("Unexpected lock count", 2, locks.size());
+    checkLock(LockType.SHARED_READ, LockState.ACQUIRED, "default", "TAB_BLOCKED", null, locks.get(0));
+    checkLock(LockType.EXCLUSIVE, LockState.WAITING, "default", "TAB_BLOCKED", null, locks.get(1));
+    Assert.assertEquals("BlockedByExtId doesn't match", locks.get(0).getLockid(), locks.get(1).getBlockedByExtId());
+    Assert.assertEquals("BlockedByIntId doesn't match", locks.get(0).getLockIdInternal(), locks.get(1).getBlockedByIntId());
+  }
+
   @Test
   public void testDummyTxnManagerOnAcidTable() throws Exception {
     // Create an ACID table with DbTxnManager

http://git-wip-us.apache.org/repos/asf/hive/blob/4e9f95a1/ql/src/test/results/clientpositive/dbtxnmgr_showlocks.q.out
----------------------------------------------------------------------
diff --git a/ql/src/test/results/clientpositive/dbtxnmgr_showlocks.q.out b/ql/src/test/results/clientpositive/dbtxnmgr_showlocks.q.out
index d9d2ed6..46d8ea1 100644
--- a/ql/src/test/results/clientpositive/dbtxnmgr_showlocks.q.out
+++ b/ql/src/test/results/clientpositive/dbtxnmgr_showlocks.q.out
@@ -2,17 +2,17 @@ PREHOOK: query: show locks
 PREHOOK: type: SHOWLOCKS
 POSTHOOK: query: show locks
 POSTHOOK: type: SHOWLOCKS
-Lock ID	Database	Table	Partition	State	Type	Transaction ID	Last Hearbeat	Acquired At	User	Hostname
+Lock ID	Database	Table	Partition	State	Blocked By	Type	Transaction ID	Last Hearbeat	Acquired At	User
 PREHOOK: query: show locks extended
 PREHOOK: type: SHOWLOCKS
 POSTHOOK: query: show locks extended
 POSTHOOK: type: SHOWLOCKS
-Lock ID	Database	Table	Partition	State	Type	Transaction ID	Last Hearbeat	Acquired At	User	Hostname
+Lock ID	Database	Table	Partition	State	Blocked By	Type	Transaction ID	Last Hearbeat	Acquired At	User
 PREHOOK: query: show locks default
 PREHOOK: type: SHOWLOCKS
 POSTHOOK: query: show locks default
 POSTHOOK: type: SHOWLOCKS
-Lock ID	Database	Table	Partition	State	Type	Transaction ID	Last Hearbeat	Acquired At	User	Hostname
+Lock ID	Database	Table	Partition	State	Blocked By	Type	Transaction ID	Last Hearbeat	Acquired At	User
 PREHOOK: query: show transactions
 PREHOOK: type: SHOW TRANSACTIONS
 POSTHOOK: query: show transactions