Re: 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/#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
--- 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
> 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
--- 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
--- 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
--- 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
--- 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 > >
