FAQ
Repository: hive
Updated Branches:
   refs/heads/master 57fcbce52 -> 106e0931f


HIVE-10483 - insert overwrite partition deadlocks on itself with DbTxnManager (Eugene Koifman, reviewed 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/106e0931
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/106e0931
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/106e0931

Branch: refs/heads/master
Commit: 106e0931f4be765b750adaf2c5cb654a233baef0
Parents: 57fcbce
Author: Eugene Koifman <ekoifman@hortonworks.com>
Authored: Tue Apr 28 20:11:56 2015 -0700
Committer: Eugene Koifman <ekoifman@hortonworks.com>
Committed: Tue Apr 28 20:11:56 2015 -0700

----------------------------------------------------------------------
  .../apache/hadoop/hive/common/JavaUtils.java | 8 ++++++
  .../hadoop/hive/metastore/txn/TxnHandler.java | 22 +++++++++++-----
  .../hadoop/hive/ql/lockmgr/DbLockManager.java | 9 +++++--
  .../apache/hadoop/hive/ql/TestTxnCommands2.java | 27 +++++++++++++++++++-
  4 files changed, 57 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/106e0931/common/src/java/org/apache/hadoop/hive/common/JavaUtils.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/common/JavaUtils.java b/common/src/java/org/apache/hadoop/hive/common/JavaUtils.java
index a212fb8..3dd8f75 100644
--- a/common/src/java/org/apache/hadoop/hive/common/JavaUtils.java
+++ b/common/src/java/org/apache/hadoop/hive/common/JavaUtils.java
@@ -136,6 +136,14 @@ public final class JavaUtils {
      LogFactory.release(loader);
    }

+ /**
+ * Utility method for ACID to normalize logging info
+ * @param extLockId LockResponse.lockid
+ */
+ public static String lockIdToString(long extLockId) {
+ return "lockid:" + extLockId;
+ }
+
    private JavaUtils() {
      // prevent instantiation
    }

http://git-wip-us.apache.org/repos/asf/hive/blob/106e0931/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 1e64fc7..704c3ed 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
@@ -28,6 +28,7 @@ import org.apache.commons.dbcp.PoolingDataSource;

  import org.apache.commons.pool.ObjectPool;
  import org.apache.commons.pool.impl.GenericObjectPool;
+import org.apache.hadoop.hive.common.JavaUtils;
  import org.apache.hadoop.hive.common.ValidTxnList;
  import org.apache.hadoop.hive.common.ValidReadTxnList;
  import org.apache.hadoop.hive.conf.HiveConf;
@@ -1115,6 +1116,8 @@ public class TxnHandler {
    private static class LockInfo {
      private final long extLockId;
      private final long intLockId;
+ //0 means there is no transaction, i.e. it a select statement which is not part of
+ //explicit transaction or a IUD statement that is not writing to ACID table
      private final long txnId;
      private final String db;
      private final String table;
@@ -1144,7 +1147,7 @@ public class TxnHandler {
          default:
            throw new MetaException("Unknown lock type " + rs.getString("hl_lock_type").charAt(0));
        }
- txnId = rs.getLong("hl_txnid");
+ txnId = rs.getLong("hl_txnid");//returns 0 if value is NULL
      }
      LockInfo(ShowLocksResponseElement e, long intLockId) {
        extLockId = e.getLockid();
@@ -1166,7 +1169,7 @@ public class TxnHandler {

      @Override
      public String toString() {
- return "extLockId:" + Long.toString(extLockId) + " intLockId:" +
+ return JavaUtils.lockIdToString(extLockId) + " intLockId:" +
          intLockId + " txnId:" + Long.toString
          (txnId) + " db:" + db + " table:" + table + " partition:" +
          partition + " state:" + (state == null ? "null" : state.toString())
@@ -1642,10 +1645,17 @@ public class TxnHandler {
     * on a database.
     */
    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);
+ 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))
+ ||
+ //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
+ //insert overwrite table T partition(p=1) select a,b from T and autoCommit=true
+ (desiredLock.txnId == 0 && desiredLock.extLockId == existingLock.extLockId);
    }

    private void wait(Connection dbConn, Savepoint save) throws SQLException {

http://git-wip-us.apache.org/repos/asf/hive/blob/106e0931/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbLockManager.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbLockManager.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbLockManager.java
index 805e090..e8c49ef 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbLockManager.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbLockManager.java
@@ -19,6 +19,7 @@ package org.apache.hadoop.hive.ql.lockmgr;

  import org.apache.commons.logging.Log;
  import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.hive.common.JavaUtils;
  import org.apache.hadoop.hive.metastore.HiveMetaStoreClient;
  import org.apache.hadoop.hive.metastore.IMetaStoreClient;
  import org.apache.hadoop.hive.metastore.api.*;
@@ -129,9 +130,9 @@ public class DbLockManager implements HiveLockManager{
    public void unlock(HiveLock hiveLock) throws LockException {
      long lockId = ((DbHiveLock)hiveLock).lockId;
      try {
- LOG.debug("Unlocking id:" + lockId);
+ LOG.debug("Unlocking " + hiveLock);
        client.unlock(lockId);
- boolean removed = locks.remove((DbHiveLock)hiveLock);
+ boolean removed = locks.remove(hiveLock);
        LOG.debug("Removed a lock " + removed);
      } catch (NoSuchLockException e) {
        LOG.error("Metastore could find no record of lock " + lockId);
@@ -228,6 +229,10 @@ public class DbLockManager implements HiveLockManager{
      public int hashCode() {
        return (int)(lockId % Integer.MAX_VALUE);
      }
+ @Override
+ public String toString() {
+ return JavaUtils.lockIdToString(lockId);
+ }
    }

    /**

http://git-wip-us.apache.org/repos/asf/hive/blob/106e0931/ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java
----------------------------------------------------------------------
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java b/ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java
index ac5ae2a..1431e19 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java
@@ -44,7 +44,9 @@ public class TestTxnCommands2 {
    private Driver d;
    private static enum Table {
      ACIDTBL("acidTbl"),
- NONACIDORCTBL("nonAcidOrcTbl");
+ ACIDTBLPART("acidTblPart"),
+ NONACIDORCTBL("nonAcidOrcTbl"),
+ NONACIDPART("nonAcidPart");

      private final String name;
      @Override
@@ -78,7 +80,9 @@ public class TestTxnCommands2 {
      d = new Driver(hiveConf);
      dropTables();
      runStatementOnDriver("create table " + Table.ACIDTBL + "(a int, b int) clustered by (a) into " + BUCKET_COUNT + " buckets stored as orc TBLPROPERTIES ('transactional'='true')");
+ runStatementOnDriver("create table " + Table.ACIDTBLPART + "(a int, b int) partitioned by (p string) clustered by (a) into " + BUCKET_COUNT + " buckets stored as orc TBLPROPERTIES ('transactional'='true')");
      runStatementOnDriver("create table " + Table.NONACIDORCTBL + "(a int, b int) clustered by (a) into " + BUCKET_COUNT + " buckets stored as orc TBLPROPERTIES ('transactional'='false')");
+ runStatementOnDriver("create table " + Table.NONACIDPART + "(a int, b int) partitioned by (p string) stored as orc TBLPROPERTIES ('transactional'='false')");
    }
    private void dropTables() throws Exception {
      for(Table t : Table.values()) {
@@ -138,6 +142,27 @@ public class TestTxnCommands2 {
      Assert.assertEquals("Bulk update2 failed", stringifyValues(updatedData2), rs2);
    }

+ @Test
+ public void testInsertOverwriteWithSelfJoin() throws Exception {
+ int[][] part1Data = {{1,7}};
+ runStatementOnDriver("insert into " + Table.NONACIDORCTBL + "(a,b) " + makeValuesClause(part1Data));
+ //this works because logically we need S lock on NONACIDORCTBL to read and X lock to write, but
+ //LockRequestBuilder dedups locks on the same entity to only keep the highest level lock requested
+ runStatementOnDriver("insert overwrite table " + Table.NONACIDORCTBL + " select 2, 9 from " + Table.NONACIDORCTBL + " T inner join " + Table.NONACIDORCTBL + " S on T.a=S.a");
+ List<String> rs = runStatementOnDriver("select a,b from " + Table.NONACIDORCTBL + " order by a,b");
+ int[][] joinData = {{2,9}};
+ Assert.assertEquals("Self join non-part insert overwrite failed", stringifyValues(joinData), rs);
+ int[][] part2Data = {{1,8}};
+ runStatementOnDriver("insert into " + Table.NONACIDPART + " partition(p=1) (a,b) " + makeValuesClause(part1Data));
+ runStatementOnDriver("insert into " + Table.NONACIDPART + " partition(p=2) (a,b) " + makeValuesClause(part2Data));
+ //here we need X lock on p=1 partition to write and S lock on 'table' to read which should
+ //not block each other since they are part of the same txn
+ runStatementOnDriver("insert overwrite table " + Table.NONACIDPART + " partition(p=1) select a,b from " + Table.NONACIDPART);
+ List<String> rs2 = runStatementOnDriver("select a,b from " + Table.NONACIDPART + " order by a,b");
+ int[][] updatedData = {{1,7},{1,8},{1,8}};
+ Assert.assertEquals("Insert overwrite partition failed", stringifyValues(updatedData), rs2);
+ //insert overwrite not supported for ACID tables
+ }
    /**
     * takes raw data and turns it into a string as if from Driver.getResults()
     * sorts rows in dictionary order

Search Discussions

Discussion Posts

Previous

Related Discussions

Discussion Navigation
viewthread | post
posts ‹ prev | 2 of 2 | next ›
Discussion Overview
groupcommits @
categorieshive, hadoop
postedApr 29, '15 at 12:46a
activeApr 29, '15 at 3:12a
posts2
users1
websitehive.apache.org

1 user in discussion

Ekoifman: 2 posts

People

Translate

site design / logo © 2021 Grokbase