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/04/28 01:01:53 UTC
Review Request 46754: HIVE-13391 add an option to LLAP to use keytab
to authenticate to read data
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/
-----------------------------------------------------------
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
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/#review130862
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/MapRecordProcessor.java (line 61)
<https://reviews.apache.org/r/46754/#comment194768>
unneeded
- Sergey Shelukhin
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
>
>
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/#review131384
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java
<https://reviews.apache.org/r/46754/#comment195333>
bogus
- Sergey Shelukhin
On May 2, 2016, 9:44 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46754/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 9:44 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 2814353
> llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java 6981061
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 3d45c7a
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java 63cb16b
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java fcfa940
> llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java fea3dc7
> 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/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
>
> Diff: https://reviews.apache.org/r/46754/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Sergey Shelukhin <se...@hortonworks.com>.
> On May 3, 2016, 3:14 p.m., Siddharth Seth wrote:
> > Looks good in terms of functionality. (Ship it if you think the reflection is not super brittle :))
> > I think we should get rid of the reflection to access private methods ASAP though - it can be really brittle and cause difficult to debug failures with different hadoop versions - ideally before a 2.1 release.
> > 1) The perf issue may not be valid, and we could get away with using a single UGI.
> > 2) A UGI pool.
> > If this gets committed as is, could you please open a follow up blocker ticket for 2.1 to remove the reflection. I can take that up at a later point.
We'd only be able to remove reflection in newer versions of Hadoop, so it will still remain in the shim, at least.
There's unfortunately no way to clone the UGI (that I see).
The problem with pools or a single UGI is that we cannot add credentials. Therefore, LLAP won't work against anything other than HDFS (e.g. HBase, etc.) in this mode... Maybe there should be a config flag for the case of bugs. However, I think UGI state is fairly stable, esp. the ctor and the getters probably won't change. We should decide, cause if we don't want reflection there's no point in committing this for now.
- Sergey
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/#review131507
-----------------------------------------------------------
On May 2, 2016, 9:44 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46754/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 9:44 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 2814353
> llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java 6981061
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 3d45c7a
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java 63cb16b
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java fcfa940
> llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java fea3dc7
> 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/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
>
> Diff: https://reviews.apache.org/r/46754/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Siddharth Seth <ss...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/#review131507
-----------------------------------------------------------
Fix it, then Ship it!
Looks good in terms of functionality. (Ship it if you think the reflection is not super brittle :))
I think we should get rid of the reflection to access private methods ASAP though - it can be really brittle and cause difficult to debug failures with different hadoop versions - ideally before a 2.1 release.
1) The perf issue may not be valid, and we could get away with using a single UGI.
2) A UGI pool.
If this gets committed as is, could you please open a follow up blocker ticket for 2.1 to remove the reflection. I can take that up at a later point.
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapSecurityHelper.java (line 95)
<https://reviews.apache.org/r/46754/#comment195512>
Wondering if this ends up being a little too brittle. HADOOP-13081 itself may break this code.
- Siddharth Seth
On May 2, 2016, 9:44 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46754/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 9:44 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 2814353
> llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java 6981061
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 3d45c7a
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java 63cb16b
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java fcfa940
> llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java fea3dc7
> 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/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
>
> Diff: https://reviews.apache.org/r/46754/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Siddharth Seth <ss...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/#review135722
-----------------------------------------------------------
Ship it!
Ship It!
- Siddharth Seth
On May 28, 2016, 2:04 a.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46754/
> -----------------------------------------------------------
>
> (Updated May 28, 2016, 2:04 a.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 6a404bd
> llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java cffa493
> 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 5ab7b3c
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java 74359fa
> llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java fea3dc7
> llap-server/src/java/org/apache/hadoop/hive/llap/io/decode/ColumnVectorProducer.java b3b571d
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapUgiFactoryFactory.java PRE-CREATION
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java e0f0676
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestFirstInFirstOutComparator.java a250882
> shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java ef2b7f7
> shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 4a96355
>
> Diff: https://reviews.apache.org/r/46754/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/
-----------------------------------------------------------
(Updated May 28, 2016, 2:04 a.m.)
Review request for hive and Siddharth Seth.
Repository: hive-git
Description
-------
see JIRA
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/common/UgiFactory.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6a404bd
llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java cffa493
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 5ab7b3c
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java 74359fa
llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java fea3dc7
llap-server/src/java/org/apache/hadoop/hive/llap/io/decode/ColumnVectorProducer.java b3b571d
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapUgiFactoryFactory.java PRE-CREATION
llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java e0f0676
llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestFirstInFirstOutComparator.java a250882
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java ef2b7f7
shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 4a96355
Diff: https://reviews.apache.org/r/46754/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Siddharth Seth <ss...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/#review134432
-----------------------------------------------------------
Any possibility of simple unit tests ?
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapUgiFactoryFactory.java (line 36)
<https://reviews.apache.org/r/46754/#comment199242>
Lets skip the reflection, and create a single UGI for now.
A follow up jira to create a UGI pool should take care of the potential single UGI perf issue.
- Siddharth Seth
On May 20, 2016, 1:49 a.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46754/
> -----------------------------------------------------------
>
> (Updated May 20, 2016, 1:49 a.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 9cc8fbe
> llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java cffa493
> 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/TaskRunnerCallable.java 74359fa
> llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java fea3dc7
> llap-server/src/java/org/apache/hadoop/hive/llap/io/decode/ColumnVectorProducer.java b3b571d
> llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapUgiFactoryFactory.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/comparator/TestFirstInFirstOutComparator.java a250882
>
> Diff: https://reviews.apache.org/r/46754/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/
-----------------------------------------------------------
(Updated May 20, 2016, 1:49 a.m.)
Review request for hive and Siddharth Seth.
Repository: hive-git
Description
-------
see JIRA
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/common/UgiFactory.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 9cc8fbe
llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java cffa493
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/TaskRunnerCallable.java 74359fa
llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java fea3dc7
llap-server/src/java/org/apache/hadoop/hive/llap/io/decode/ColumnVectorProducer.java b3b571d
llap-server/src/java/org/apache/hadoop/hive/llap/security/LlapUgiFactoryFactory.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/comparator/TestFirstInFirstOutComparator.java a250882
Diff: https://reviews.apache.org/r/46754/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/
-----------------------------------------------------------
(Updated May 3, 2016, 9:07 p.m.)
Review request for hive and Siddharth Seth.
Repository: hive-git
Description
-------
see JIRA
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/common/UgiFactory.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 06a6906
llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java 6981061
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 78b37f7
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java d23a44a
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java fcfa940
llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java fea3dc7
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 f958bc4
llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java c6ba14e
llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestFirstInFirstOutComparator.java 08ee769
Diff: https://reviews.apache.org/r/46754/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/
-----------------------------------------------------------
(Updated May 2, 2016, 9:44 p.m.)
Review request for hive and Siddharth Seth.
Repository: hive-git
Description
-------
see JIRA
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/common/UgiFactory.java PRE-CREATION
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 2814353
llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java 6981061
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java 3d45c7a
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java 63cb16b
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java fcfa940
llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java fea3dc7
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/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
Diff: https://reviews.apache.org/r/46754/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Sergey Shelukhin <se...@hortonworks.com>.
> On April 30, 2016, 7:06 a.m., Siddharth Seth wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java, line 192
> > <https://reviews.apache.org/r/46754/diff/1/?file=1364102#file1364102line192>
> >
> > 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.
What about HBase and other systems? using the keytab would mean giving that user access or not having it at all. I'll add the clone of UGI...
- Sergey
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46754/#review131247
-----------------------------------------------------------
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
>
>
Re: Review Request 46754: HIVE-13391 add an option to LLAP to use
keytab to authenticate to read data
Posted by Siddharth Seth <ss...@apache.org>.
-----------------------------------------------------------
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
>
>