You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sergey Shelukhin <se...@hortonworks.com> on 2016/05/18 23:03:58 UTC
Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to LLAPIF
splits
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/
-----------------------------------------------------------
Review request for hive, Jason Dere and Siddharth Seth.
Repository: hive-git
Description
-------
see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
Diffs
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java cbb3a72
llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java f10351b
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
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/LlapTokenLocalClient.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenProvider.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 465b204
llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 349ee14
llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
llap-server/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinatorImpl.java PRE-CREATION
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/LlapSecurityHelper.java PRE-CREATION
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
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java c9b912b
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java ce69ee6
service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
Diff: https://reviews.apache.org/r/47563/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Sergey Shelukhin <se...@hortonworks.com>.
> On May 31, 2016, 11:09 p.m., Siddharth Seth wrote:
> > llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java, line 350
> > <https://reviews.apache.org/r/47563/diff/3/?file=1399778#file1399778line350>
> >
> > Is a signature always expected ? If so, may as well convert this to a precondition.
> > Otherwise, in what case will it not be expected ?
Unsecure case?
> On May 31, 2016, 11:09 p.m., Siddharth Seth wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java, line 40
> > <https://reviews.apache.org/r/47563/diff/3/?file=1399781#file1399781line40>
> >
> > Sharing this with the main SecretManager will be in some other patch ?
yeah
> On May 31, 2016, 11:09 p.m., Siddharth Seth wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java, line 58
> > <https://reviews.apache.org/r/47563/diff/3/?file=1399781#file1399781line58>
> >
> > Need to look at what this 'user' is used for. I'm not sure why the field is actually needed.
it's removed in some later patch
- Sergey
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135706
-----------------------------------------------------------
On May 27, 2016, 9:32 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated May 27, 2016, 9:32 p.m.)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6a404bd
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java f10351b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 465b204
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 349ee14
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinatorImpl.java PRE-CREATION
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 2524dc2
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java PRE-CREATION
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java c9b912b
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 83d492a
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Sergey Shelukhin <se...@hortonworks.com>.
> On May 31, 2016, 11:09 p.m., Siddharth Seth wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 2880
> > <https://reviews.apache.org/r/47563/diff/3/?file=1399766#file1399766line2880>
> >
> > Is this needed ? Can we create the instance on first access instead of adding a config.
We may not have a daemon config when we ask for coordinator, and if we do pass in a config it makes the API brittle cause the question is whether we should handle different configs...
I think it's better to initialize everything at init time. There's a comment somewhere saying it should also be passed around like in normal OOP, not accessed via a global.
- Sergey
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135706
-----------------------------------------------------------
On May 27, 2016, 9:32 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated May 27, 2016, 9:32 p.m.)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6a404bd
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java f10351b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 465b204
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 349ee14
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinatorImpl.java PRE-CREATION
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 2524dc2
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java PRE-CREATION
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java c9b912b
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 83d492a
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Siddharth Seth <ss...@apache.org>.
- Siddharth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135706
-----------------------------------------------------------
On June 1, 2016, 9:23 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated June 1, 2016, 9:23 p.m.)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b2e6b6f
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java af889b6
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java 478a40a
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 5aa4b84
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java 067a98e
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 1df6df0
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java d439c07
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java 4174593
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java 0420225
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java d04cfa3
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 140dbda
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Sergey Shelukhin <se...@hortonworks.com>.
> On May 31, 2016, 11:09 p.m., Siddharth Seth wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java, line 381
> > <https://reviews.apache.org/r/47563/diff/3/?file=1399784#file1399784line381>
> >
> > Not sure why the user is required in the client. Seems like this will cause all kinds of issues with doAs=true.
> > Multiple llap instances is not supported yet - can avoid adding bits and pieces towards it.
cluster name in ZK paths/etc. includes user name... multiple LLAP clusters are supported for different users. Anyway it's a simple thing
> On May 31, 2016, 11:09 p.m., Siddharth Seth wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java, line 398
> > <https://reviews.apache.org/r/47563/diff/3/?file=1399785#file1399785line398>
> >
> > This seems like a guaranteed NPE. The user will be set in a proto - which will throw a NullPointerException.
> >
> > Hopefully this will be caught by some tests.
> >
> > Would be good to add a couple of tests here as well.
it's a magical UDTF living in HS2... kinda hard to unit test. We can file a jira to refactor
- Sergey
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135706
-----------------------------------------------------------
On May 27, 2016, 9:32 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated May 27, 2016, 9:32 p.m.)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6a404bd
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java f10351b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 465b204
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 349ee14
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinatorImpl.java PRE-CREATION
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 2524dc2
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java PRE-CREATION
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java c9b912b
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 83d492a
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Siddharth Seth <ss...@apache.org>.
> On May 31, 2016, 11:09 p.m., Siddharth Seth wrote:
> > llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java, line 350
> > <https://reviews.apache.org/r/47563/diff/3/?file=1399778#file1399778line350>
> >
> > Is a signature always expected ? If so, may as well convert this to a precondition.
> > Otherwise, in what case will it not be expected ?
>
> Sergey Shelukhin wrote:
> Unsecure case?
A comnment in the code mentions that TaskSpec will be used for the unsecure case. If we're using the vertexBinary for unsecure - that's even better, since it stays consistent. (There's a comment indicating the same somewhere else in the review)
- Siddharth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135706
-----------------------------------------------------------
On June 1, 2016, 9:23 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated June 1, 2016, 9:23 p.m.)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b2e6b6f
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java af889b6
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java 478a40a
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 5aa4b84
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java 067a98e
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 1df6df0
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java d439c07
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java 4174593
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java 0420225
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java d04cfa3
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 140dbda
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Siddharth Seth <ss...@apache.org>.
> On May 31, 2016, 11:09 p.m., Siddharth Seth wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java, line 381
> > <https://reviews.apache.org/r/47563/diff/3/?file=1399784#file1399784line381>
> >
> > Not sure why the user is required in the client. Seems like this will cause all kinds of issues with doAs=true.
> > Multiple llap instances is not supported yet - can avoid adding bits and pieces towards it.
>
> Sergey Shelukhin wrote:
> cluster name in ZK paths/etc. includes user name... multiple LLAP clusters are supported for different users. Anyway it's a simple thing
Any thoughts around doAs=true. Won't the ZK path constructed in this case be incorrect ?
That's officially not supported. Maybe we should have a check for this somewhere (not related to this patch) saying - If LLAP is being used, ensure doAs is false.
- Siddharth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135706
-----------------------------------------------------------
On June 1, 2016, 9:23 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated June 1, 2016, 9:23 p.m.)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b2e6b6f
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java af889b6
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java 478a40a
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 5aa4b84
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java 067a98e
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 1df6df0
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java d439c07
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java 4174593
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java 0420225
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java d04cfa3
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 140dbda
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Siddharth Seth <ss...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135706
-----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 2880)
<https://reviews.apache.org/r/47563/#comment200742>
Is this needed ? Can we create the instance on first access instead of adding a config.
llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java (line 46)
<https://reviews.apache.org/r/47563/#comment200744>
We should get rid of TaskSpec. Adding vertex parallelism to this allows for that.
If TaskSpec is required in non-secure mode - it can become a union like the payload is.
TaskSpec is otherwise a very heavyweight object.
Once all the task number attempt number etc is set by the client - we can send VertexSpec only once - instead of for each Task (that would be a reasonably perf enhancement for a future jira)
llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java (lines 146 - 147)
<https://reviews.apache.org/r/47563/#comment200746>
Nit: Rename to getVertexBinary ?
llap-common/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java (line 28)
<https://reviews.apache.org/r/47563/#comment200748>
Javadoc would be useful. (Seems to be in the impl isntead of here), especially given that LlapCooridnator is really easy to confuse for something that does fragment handling.
This is more like a LlapSecurityCoordinator / LlapSecurityHelper at the moment.
llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java (line 315)
<https://reviews.apache.org/r/47563/#comment200749>
Is a signature always expected ? If so, may as well convert this to a precondition.
Otherwise, in what case will it not be expected ?
llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java (line 324)
<https://reviews.apache.org/r/47563/#comment200750>
This can be replaced by 'taskNum' - that leaves only vertexParallelism from TaskSpec - which can be included in the Info object.
llap-server/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinatorImpl.java (line 1)
<https://reviews.apache.org/r/47563/#comment200751>
Should this be in llap-server, or llap-common/llap-client.
Don't think HS2 needs to depend on llap-server for anything.
llap-server/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinatorImpl.java (line 138)
<https://reviews.apache.org/r/47563/#comment200753>
Catch Exception instead of Throwable ?
Also log the message so that we know what's going on if this happens.
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java (line 40)
<https://reviews.apache.org/r/47563/#comment200755>
Sharing this with the main SecretManager will be in some other patch ?
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java (line 58)
<https://reviews.apache.org/r/47563/#comment200757>
Need to look at what this 'user' is used for. I'm not sure why the field is actually needed.
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java (line 74)
<https://reviews.apache.org/r/47563/#comment200752>
Log on close ? At INFO at least initially, later at debug
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java (line 353)
<https://reviews.apache.org/r/47563/#comment200758>
Not sure why the user is required in the client. Seems like this will cause all kinds of issues with doAs=true.
Multiple llap instances is not supported yet - can avoid adding bits and pieces towards it.
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java (line 328)
<https://reviews.apache.org/r/47563/#comment200760>
Null check ?
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java (line 339)
<https://reviews.apache.org/r/47563/#comment200762>
Don't think this is enforced at the moment.
Is it required. Can we use the SignedVertexSpec with and without security to have a consistent code path ?
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java (line 389)
<https://reviews.apache.org/r/47563/#comment200763>
Nit: rename to createSignedVertexSpec
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java (line 392)
<https://reviews.apache.org/r/47563/#comment200764>
This seems like a guaranteed NPE. The user will be set in a proto - which will throw a NullPointerException.
Hopefully this will be caught by some tests.
Would be good to add a couple of tests here as well.
- Siddharth Seth
On May 27, 2016, 9:32 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated May 27, 2016, 9:32 p.m.)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6a404bd
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java f10351b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 465b204
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 349ee14
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinatorImpl.java PRE-CREATION
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 2524dc2
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java PRE-CREATION
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java c9b912b
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 83d492a
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Siddharth Seth <ss...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135891
-----------------------------------------------------------
Ship it!
Looks good. I'll create a follow up to remove the unnecessary TaskSpec creation, and potentially skipping the unnecessary VertexSpec being sent each time as a response from GenericUDTFGetSplits.
- Siddharth Seth
On June 2, 2016, midnight, Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated June 2, 2016, midnight)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b2e6b6f
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java af889b6
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java 478a40a
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 540f978
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java 067a98e
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 1df6df0
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java d439c07
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java 4174593
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java 0420225
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java d04cfa3
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 140dbda
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135988
-----------------------------------------------------------
llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java (line 139)
<https://reviews.apache.org/r/47563/#comment200998>
bogus comment
- Sergey Shelukhin
On June 2, 2016, 7:41 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated June 2, 2016, 7:41 p.m.)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java ff5c858
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java 0edb1cd
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java af889b6
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java 478a40a
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 540f978
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java 067a98e
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 1df6df0
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java d439c07
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java 4174593
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java 0420225
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java d04cfa3
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 140dbda
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/
-----------------------------------------------------------
(Updated June 2, 2016, 7:41 p.m.)
Review request for hive, Jason Dere and Siddharth Seth.
Repository: hive-git
Description
-------
see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java ff5c858
llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java 0edb1cd
llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java af889b6
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java 478a40a
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 540f978
llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java 067a98e
llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 1df6df0
llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java d439c07
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java 4174593
llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java 0420225
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java d04cfa3
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 140dbda
service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
Diff: https://reviews.apache.org/r/47563/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/
-----------------------------------------------------------
(Updated June 2, 2016, midnight)
Review request for hive, Jason Dere and Siddharth Seth.
Repository: hive-git
Description
-------
see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b2e6b6f
llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java af889b6
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java 478a40a
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 540f978
llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java 067a98e
llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 1df6df0
llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java d439c07
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java 4174593
llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java 0420225
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java d04cfa3
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 140dbda
service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
Diff: https://reviews.apache.org/r/47563/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Sergey Shelukhin <se...@hortonworks.com>.
> On June 1, 2016, 10:14 p.m., Siddharth Seth wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java, line 348
> > <https://reviews.apache.org/r/47563/diff/4/?file=1403574#file1403574line348>
> >
> > I don't see where this is being enforced ? The signer is used irrespective of secure/non-secure.
> >
> > Are we going with the approach of sending a serialized VertexSpec in non-secure (without a signatre), or sending the TaskSpec ?
> >
> > Scratch that. Looks like it's being checked in createSignedVertexSpec. The comment still seems incorrect though, since TaskSpec is never sent any longer.
> >
> > Assuming the serialized spec without a signature works in LLAP (i.e. when the daemon gets the VertexSpecBinary without a signature - it handles it correctly)
the comment is wrong
- Sergey
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135873
-----------------------------------------------------------
On June 1, 2016, 9:23 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated June 1, 2016, 9:23 p.m.)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b2e6b6f
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java af889b6
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java 478a40a
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 5aa4b84
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java 067a98e
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 1df6df0
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java d439c07
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java 4174593
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java 0420225
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java d04cfa3
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 140dbda
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Siddharth Seth <ss...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/#review135873
-----------------------------------------------------------
llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java (line 44)
<https://reviews.apache.org/r/47563/#comment200901>
Remove TaskSpec from the cosntructor also, and a corresponding change in GenericUDTFGetSplits to construct TaskSpec only once if possible (otherwise that's a separate jira)
llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java (line 146)
<https://reviews.apache.org/r/47563/#comment200905>
Does this need to be volatile or an AtomicReference ?
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java (line 341)
<https://reviews.apache.org/r/47563/#comment200902>
I don't see where this is being enforced ? The signer is used irrespective of secure/non-secure.
Are we going with the approach of sending a serialized VertexSpec in non-secure (without a signatre), or sending the TaskSpec ?
Scratch that. Looks like it's being checked in createSignedVertexSpec. The comment still seems incorrect though, since TaskSpec is never sent any longer.
Assuming the serialized spec without a signature works in LLAP (i.e. when the daemon gets the VertexSpecBinary without a signature - it handles it correctly)
- Siddharth Seth
On June 1, 2016, 9:23 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47563/
> -----------------------------------------------------------
>
> (Updated June 1, 2016, 9:23 p.m.)
>
>
> Review request for hive, Jason Dere and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b2e6b6f
> llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
> llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
> llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java af889b6
> llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java 478a40a
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 5aa4b84
> llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java 067a98e
> llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
> llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 1df6df0
> llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java d439c07
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java 4174593
> llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java 0420225
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java d04cfa3
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 140dbda
> service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
>
> Diff: https://reviews.apache.org/r/47563/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/
-----------------------------------------------------------
(Updated June 1, 2016, 9:23 p.m.)
Review request for hive, Jason Dere and Siddharth Seth.
Repository: hive-git
Description
-------
see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b2e6b6f
llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
llap-client/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java af889b6
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java 478a40a
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 5aa4b84
llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java 067a98e
llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 1df6df0
llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java d439c07
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java 4174593
llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java 0420225
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java d04cfa3
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 140dbda
service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
Diff: https://reviews.apache.org/r/47563/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/
-----------------------------------------------------------
(Updated May 27, 2016, 9:32 p.m.)
Review request for hive, Jason Dere and Siddharth Seth.
Repository: hive-git
Description
-------
see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6a404bd
llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java f10351b
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/SecretManager.java 465b204
llap-common/src/java/org/apache/hadoop/hive/llap/security/SigningSecretManager.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 349ee14
llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
llap-server/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinatorImpl.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 2524dc2
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
llap-server/src/test/org/apache/hadoop/hive/llap/security/TestLlapSignerImpl.java PRE-CREATION
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java c9b912b
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 83d492a
service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
Diff: https://reviews.apache.org/r/47563/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 47563: HIVE-13675 LLAP: add HMAC signatures to
LLAPIF splits
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47563/
-----------------------------------------------------------
(Updated May 18, 2016, 11:04 p.m.)
Review request for hive, Jason Dere and Siddharth Seth.
Repository: hive-git
Description
-------
see JIRA. Please ignore the first iteration, RB doesn't allow base patches on submit
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java cbb3a72
llap-client/src/java/org/apache/hadoop/hive/llap/SubmitWorkInfo.java 6704294
llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapProxy.java 6c2618b
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java f10351b
llap-client/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClientImpl.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinator.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapSigner.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenLocalClient.java PRE-CREATION
llap-common/src/java/org/apache/hadoop/hive/llap/tez/Converters.java e43b72b
llap-common/src/test/org/apache/hadoop/hive/llap/tez/TestConverters.java 349ee14
llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java 4306c22
llap-server/src/java/org/apache/hadoop/hive/llap/coordinator/LlapCoordinatorImpl.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 2524dc2
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSignerImpl.java PRE-CREATION
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java 026df3b
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java c9b912b
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java ce69ee6
service/src/java/org/apache/hive/service/server/HiveServer2.java d61edf5
Diff: https://reviews.apache.org/r/47563/diff/
Testing
-------
Thanks,
Sergey Shelukhin