You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Jimmy Xiang <jx...@cloudera.com> on 2015/04/28 19:56:13 UTC

Review Request 33633: HIVE-10499 Ensure Session/ZooKeeperClient instances are closed

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

Review request for hive and Szehon Ho.


Bugs: HIVE-10499
    https://issues.apache.org/jira/browse/HIVE-10499


Repository: hive-git


Description
-------

Make sure Session/SessionState/ZooKeeperHiveClient instances are closed if some exception is thrown in a wrong place, so as to avoid some possible resource leakage.


Diffs
-----

  jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 496c820 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java f14b974 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java e02997a 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 222cb45 

Diff: https://reviews.apache.org/r/33633/diff/


Testing
-------


Thanks,

Jimmy Xiang


Re: Review Request 33633: HIVE-10499 Ensure Session/ZooKeeperClient instances are closed

Posted by Jimmy Xiang <jx...@cloudera.com>.

> On April 28, 2015, 6:03 p.m., Szehon Ho wrote:
> > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java, line 568
> > <https://reviews.apache.org/r/33633/diff/1/?file=944227#file944227line568>
> >
> >     Is it necessary to call close twice?  Can we consolidate the two finally blocks?

The first one is in a block and may not get a chance to be executed. After the first one closes it, it is set to null. The second one is to make sure it is really closed. The first one throws an Exception. The second one just logs a warning. I thought about consolidating them but preferred not to change the existing behavior.


- Jimmy


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


On April 28, 2015, 5:56 p.m., Jimmy Xiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33633/
> -----------------------------------------------------------
> 
> (Updated April 28, 2015, 5:56 p.m.)
> 
> 
> Review request for hive and Szehon Ho.
> 
> 
> Bugs: HIVE-10499
>     https://issues.apache.org/jira/browse/HIVE-10499
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Make sure Session/SessionState/ZooKeeperHiveClient instances are closed if some exception is thrown in a wrong place, so as to avoid some possible resource leakage.
> 
> 
> Diffs
> -----
> 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 496c820 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java f14b974 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java e02997a 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 222cb45 
> 
> Diff: https://reviews.apache.org/r/33633/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jimmy Xiang
> 
>


Re: Review Request 33633: HIVE-10499 Ensure Session/ZooKeeperClient instances are closed

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33633/#review81845
-----------------------------------------------------------



service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
<https://reviews.apache.org/r/33633/#comment132336>

    Is it necessary to call close twice?  Can we consolidate the two finally blocks?


- Szehon Ho


On April 28, 2015, 5:56 p.m., Jimmy Xiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33633/
> -----------------------------------------------------------
> 
> (Updated April 28, 2015, 5:56 p.m.)
> 
> 
> Review request for hive and Szehon Ho.
> 
> 
> Bugs: HIVE-10499
>     https://issues.apache.org/jira/browse/HIVE-10499
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Make sure Session/SessionState/ZooKeeperHiveClient instances are closed if some exception is thrown in a wrong place, so as to avoid some possible resource leakage.
> 
> 
> Diffs
> -----
> 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 496c820 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java f14b974 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java e02997a 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 222cb45 
> 
> Diff: https://reviews.apache.org/r/33633/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jimmy Xiang
> 
>


Re: Review Request 33633: HIVE-10499 Ensure Session/ZooKeeperClient instances are closed

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33633/#review81878
-----------------------------------------------------------

Ship it!


Ship It!

- Szehon Ho


On April 28, 2015, 5:56 p.m., Jimmy Xiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33633/
> -----------------------------------------------------------
> 
> (Updated April 28, 2015, 5:56 p.m.)
> 
> 
> Review request for hive and Szehon Ho.
> 
> 
> Bugs: HIVE-10499
>     https://issues.apache.org/jira/browse/HIVE-10499
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Make sure Session/SessionState/ZooKeeperHiveClient instances are closed if some exception is thrown in a wrong place, so as to avoid some possible resource leakage.
> 
> 
> Diffs
> -----
> 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 496c820 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java f14b974 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java e02997a 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 222cb45 
> 
> Diff: https://reviews.apache.org/r/33633/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jimmy Xiang
> 
>