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



service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java
<https://reviews.apache.org/r/17437/#comment68299>

    I figure you have moved the functions that don't need to be executed under 
doAs() to this base interface. Can you also add a class comment about that (ie 
only functions that need not be executed under doAs action should be put here)?
    



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

    can you add @Override annotation ?
    



service/src/java/org/apache/hive/service/cli/session/SessionManager.java
<https://reviews.apache.org/r/17437/#comment68301>

    Having the threadlocal username and ipaddress in TSetIpAddressProcessor 
looks fine to me. We don't seem to gain much by having the extra hop in 
SessionManager. TSetIpAddressProcessor is the only class that sets/resets it.
    I agree that this can break other pending patches, but we can hopefully get 
this in quickly.
    


- Thejas Nair


On Jan. 28, 2014, 1:21 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17437/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2014, 1:21 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6312
>     https://issues.apache.org/jira/browse/HIVE-6312
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> TUGIContainingProcessor creates new Subject for each invocation which induces 
> FileSystem leakage when cache is enable(true by default).
> 
> 
> Diffs
> -----
> 
>   service/src/java/org/apache/hive/service/auth/PlainSaslHelper.java 15b1675 
>   service/src/java/org/apache/hive/service/auth/TSetIpAddressProcessor.java 
> 0bf34ce 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 
> c8fb8ec 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 
> PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 
> 445c858 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionProxy.java 
> 76f18a9 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 
> bfe0e7b 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> b5a6138 
>   service/src/test/org/apache/hive/service/auth/TestPlainSaslHelper.java 
> 8fa4afd 
> 
> Diff: https://reviews.apache.org/r/17437/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>

Reply via email to