----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46754/#review131247 -----------------------------------------------------------
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java (line 192) <https://reviews.apache.org/r/46754/#comment195148> If we're using a common UGI across all tasks - which is the kerberos UGI - I don't think we should add these credentials. That'll end up leaking credentials across tasks - and in general is not required assuming SQL standard auth. For non-sql standard auth - we could continue with credentials only. llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java (line 255) <https://reviews.apache.org/r/46754/#comment195147> This is the FileSyste.closeAllForUgi which will have to be handled. llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSecurityHelper.java (line 122) <https://reviews.apache.org/r/46754/#comment195149> Why is all of this synchronization and class level statics required ? Will the principal / keytab path change or a running daemon. Otherwise this seems to be an unnecessary lock across running fragments. Think it'll be simpler to create a single UGI at startup for task execution, and then share that across tasks. Alternately, if a single FS instance is a problem - create a pool of UGI instances - 1 per executor, and share those. The allowCache, static lock, compare principal seems unnecessary. - Siddharth Seth On April 27, 2016, 11:01 p.m., Sergey Shelukhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46754/ > ----------------------------------------------------------- > > (Updated April 27, 2016, 11:01 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 5360ed4 > > llap-client/src/java/org/apache/hadoop/hive/llap/registry/ServiceInstanceSet.java > 73f94f3 > > llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java > 6af30d4 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java > e80fb15 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java > 33b41e8 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapProtocolServerImpl.java > e99e689 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java > 2a60123 > > llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java > 6a72b4c > > 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/java/org/apache/hadoop/hive/llap/security/SecretManager.java > 8c7a539 > > 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 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java 8aca779 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/MapRecordProcessor.java > 0584ad8 > > Diff: https://reviews.apache.org/r/46754/diff/ > > > Testing > ------- > > > Thanks, > > Sergey Shelukhin > >
