> On May 20, 2016, 2:21 a.m., Siddharth Seth wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapProtocolServerImpl.java, > > line 290 > > <https://reviews.apache.org/r/46956/diff/2/?file=1387298#file1387298line290> > > > > What user is expected over here. > > 1. In case of an invocation by HS2 to run a Tez query - I'm assuming > > this would be the HS2 service user (which is the same as the LLAP service > > user). (That needs to be validated) > > 2. In case of external services - would this be the HS2 service user or > > the user associated with the external service ? > > > > If it's the HS2 user each time, is the "user"/"realuser" field in the > > TokenIdentifier required ? That seems to be passed in as a null everywhere. > > Assuming the appId is what will be used to differentiate different > > external clients ? and that in case of Tez - there's no differentiation. > > Sergey Shelukhin wrote: > This is the calling user in case of RPC. > > Siddharth Seth wrote: > This goes back to whether it's invoked locally or remotely. > Local and Remote calls by HS2 to obtain a token on behalf of an external > client, will need to pass in the user name to generate the token correctly. > What's obtained on the RPC call will almost always be the Hive user - at > least for the remote call. > Behaviour should not change if the flag to get the token locally / > remotely is changed. > > Sergey Shelukhin wrote: > This is protobuf, it's always invoked remotely. Two paths only meet at > SecretManager level. > > Siddharth Seth wrote: > Reference was to the way the token is obtained (locally or remotely) - > not the specific call.
fixed by only retaining the remote API for CLI and removing the setting. HS2 user should have access to ZK paths, for registry if nothing else, so this shouldn't be a problem > On May 20, 2016, 2:21 a.m., Siddharth Seth wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java, > > line 61 > > <https://reviews.apache.org/r/46956/diff/2/?file=1387304#file1387304line61> > > > > I don't think we need to create a new instance of the > > ZKDelegationTokenSecretManager. > > > > The one created earlier to generate tokens should be passed in. > > > > The KeySigner could be an interface instead, and SecretManager (extends > > ZKDelegationTokenSecretManager) can implement this. ACL checks etc are > > already setup there. There's no requirement to have two independent copies > > of the ZKSecretManager running in the same daemon. > > Sergey Shelukhin wrote: > This one has completely different logic and even different template > parameter. > > Siddharth Seth wrote: > Most of the logic is in the BaseSecretManager itself, correct ? > The same instance can be used to generate tokens, as well as sign. Is > there a downside to that ? > Setting up two instances would create extra threads, and confusion while > debugging; Also potentially additional load to ZK, additional logins, etc Merged the classes. They are created separately for now, I'll add central creation in one of the subsequent patches that adds llapcoordinator. - Sergey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46956/#review134076 ----------------------------------------------------------- On May 21, 2016, 12:07 a.m., Sergey Shelukhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46956/ > ----------------------------------------------------------- > > (Updated May 21, 2016, 12:07 a.m.) > > > Review request for hive, Gunther Hagleitner, Jason Dere, and Siddharth Seth. > > > Repository: hive-git > > > Description > ------- > > see jira > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 4cfa5f1 > > llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java > f10351b > llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java > PRE-CREATION > > llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenIdentifier.java > e28eddd > > llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java > 465b204 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java > 2524dc2 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java > de817e3 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapProtocolServerImpl.java > b94fc2e > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapTokenChecker.java > 03ee055 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java > 8abd198 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorService.java > eac0e8f > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java > 74359fa > > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java > PRE-CREATION > > llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java > 279baf1 > > llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TestLlapTokenChecker.java > aaaa762 > > llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestFirstInFirstOutComparator.java > a250882 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java c9b912b > > Diff: https://reviews.apache.org/r/46956/diff/ > > > Testing > ------- > > > Thanks, > > Sergey Shelukhin > >
