----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17471/#review33496 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TestTezSessionState.java <https://reviews.apache.org/r/17471/#comment62960> I believe that file is in the wrong location. Should be in ql/test, right? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java <https://reviews.apache.org/r/17471/#comment62963> Everything is static in this class. I think it'd be better to have a singleton and non-static members. This way we could have multiple pools if desired. Also should make testing easier. ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java <https://reviews.apache.org/r/17471/#comment62964> BlockingQueue should be able to tell you length, right? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java <https://reviews.apache.org/r/17471/#comment62965> Can't sessionType denote an actual type? Class<?> is extremely general and there are no comments explaining the use. ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java <https://reviews.apache.org/r/17471/#comment62961> nit: some ws issues ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java <https://reviews.apache.org/r/17471/#comment62966> this should come from a site file not be hard coded right? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java <https://reviews.apache.org/r/17471/#comment62967> don't think this is needed ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java <https://reviews.apache.org/r/17471/#comment62968> is this the right name? shouldn't that be a yarn var? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java <https://reviews.apache.org/r/17471/#comment62969> comment doesn't match signature ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java <https://reviews.apache.org/r/17471/#comment62976> It doesn't look like you're keeping track of this sessionstate here. I think we should. The user should always get/return sessions and we handle the alloc/dealloc. (why can't return close the session for non default for instance?) service/src/java/org/apache/hive/service/server/HiveServer2.java <https://reviews.apache.org/r/17471/#comment62977> need to handle exception properly - Gunther Hagleitner On Jan. 28, 2014, 10:34 p.m., Vikram Dixit Kumaraswamy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17471/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2014, 10:34 p.m.) > > > Review request for hive. > > > Bugs: HIVE-6325 > https://issues.apache.org/jira/browse/HIVE-6325 > > > Repository: hive-git > > > Description > ------- > > Enable using multiple concurrent sessions in tez. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 84ee78f > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 9ad5986 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TestTezSessionState.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java b8552a3 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionStateFactory.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezTask.java c6f431c > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java d7edda1 > ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestTezSessionPool.java > PRE-CREATION > service/src/java/org/apache/hive/service/server/HiveServer2.java fa13783 > > Diff: https://reviews.apache.org/r/17471/diff/ > > > Testing > ------- > > Added multi-threaded junit tests. > > > Thanks, > > Vikram Dixit Kumaraswamy > >
