You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Vihang Karajgaonkar <vi...@cloudera.com> on 2017/04/18 19:15:08 UTC
Review Request 58516: HIVE-16213 : ObjectStore can leak Queries when
rollbackTransaction throws an exception
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58516/
-----------------------------------------------------------
Review request for hive, Alexander Kolbasov, Sergio Pena, and Sahil Takiar.
Bugs: HIVE-16213
https://issues.apache.org/jira/browse/HIVE-16213
Repository: hive-git
Description
-------
HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an exception
Diffs
-----
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6b217516bc74c612348b8edeca077dfbdbdb1a40
Diff: https://reviews.apache.org/r/58516/diff/1/
Testing
-------
Thanks,
Vihang Karajgaonkar
Re: Review Request 58516: HIVE-16213 : ObjectStore can leak Queries
when rollbackTransaction throws an exception
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58516/#review172704
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Kolbasov
On April 21, 2017, 5:55 p.m., Vihang Karajgaonkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58516/
> -----------------------------------------------------------
>
> (Updated April 21, 2017, 5:55 p.m.)
>
>
> Review request for hive, Alexander Kolbasov, Sergio Pena, and Sahil Takiar.
>
>
> Bugs: HIVE-16213
> https://issues.apache.org/jira/browse/HIVE-16213
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an exception
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6b217516bc74c612348b8edeca077dfbdbdb1a40
> metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 9b8eaf2ab72fcf1fe0d58e8913ac1b84ec7e71cf
>
>
> Diff: https://reviews.apache.org/r/58516/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vihang Karajgaonkar
>
>
Re: Review Request 58516: HIVE-16213 : ObjectStore can leak Queries
when rollbackTransaction throws an exception
Posted by Vihang Karajgaonkar <vi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58516/
-----------------------------------------------------------
(Updated April 21, 2017, 5:55 p.m.)
Review request for hive, Alexander Kolbasov, Sergio Pena, and Sahil Takiar.
Changes
-------
renamed the method
Bugs: HIVE-16213
https://issues.apache.org/jira/browse/HIVE-16213
Repository: hive-git
Description
-------
HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an exception
Diffs (updated)
-----
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6b217516bc74c612348b8edeca077dfbdbdb1a40
metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 9b8eaf2ab72fcf1fe0d58e8913ac1b84ec7e71cf
Diff: https://reviews.apache.org/r/58516/diff/3/
Changes: https://reviews.apache.org/r/58516/diff/2-3/
Testing
-------
Thanks,
Vihang Karajgaonkar
Re: Review Request 58516: HIVE-16213 : ObjectStore can leak Queries
when rollbackTransaction throws an exception
Posted by Vihang Karajgaonkar <vi...@cloudera.com>.
> On April 21, 2017, 6:19 a.m., Alexander Kolbasov wrote:
> > The name is not very good rollbackIfNotSuccessful since this thing also closes query, but rollbackaIfNotSuccessfulButCloseQueryAnyway isn't much better :-).
> > May be just cleanup()?
I agree with the name not being great. How about I use rollbackAndCleanup() and add a good javadoc :)
- Vihang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58516/#review172590
-----------------------------------------------------------
On April 20, 2017, 10:01 p.m., Vihang Karajgaonkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58516/
> -----------------------------------------------------------
>
> (Updated April 20, 2017, 10:01 p.m.)
>
>
> Review request for hive, Alexander Kolbasov, Sergio Pena, and Sahil Takiar.
>
>
> Bugs: HIVE-16213
> https://issues.apache.org/jira/browse/HIVE-16213
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an exception
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6b217516bc74c612348b8edeca077dfbdbdb1a40
> metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 9b8eaf2ab72fcf1fe0d58e8913ac1b84ec7e71cf
>
>
> Diff: https://reviews.apache.org/r/58516/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vihang Karajgaonkar
>
>
Re: Review Request 58516: HIVE-16213 : ObjectStore can leak Queries
when rollbackTransaction throws an exception
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58516/#review172590
-----------------------------------------------------------
Ship it!
The name is not very good rollbackIfNotSuccessful since this thing also closes query, but rollbackaIfNotSuccessfulButCloseQueryAnyway isn't much better :-).
May be just cleanup()?
- Alexander Kolbasov
On April 20, 2017, 10:01 p.m., Vihang Karajgaonkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58516/
> -----------------------------------------------------------
>
> (Updated April 20, 2017, 10:01 p.m.)
>
>
> Review request for hive, Alexander Kolbasov, Sergio Pena, and Sahil Takiar.
>
>
> Bugs: HIVE-16213
> https://issues.apache.org/jira/browse/HIVE-16213
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an exception
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6b217516bc74c612348b8edeca077dfbdbdb1a40
> metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 9b8eaf2ab72fcf1fe0d58e8913ac1b84ec7e71cf
>
>
> Diff: https://reviews.apache.org/r/58516/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vihang Karajgaonkar
>
>
Re: Review Request 58516: HIVE-16213 : ObjectStore can leak Queries
when rollbackTransaction throws an exception
Posted by Vihang Karajgaonkar <vi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58516/
-----------------------------------------------------------
(Updated April 20, 2017, 10:01 p.m.)
Review request for hive, Alexander Kolbasov, Sergio Pena, and Sahil Takiar.
Changes
-------
Added a simple test case and fixed a few more leaks.
Bugs: HIVE-16213
https://issues.apache.org/jira/browse/HIVE-16213
Repository: hive-git
Description
-------
HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an exception
Diffs (updated)
-----
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6b217516bc74c612348b8edeca077dfbdbdb1a40
metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 9b8eaf2ab72fcf1fe0d58e8913ac1b84ec7e71cf
Diff: https://reviews.apache.org/r/58516/diff/2/
Changes: https://reviews.apache.org/r/58516/diff/1-2/
Testing
-------
Thanks,
Vihang Karajgaonkar
Re: Review Request 58516: HIVE-16213 : ObjectStore can leak Queries
when rollbackTransaction throws an exception
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58516/#review172556
-----------------------------------------------------------
Ship it!
It looks good.
- Sergio Pena
On April 18, 2017, 7:15 p.m., Vihang Karajgaonkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58516/
> -----------------------------------------------------------
>
> (Updated April 18, 2017, 7:15 p.m.)
>
>
> Review request for hive, Alexander Kolbasov, Sergio Pena, and Sahil Takiar.
>
>
> Bugs: HIVE-16213
> https://issues.apache.org/jira/browse/HIVE-16213
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an exception
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6b217516bc74c612348b8edeca077dfbdbdb1a40
>
>
> Diff: https://reviews.apache.org/r/58516/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vihang Karajgaonkar
>
>
Re: Review Request 58516: HIVE-16213 : ObjectStore can leak Queries
when rollbackTransaction throws an exception
Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58516/#review172268
-----------------------------------------------------------
Ship it!
LGTM. It would be nice to add a unit test to one of the `ObjectStore` methods, say `getMDatabase`, to check that if `rollbackTransation` throws an exception then `query.closeAll` is invoked.
- Sahil Takiar
On April 18, 2017, 7:15 p.m., Vihang Karajgaonkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58516/
> -----------------------------------------------------------
>
> (Updated April 18, 2017, 7:15 p.m.)
>
>
> Review request for hive, Alexander Kolbasov, Sergio Pena, and Sahil Takiar.
>
>
> Bugs: HIVE-16213
> https://issues.apache.org/jira/browse/HIVE-16213
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an exception
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6b217516bc74c612348b8edeca077dfbdbdb1a40
>
>
> Diff: https://reviews.apache.org/r/58516/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vihang Karajgaonkar
>
>