> On June 2, 2015, 2:37 a.m., Thejas Nair wrote:
> > service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java,
> >  line 55
> > <https://reviews.apache.org/r/34776/diff/5/?file=976068#file976068line55>
> >
> >     I think this approach will cause the bug in HIVE-6245.
> >     If new Hive object is not created within doAs block, it would not 
> > create it with the correct user.
> >     I need to look some more into that.

Can you point at the doAs block you have in mind in existing code? ctor is 
called from openSession and that from variety of openSession overloads in the 
service, with no doAs blocks.
I can replicate it in createHive if needed


> On June 2, 2015, 2:37 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java, line 74
> > <https://reviews.apache.org/r/34776/diff/5/?file=976065#file976065line74>
> >
> >     If we follow this approach every place, it is going to lead to an 
> > explosion of thread local.
> >     
> >     Also, the use of thread local this way can lead to new bugs getting 
> > introduced over time.
> >     
> >     The driver objects lifetime is beyond compilation. The execution is 
> > usually done in a different thread.
> >     
> >     I feel a cleaner/more robust solution would be to have these objects 
> > tied to a driver instance rather than a thread. Not sure what the best 
> > approach for that would be - maybe have a thread local driver, and have it 
> > contain a context object that holds related fields ? Thoughts ?
> >     
> >     Rant : Having a Utils class have state is also counter intuitive, maybe 
> > the part that had state should have been part of a different class ..

There's a separate jira to refactor and add OperationState. I got rid of utils 
threadlocal; I think having one per session would be safe enough. All the 
required fields can be put in one threadlocal.


- Sergey


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


On June 2, 2015, 12:53 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34776/
> -----------------------------------------------------------
> 
> (Updated June 2, 2015, 12:53 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d733d71 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 5dac29f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java 0edfc5d 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 37b6d6f 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 
> 343c68e 
>   
> service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java
>  a29e5d1 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java b4d517f 
> 
> Diff: https://reviews.apache.org/r/34776/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Reply via email to