You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by ct...@apache.org on 2017/04/07 12:49:06 UTC
[3/3] hive git commit: HIVE-16334: Query lock contains the query
string, which can cause OOM on ZooKeeper (Peter Vary via Chaoyu Tang)
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 <ct...@cloudera.com>
Authored: Fri Apr 7 08:44:28 2017 -0400
Committer: Chaoyu Tang <ct...@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);