----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46754/#review131507 -----------------------------------------------------------
Fix it, then Ship it! Looks good in terms of functionality. (Ship it if you think the reflection is not super brittle :)) I think we should get rid of the reflection to access private methods ASAP though - it can be really brittle and cause difficult to debug failures with different hadoop versions - ideally before a 2.1 release. 1) The perf issue may not be valid, and we could get away with using a single UGI. 2) A UGI pool. If this gets committed as is, could you please open a follow up blocker ticket for 2.1 to remove the reflection. I can take that up at a later point. llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSecurityHelper.java (line 95) <https://reviews.apache.org/r/46754/#comment195512> Wondering if this ends up being a little too brittle. HADOOP-13081 itself may break this code. - Siddharth Seth On May 2, 2016, 9:44 p.m., Sergey Shelukhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46754/ > ----------------------------------------------------------- > > (Updated May 2, 2016, 9:44 p.m.) > > > Review request for hive and Siddharth Seth. > > > Repository: hive-git > > > Description > ------- > > see JIRA > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/UgiFactory.java PRE-CREATION > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 2814353 > > llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java > 6981061 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java > 3d45c7a > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java > 63cb16b > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java > fcfa940 > > llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java > fea3dc7 > > llap-server/src/java/org/apache/hadoop/hive/llap/io/decode/ColumnVectorProducer.java > b3b571d > > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSecurityHelper.java > 76ba225 > > llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java > 24f4442 > > llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestFirstInFirstOutComparator.java > 08ee769 > > Diff: https://reviews.apache.org/r/46754/diff/ > > > Testing > ------- > > > Thanks, > > Sergey Shelukhin > >
