> 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.

Following in the footsteps of slider


> 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 ?

renewer is set elsewhere


> 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.

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.


> 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 134
> > <https://reviews.apache.org/r/40315/diff/2/?file=1127517#file1127517line134>
> >
> >     Sanity checks for the values. Empty strings are not allowed.

these are passed as defaults to ZK config getters; then, they are checked when 
attempting to log in


> 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.

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


> 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.

>From the conf passed to refreshServiceAcl it looks like.


> 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.

that looks like what other services do


> 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 ?

shouldn't matter


> On Nov. 24, 2015, 3:56 a.m., Siddharth Seth wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/tezplugins/TaskCommunicator.java,
> >  line 105
> > <https://reviews.apache.org/r/40315/diff/2/?file=1127525#file1127525line105>
> >
> >     Stop logging the token.

This part doesn't contain any secrets, changed to debug


> On Nov. 24, 2015, 3:56 a.m., Siddharth Seth wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/tezplugins/TaskCommunicator.java,
> >  line 511
> > <https://reviews.apache.org/r/40315/diff/2/?file=1127525#file1127525line511>
> >
> >     Required for each host separately ? Setting the host may not be 
> > required.

I think it is. Server has to run the principal with _HOST for hadoop IPC to work


On Nov. 24, 2015, 3:56 a.m., Sergey Shelukhin wrote:
> > Haven't looked at the details of the ZKSecretManager - but it looks like 
> > the Tokens issued by any of the LLAP instances can be used by an 
> > application to communicate with all other instances.
> > Also, are the tokens the same for different applications ?

Yes, and no.


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40315/#review107706
-----------------------------------------------------------


On Nov. 16, 2015, 7:45 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40315/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2015, 7:45 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 838f25c 
>   llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapIoProxy.java 
> 4c31e32 
>   
> 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 9ab3e98 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 07f26be 
>   serde/src/java/org/apache/hadoop/hive/serde2/AbstractSerDe.java 9269ff4 
> 
> Diff: https://reviews.apache.org/r/40315/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Reply via email to