> On June 4, 2015, 10:31 p.m., Thejas Nair wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java, line 184 > > <https://reviews.apache.org/r/34776/diff/5-6/?file=976065#file976065line184> > > > > shoudl this also be static ?
no, see class comment - tests override it > On June 4, 2015, 10:31 p.m., Thejas Nair wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java, line 209 > > <https://reviews.apache.org/r/34776/diff/5-6/?file=976065#file976065line209> > > > > also make static ? same > On June 4, 2015, 10:31 p.m., Thejas Nair wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/TezCompiler.java, line 321 > > <https://reviews.apache.org/r/34776/diff/6/?file=976761#file976761line321> > > > > can't everything in the GenTezUtils be made static and then we don't > > need to create any instances of it ? > > Maybe make the constructor of GenTezUtils private. see above? > On June 4, 2015, 10:31 p.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> > > > > HiveSessionProxy is the one that does the doAs(). > > > > I looked some more, it looks like this should not cause a problem with > > the safeguards added in HIVE-7890. (but I think we should treat that change > > as a safeguard, as I am not sure of the performance implications of that > > additional check every time). > > > > However, with this approach, we would end up creating a new Hive object > > in most cases, if there are large number of users. Also the checks done in > > "Hive.get(conf);" to see if new object needs to be created is not trivial. > > > > The cases were sessions are used in parallel is rare, so paying this > > performance penalty for the more common case is not really justified IMO. > > > > I think we should just use locks around metastore client class for > > that. I think thats something we should pursue in a different jira. does it require anything in this JIRA? - Sergey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34776/#review86727 ----------------------------------------------------------- On June 2, 2015, 7:07 p.m., Sergey Shelukhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34776/ > ----------------------------------------------------------- > > (Updated June 2, 2015, 7:07 p.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/optimizer/RemoveDynamicPruningBySize.java > 5d01311 > ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezProcContext.java adc31ae > ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java 0edfc5d > ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezWork.java 6db8220 > ql/src/java/org/apache/hadoop/hive/ql/parse/TezCompiler.java 56707af > 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/test/org/apache/hive/service/cli/CLIServiceTest.java b4d517f > > Diff: https://reviews.apache.org/r/34776/diff/ > > > Testing > ------- > > > Thanks, > > Sergey Shelukhin > >
