HIVE-25829: Tez exec mode support for credential provider for jobs #2911
Conversation
| Stream.of( | ||
| JobConf.MAPRED_MAP_TASK_ENV, | ||
| JobConf.MAPRED_REDUCE_TASK_ENV, | ||
| MRJobConfig.MR_AM_ADMIN_USER_ENV) | ||
| MRJobConfig.MR_AM_ADMIN_USER_ENV, | ||
| "tez.am.launch.env") |
There was a problem hiding this comment.
Is this credential required for tez.task.launch.env ?
There was a problem hiding this comment.
thanks @nareshpr, I'll double-check, I tested this patch on a cluster as a POC, and I saw a simple map/reduce dag worked
also, I'm going to decide how to use this as referring to TezConfiguration.TEZ_AM_LAUNCH_ENV, which is not visible currently in hive-common
abstractdog
force-pushed
the
HIVE-25829
branch
from
3ed8dff
to
1af0a2a
Compare
abstractdog
force-pushed
the
HIVE-25829
branch
from
1af0a2a
to
a961b22
Compare
|
can I get a review on this @nareshpr , @vihangk1 , @deniskuzZ , @szlta |
| * as this config has an effect in HS2 (on TezClient codepath), and the original hadoop | ||
| * credential store should be used. | ||
| */ | ||
| HiveConfUtil.updateCredentialProviderPasswordForJobs(tezConfig); |
There was a problem hiding this comment.
Do you know if that information won't be localized by Tez in plain text?
There was a problem hiding this comment.
the credential store password appears in the launch-container.sh script, which is created by yarn, and it contains the environment variables, I think this is the same as in case of any other execution engine that's localized by yarn, but I'm sure that launch-container.sh is not included into application logs, so it's only readable of somebody having access to the cluster nodes
abstractdog
force-pushed
the
HIVE-25829
branch
from
a961b22
to
64c11d8
Compare
kgyrtkirk
added
tests pending
tests unstable
and removed
tests unstable
tests pending
labels
|
test failures are due to https://issues.apache.org/jira/browse/HIVE-25859 |
|
can I get approval or rejection on this one? looks like there is an extreme customer push on this case test failures are all due to https://issues.apache.org/jira/browse/HIVE-25859 |
abstractdog
force-pushed
the
HIVE-25829
branch
from
64c11d8
to
7ec62b8
Compare
| @@ -244,6 +256,16 @@ public static String getJobCredentialProviderPassword(Configuration conf) { | |||
| return null; | |||
| } | |||
|
|
|||
| /** | |||
| * Sets a "keyName=newKeyValue" pair to a jobConf to a given property. | |||
| * If the property is empty, is simply inserts keyName=newKeyValue, | |||
There was a problem hiding this comment.
going to fix it before committing
There was a problem hiding this comment.
I would expect to add the TEZ case as part of https://github.com/apache/hive/blob/master/ql/src/test/org/apache/hadoop/hive/ql/exec/TestHiveCredentialProviders.java
This can also be done as a follow up -- changes look straightforward to me
absolutely makes sense, added corresponding cases to the existing unit tests |
kgyrtkirk
added
tests failed
tests pending
tests passed
and removed
tests pending
tests failed
labels
…pache#2911) (Laszlo Bodor reviewed by Panagiotis Garefalakis, Denys Kuzmenko)
…pache#2911) (Laszlo Bodor reviewed by Panagiotis Garefalakis, Denys Kuzmenko)
What changes were proposed in this pull request?
Adapting the changes of HIVE-14822 to Tez.
The actual change is very small:
3. depending on HADOOP-18066, setting the hs2 job keystore password into config (+added as redacted), details are on HADOOP-18066<-- not anymore, fixed code in TezSessionStateWhy are the changes needed?
Tez support.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Tested on cluster against Azure.