hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ct...@apache.org
Subject [3/3] hive git commit: HIVE-16334: Query lock contains the query string, which can cause OOM on ZooKeeper (Peter Vary via Chaoyu Tang)
Date Fri, 07 Apr 2017 12:49:06 GMT
HIVE-16334: Query lock contains the query string, which can cause OOM on ZooKeeper (Peter Vary
via Chaoyu Tang)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/0d0e4976
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/0d0e4976
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/0d0e4976

Branch: refs/heads/master
Commit: 0d0e4976c155a450b93a14ce678b6ffb21f639df
Parents: 72fb816
Author: Chaoyu Tang <ctang@cloudera.com>
Authored: Fri Apr 7 08:44:28 2017 -0400
Committer: Chaoyu Tang <ctang@cloudera.com>
Committed: Fri Apr 7 08:44:28 2017 -0400

----------------------------------------------------------------------
 .../org/apache/hadoop/hive/conf/HiveConf.java   |  3 ++
 .../hadoop/hive/ql/lockmgr/DummyTxnManager.java |  3 +-
 .../hadoop/hive/ql/lockmgr/HiveLockObject.java  | 18 ++++++++----
 .../hive/ql/lockmgr/HiveTxnManagerImpl.java     |  5 ++--
 .../hive/ql/lockmgr/TestDummyTxnManager.java    |  4 +--
 .../ql/lockmgr/TestEmbeddedLockManager.java     |  4 ++-
 .../hive/ql/lockmgr/TestHiveLockObject.java     | 30 ++++++++++++++++++--
 .../zookeeper/TestZookeeperLockManager.java     |  2 +-
 8 files changed, 55 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/0d0e4976/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
index 7e11649..4f7f1e7 100644
--- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
+++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
@@ -1771,6 +1771,9 @@ public class HiveConf extends Configuration {
     HIVE_LOCK_MAPRED_ONLY("hive.lock.mapred.only.operation", false,
         "This param is to control whether or not only do lock on queries\n" +
         "that need to execute at least one mapred job."),
+    HIVE_LOCK_QUERY_STRING_MAX_LENGTH("hive.lock.query.string.max.length", 1000000,
+        "The maximum length of the query string to store in the lock.\n" +
+        "The default value is 1000000, since the data limit of a znode is 1MB"),
 
      // Zookeeper related configs
     HIVE_ZOOKEEPER_QUORUM("hive.zookeeper.quorum", "",

http://git-wip-us.apache.org/repos/asf/hive/blob/0d0e4976/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 53ee9c8..24df25b 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
@@ -301,7 +301,8 @@ class DummyTxnManager extends HiveTxnManagerImpl {
       new HiveLockObject.HiveLockObjectData(plan.getQueryId(),
                              String.valueOf(System.currentTimeMillis()),
                              "IMPLICIT",
-                             plan.getQueryStr());
+                             plan.getQueryStr(),
+                             conf);
 
     if (db != null) {
       locks.add(new HiveLockObj(new HiveLockObject(db.getName(), lockData),

http://git-wip-us.apache.org/repos/asf/hive/blob/0d0e4976/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 fff03df..a514339 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
@@ -23,6 +23,7 @@ import java.util.Map;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.builder.HashCodeBuilder;
 import org.apache.hadoop.hive.common.StringInternUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.MetaStoreUtils;
 import org.apache.hadoop.hive.ql.metadata.DummyPartition;
 import org.apache.hadoop.hive.ql.metadata.Hive;
@@ -48,16 +49,23 @@ public class HiveLockObject {
      * Note: The parameters are used to uniquely identify a HiveLockObject. 
      * The parameters will be stripped off any ':' characters in order not 
      * to interfere with the way the data is serialized (':' delimited string).
+     * The query string might be truncated depending on HIVE_LOCK_QUERY_STRING_MAX_LENGTH
+     * @param queryId The query identifier will be added to the object without change
+     * @param lockTime The lock time  will be added to the object without change
+     * @param lockMode The lock mode  will be added to the object without change
+     * @param queryStr The query string might be truncated based on
+     *     HIVE_LOCK_QUERY_STRING_MAX_LENGTH conf variable
+     * @param conf The hive configuration based on which we decide if we should truncate
the query
+     *     string or not
      */
-    public HiveLockObjectData(String queryId,
-        String lockTime,
-        String lockMode,
-        String queryStr) {
+    public HiveLockObjectData(String queryId, String lockTime, String lockMode, String queryStr,
+        HiveConf conf) {
       this.queryId = removeDelimiter(queryId);
       this.lockTime = StringInternUtils.internIfNotNull(removeDelimiter(lockTime));
       this.lockMode = removeDelimiter(lockMode);
       this.queryStr = StringInternUtils.internIfNotNull(
-          removeDelimiter(queryStr == null ? null : queryStr.trim()));
+          queryStr == null ? null : StringUtils.substring(removeDelimiter(queryStr.trim()),
0,
+              conf.getIntVar(HiveConf.ConfVars.HIVE_LOCK_QUERY_STRING_MAX_LENGTH)));
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/hive/blob/0d0e4976/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java
index a371a5a..9fa416c 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java
@@ -93,7 +93,8 @@ abstract class HiveTxnManagerImpl implements HiveTxnManager {
         new HiveLockObjectData(lockTbl.getQueryId(),
             String.valueOf(System.currentTimeMillis()),
             "EXPLICIT",
-            lockTbl.getQueryStr());
+            lockTbl.getQueryStr(),
+            conf);
 
     if (partSpec == null) {
       HiveLock lck = lockMgr.lock(new HiveLockObject(tbl, lockData), mode, true);
@@ -151,7 +152,7 @@ abstract class HiveTxnManagerImpl implements HiveTxnManager {
     HiveLockObjectData lockData =
         new HiveLockObjectData(lockDb.getQueryId(),
             String.valueOf(System.currentTimeMillis()),
-            "EXPLICIT", lockDb.getQueryStr());
+            "EXPLICIT", lockDb.getQueryStr(), conf);
 
     HiveLock lck = lockMgr.lock(new HiveLockObject(dbObj.getName(), lockData), mode, true);
     if (lck == null) {

http://git-wip-us.apache.org/repos/asf/hive/blob/0d0e4976/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 de3b8ad..9d27d21 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
@@ -147,9 +147,9 @@ public class TestDummyTxnManager {
     String path1 = "path1";
     String path2 = "path2";
     HiveLockObjectData lockData1 = new HiveLockObjectData(
-        "query1", "1", "IMPLICIT", "drop table table1");
+        "query1", "1", "IMPLICIT", "drop table table1", conf);
     HiveLockObjectData lockData2 = new HiveLockObjectData(
-        "query1", "1", "IMPLICIT", "drop table table1");
+        "query1", "1", "IMPLICIT", "drop table table1", conf);
 
     // Start with the following locks:
     // [path1, shared]

http://git-wip-us.apache.org/repos/asf/hive/blob/0d0e4976/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestEmbeddedLockManager.java
----------------------------------------------------------------------
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestEmbeddedLockManager.java b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestEmbeddedLockManager.java
index 0afbc1c..95a9856 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestEmbeddedLockManager.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestEmbeddedLockManager.java
@@ -27,6 +27,7 @@ import org.junit.Assert;
 public class TestEmbeddedLockManager extends TestCase {
 
   private int counter;
+  private HiveConf conf = new HiveConf();
 
   public void testLocking() throws LockException {
     HiveConf conf = new HiveConf();
@@ -119,7 +120,8 @@ public class TestEmbeddedLockManager extends TestCase {
   }
 
   private HiveLockObject lockObj(String path, String query) {
-    HiveLockObjectData data = new HiveLockObjectData(String.valueOf(++counter), null, null,
query);
+    HiveLockObjectData data = new HiveLockObjectData(String.valueOf(++counter), null, null,
+        query, conf);
     return new HiveLockObject(path.split("/"), data);
   }
 }

http://git-wip-us.apache.org/repos/asf/hive/blob/0d0e4976/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestHiveLockObject.java
----------------------------------------------------------------------
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestHiveLockObject.java b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestHiveLockObject.java
index 19cb129..024114f 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestHiveLockObject.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestHiveLockObject.java
@@ -20,15 +20,20 @@ package org.apache.hadoop.hive.ql.lockmgr;
 
 import junit.framework.Assert;
 
+import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.ql.lockmgr.HiveLockObject.HiveLockObjectData;
 import org.junit.Test;
 
 public class TestHiveLockObject {
 
+  private HiveConf conf = new HiveConf();
+
   @Test
   public void testEqualsAndHashCode() {
-    HiveLockObjectData data1 = new HiveLockObjectData("ID1", "SHARED", "1997-07-01", "select
* from mytable");
-    HiveLockObjectData data2 = new HiveLockObjectData("ID1", "SHARED", "1997-07-01", "select
* from mytable");
+    HiveLockObjectData data1 = new HiveLockObjectData("ID1", "SHARED", "1997-07-01",
+        "select * from mytable", conf);
+    HiveLockObjectData data2 = new HiveLockObjectData("ID1", "SHARED", "1997-07-01",
+        "select * from mytable", conf);
     Assert.assertEquals(data1, data2);
     Assert.assertEquals(data1.hashCode(), data2.hashCode());
 
@@ -38,4 +43,25 @@ public class TestHiveLockObject {
     Assert.assertEquals(obj1.hashCode(), obj2.hashCode());
   }
 
+  @Test
+  public void testTruncate() {
+    conf.setIntVar(HiveConf.ConfVars.HIVE_LOCK_QUERY_STRING_MAX_LENGTH, 1000000);
+    HiveLockObjectData data0 = new HiveLockObjectData("ID1", "SHARED", "1997-07-01",
+        "01234567890", conf);
+    Assert.assertEquals("With default settings query string should not be truncated",
+        data0.getQueryStr().length(), 11);
+    conf.setIntVar(HiveConf.ConfVars.HIVE_LOCK_QUERY_STRING_MAX_LENGTH, 10);
+    HiveLockObjectData data1 = new HiveLockObjectData("ID1", "SHARED", "1997-07-01",
+        "01234567890", conf);
+    HiveLockObjectData data2 = new HiveLockObjectData("ID1", "SHARED", "1997-07-01",
+        "0123456789", conf);
+    HiveLockObjectData data3 = new HiveLockObjectData("ID1", "SHARED", "1997-07-01",
+        "012345678", conf);
+    HiveLockObjectData data4 = new HiveLockObjectData("ID1", "SHARED", "1997-07-01",
+        null, conf);
+    Assert.assertEquals("Long string truncation failed", data1.getQueryStr().length(), 10);
+    Assert.assertEquals("String truncation failed", data2.getQueryStr().length(), 10);
+    Assert.assertEquals("Short string should not be truncated", data3.getQueryStr().length(),
9);
+    Assert.assertNull("Null query string handling failed", data4.getQueryStr());
+  }
 }

http://git-wip-us.apache.org/repos/asf/hive/blob/0d0e4976/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/zookeeper/TestZookeeperLockManager.java
----------------------------------------------------------------------
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/zookeeper/TestZookeeperLockManager.java
b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/zookeeper/TestZookeeperLockManager.java
index 3f9926e..a7a76a4 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/zookeeper/TestZookeeperLockManager.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/lockmgr/zookeeper/TestZookeeperLockManager.java
@@ -62,7 +62,7 @@ public class TestZookeeperLockManager {
   @Before
   public void setup() {
     conf = new HiveConf();
-    lockObjData = new HiveLockObjectData("1", "10", "SHARED", "show tables");
+    lockObjData = new HiveLockObjectData("1", "10", "SHARED", "show tables", conf);
     hiveLock = new HiveLockObject(TABLE, lockObjData);
     zLock = new ZooKeeperHiveLock(TABLE_LOCK_PATH, hiveLock, HiveLockMode.SHARED);
 


Mime
View raw message