Mailing-List: contact dev-help@hive.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hive.apache.org
Content-Type: multipart/alternative;
 boundary="===============0099994050130398943=="
MIME-Version: 1.0
Subject: Re: Review Request 58086: HIVE-16334 Query lock contains the query
 string, which can cause OOM on ZooKeeper
From: Vihang Karajgaonkar <vihang@cloudera.com>
To: Barna Zsombor Klara <zsombor.klara@cloudera.com>,
 Marta Kuczora <kuczoram@cloudera.com>, Aihua Xu <axu@cloudera.com>,
 Miklos Csanady <miklos.csanady@cloudera.com>, Chaoyu Tang <ctang.ma@gmail.com>
Cc: Peter Vary <pvary@cloudera.com>, hive <dev@hive.apache.org>,
 Vihang Karajgaonkar <vihang@cloudera.com>
Date: Wed, 05 Apr 2017 16:40:38 -0000
Message-ID: <20170405164038.56253.79841@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: Vihang Karajgaonkar <noreply@reviews.apache.org>
References: <20170330180133.31070.75677@reviews-vm2.apache.org>
In-Reply-To: <20170330180133.31070.75677@reviews-vm2.apache.org>
Reply-To: Vihang Karajgaonkar <vihang@cloudera.com>
archived-at: Wed, 05 Apr 2017 16:40:49 -0000

--===============0099994050130398943==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58086/#review171135
-----------------------------------------------------------


Fix it, then Ship it!


Thanks for the patch Peter. Overall looks good. Couple of suggestions below


common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 1774 (patched)
<https://reviews.apache.org/r/58086/#comment244025>

    Do you think we should use a large value for the default instead of -1?


- Vihang Karajgaonkar


On March 30, 2017, 6:01 p.m., Peter Vary wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58086/
> -----------------------------------------------------------
> 
> (Updated March 30, 2017, 6:01 p.m.)
> 
> 
> Review request for hive, Aihua Xu, Chaoyu Tang, Marta Kuczora, Miklos Csanady, and Barna Zsombor Klara.
> 
> 
> Bugs: HIVE-16334
>     https://issues.apache.org/jira/browse/HIVE-16334
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> The patch contains the following changes:
> - Added a new config variable which defines the maximum length of the query string in the lock object
> - When constructing the HiveLockObject gets the configuration as a new parameter
> - When constructing the HiveLockObject truncates the length of the query string if it is necessary
> - Updates every occurance of the HiveLockObject creation
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e12fea1 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java 53ee9c8 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockObject.java fff03df 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java a371a5a 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDummyTxnManager.java de3b8ad 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestEmbeddedLockManager.java 0afbc1c 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestHiveLockObject.java 19cb129 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/zookeeper/TestZookeeperLockManager.java 3f9926e 
> 
> 
> Diff: https://reviews.apache.org/r/58086/diff/1/
> 
> 
> Testing
> -------
> 
> Added new unit test to test the truncation
> Updated the existing unit tests
> 
> 
> Thanks,
> 
> Peter Vary
> 
>


--===============0099994050130398943==--