> On Nov. 24, 2015, 3:56 a.m., Siddharth Seth wrote: > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 2361 > > <https://reviews.apache.org/r/40315/diff/2/?file=1127510#file1127510line2361> > > > > Don't think the default value - "*" - has any significance here. > > Replace by null - to avoid confusion. > > Sergey Shelukhin wrote: > Following in the footsteps of slider
Can we drop the * in favor of a null. (or no default if Hive supports that). > On Nov. 24, 2015, 3:56 a.m., Siddharth Seth wrote: > > llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenIdentifier.java, > > line 71 > > <https://reviews.apache.org/r/40315/diff/2/?file=1127512#file1127512line71> > > > > Does a renewer for a token type have to be specified ? > > Sergey Shelukhin wrote: > renewer is set elsewhere Not the user which does the renewal. Implementation of the Renewer interface. See TrivialRenewer in Token.java. Not sure if one is required for each token kind - would be safer to add one which accepts the LLAP token kind and says it's not managed for now. > On Nov. 24, 2015, 3:56 a.m., Siddharth Seth wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemonProtocolClientImpl.java, > > line 129 > > <https://reviews.apache.org/r/40315/diff/2/?file=1127516#file1127516line129> > > > > This could be moved into it's own protocol (but listening on the same > > server). > > > > The methods so far are for access from the AM. > > > > getTokens is to be used by Clients. > > > > What that also allows is for the annotations to change. > > getTokens() - protected by Kerberos, and cannot be obtained using a > > token. > > Remaining methods - require a token. > > Sergey Shelukhin wrote: > why add an extra protocol? it seems like most services don't handle > tokens like this. HDFS gives out tokens as part of normal interface. HDFS also doesn't have a separation of API vs server side jars. Reason for separation is that they're very different operations - one relates to exuecuting and tracking work, the other to access. Consider the case where there is a central service which is responsible for handing out these delegation tokens (rotation, etc etc). That will definitely not implement the submitWork APIs, and the daemons cannot implement the getTokns at that point. Doesn't need to be done rightnow, and can be changed later since this API is private for the moment (It is private, right?) Re-opened primarily for the "Is this private" bit. > On Nov. 24, 2015, 3:56 a.m., Siddharth Seth wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemonProtocolServerImpl.java, > > line 251 > > <https://reviews.apache.org/r/40315/diff/2/?file=1127517#file1127517line251> > > > > YARN can take care of renewing delegation tokens - assuming the service > > supports it (i.e. the ZKSecretManager on one of the LLAP instances or a > > direct connection to ZK from the RM - but that isn't a good idea). > > Eventually, I believe the renweer would need to change to the RM > > service user. > > Sergey Shelukhin wrote: > Hmm... not sure how this would work. Can you file a follow-up JIRA? By MR > logic, the renewer would be a central job manager, e.g. HS2 YARN (The RM) is given tokens for a job. It checks whether these tokens are managed or not (via the Renewer interface), and takes care of renewing them while the job is alive. That's what is done for HDFS delegation tokens anyway - renewed by the RM every day while the job is running. The same could be done for LLAP - but this is for later. Opening a jira to track renewal of tokens. > On Nov. 24, 2015, 3:56 a.m., Siddharth Seth wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapDaemonPolicyProvider.java, > > line 26 > > <https://reviews.apache.org/r/40315/diff/2/?file=1127519#file1127519line26> > > > > How is the default value picked up ? (definitely not from the hive conf) > > OR > > What is the default value - "*" or " ". > > I'm not sure how other services handle this - but this can be set to " > > " by default on secure clusters, and "*" on non-secure clusters. > > Sergey Shelukhin wrote: > From the conf passed to refreshServiceAcl it looks like. What if the fiels is not set ? > On Nov. 24, 2015, 3:56 a.m., Siddharth Seth wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapDaemonPolicyProvider.java, > > line 32 > > <https://reviews.apache.org/r/40315/diff/2/?file=1127519#file1127519line32> > > > > clone not required. > > Sergey Shelukhin wrote: > that looks like what other services do Looking at yarn services, this isn't required. A final array is being returned - this was likely changed to get past findbugs warnings, and ignored in YARN > On Nov. 24, 2015, 3:56 a.m., Siddharth Seth wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSecurityHelper.java, > > line 53 > > <https://reviews.apache.org/r/40315/diff/2/?file=1127520#file1127520line53> > > > > This would matter when running under HiveServer ? or is the > > synchronization in LlapIoProxy taking care of this ? > > Sergey Shelukhin wrote: > shouldn't matter why ? Multiple threads launching AMs and accessing Proxy.getTokens() - Siddharth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40315/#review107706 ----------------------------------------------------------- On Nov. 24, 2015, 11:11 p.m., Sergey Shelukhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40315/ > ----------------------------------------------------------- > > (Updated Nov. 24, 2015, 11:11 p.m.) > > > Review request for hive, Gopal V and Siddharth Seth. > > > Repository: hive-git > > > Description > ------- > > see JIRA > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java fffedd9 > llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapIoProxy.java > 4c31e32 > llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java > PRE-CREATION > > llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenIdentifier.java > PRE-CREATION > > llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenProvider.java > PRE-CREATION > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/LlapDaemonProtocolBlockingPB.java > 5ad2344 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java > 98b1ccd > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemonProtocolClientImpl.java > 4b13277 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemonProtocolServerImpl.java > 784c631 > > llap-server/src/java/org/apache/hadoop/hive/llap/protocol/LlapTaskUmbilicalProtocol.java > fae7654 > > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapDaemonPolicyProvider.java > PRE-CREATION > > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSecurityHelper.java > PRE-CREATION > > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapServerSecurityInfo.java > PRE-CREATION > > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapTokenSelector.java > PRE-CREATION > > llap-server/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java > d327fc0 > > llap-server/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapUmbilicalPolicyProvider.java > PRE-CREATION > > llap-server/src/java/org/apache/hadoop/hive/llap/tezplugins/TaskCommunicator.java > 33e998c > > llap-server/src/main/resources/META-INF/services/org.apache.hadoop.security.SecurityInfo > PRE-CREATION > llap-server/src/protobuf/LlapDaemonProtocol.proto 0ba6acf > > llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TestLlapDaemonProtocolServerImpl.java > 8d45c95 > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java a210b95 > ql/src/java/org/apache/hadoop/hive/ql/exec/GlobalWorkMapFactory.java > 59ee347 > ql/src/java/org/apache/hadoop/hive/ql/exec/ObjectCacheFactory.java 3d9771a > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/MapRecordProcessor.java > 914b4e7 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/ReduceRecordProcessor.java > efcf88c > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 07f26be > ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 3feab1a > serde/src/java/org/apache/hadoop/hive/serde2/AbstractSerDe.java 9269ff4 > > Diff: https://reviews.apache.org/r/40315/diff/ > > > Testing > ------- > > > Thanks, > > Sergey Shelukhin > >
