[
https://issues.apache.org/jira/browse/MAPREDUCE-7069?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16425908#comment-16425908
]
Jim Brennan commented on MAPREDUCE-7069:
----------------------------------------
[~jlowe] thanks for the thorough review. Much appreciated!
{quote}It's a bit odd and inefficient that setVMEnv calls
MRApps.setEnvFromInputProperty twice. I think it would be clearer and more
efficient to call it once, place the results in a temporary map (like it
already does in the second call), then only set HADOOP_ROOT_LOGGER and
HADOOP_CLIENT_OPTS in the environment if they are not set in the temporary map.
Then at the end we can simply call addAll to dump the contents of the temporary
map into the environment map.
{quote}
The reason it was done with two calls is because of the way environment
variables are handled when they are already defined in the environment map. If
you an environment variable that you are updating already exists in the
environment, setEnvFromInput* functions append the new value to the existing
value, using the appropriate separator. The special handling for
HADOOP_ROOT_LOGGER and HADOOP_CLIENT_OPTS is to overwrite them instead of
appending. That said, I can definitely change it to do it the way you suggest,
except I can't just use addAll() - you ultimately need to use
Apps.addToEnvironment on each k/v pair. I could expose an
Apps.setEnvFromInputStringMapNoExpand() (or add a noExpand boolean to the
existing one) to handle this though.
Thanks for the documentation/comment recommendations - I was going to ask about
that - I'll clean those up.
{quote}Nit: setEnvFromInputStringMap does not need to be public.
{quote}
Will fix. In an earlier iteration I was calling this directly.
{quote}Would it be easier to call tmpEnv.addAll(inputMap) and pass tmpEnv
instead of inputMap? Then we don't need to explicitly iterate the map.
{quote}
Yes. I will make this change.
> Add ability to specify user environment variables individually
> --------------------------------------------------------------
>
> Key: MAPREDUCE-7069
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-7069
> Project: Hadoop Map/Reduce
> Issue Type: Improvement
> Reporter: Jim Brennan
> Assignee: Jim Brennan
> Priority: Major
> Attachments: MAPREDUCE-7069.001.patch, MAPREDUCE-7069.002.patch
>
>
> As reported in YARN-6830, it is currently not possible to specify an
> environment variable that contains commas via {{mapreduce.map.env}},
> mapreduce.reduce.env, or {{mapreduce.admin.user.env}}.
> To address this, [~aw] proposed in [YARN-6830] that we add the ability to
> specify environment variables individually:
> {quote}e.g, mapreduce.map.env.[foo]=bar gets turned into foo=bar
> {quote}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: mapreduce-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: mapreduce-issues-h...@hadoop.apache.org
