[
https://issues.apache.org/jira/browse/YARN-8071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16439791#comment-16439791
]
Jim Brennan commented on YARN-8071:
-----------------------------------
[~jlowe] thanks for the review.
{quote}The original code passed the current environment map, allowing admin
variables to reference other variables defined so far in the environment. The
new code passes an empty map which would seem to preclude this and could be a
backwards compatibility issue.
{quote}
Thanks for pointing this out. I meant to ask specifically about this change.
This was intentional. I agree it is a change in functionality, but it seemed to
me that the current behavior may actually be a bug, not the intended behavior.
I based this on the documentation for {{yarn.nodemanager.admin-env}}, the
comment that precedes this code ({{variables here will be forced in, even if
the container has specified them.}}, and the fact that everything else in this
function overrides any user-specified variable (with the exception of the
windows-specific classpath stuff).
That said, I don't have a good idea of how likely this change would be to break
something, so I am definitely willing to change it if it is considered too
dangerous.
{quote}The changes to TestContainerLaunch#testPrependDistcache appear to be
unnecessary?
{quote}
They were intentional. When I was testing my new test case, I realized that
passing the empty set for the {{nmVars}} argument leads to exceptions in
{{addToEnvMap()}}, so I fixed the testPrependDistcache() cases as well - I
assume this windows-only test must be failing without this fix.
> Add ability to specify nodemanager environment variables individually
> ---------------------------------------------------------------------
>
> Key: YARN-8071
> URL: https://issues.apache.org/jira/browse/YARN-8071
> Project: Hadoop YARN
> Issue Type: Bug
> Components: yarn
> Affects Versions: 3.0.0
> Reporter: Jim Brennan
> Assignee: Jim Brennan
> Priority: Major
> Attachments: YARN-8071.001.patch, YARN-8071.002.patch
>
>
> YARN-6830 describes a problem where environment variables that contain commas
> cannot be specified via {{-Dmapreduce.map.env}}.
> For example:
> {{-Dmapreduce.map.env="MODE=bar,IMAGE_NAME=foo,MOUNTS=/tmp/foo,/tmp/bar"}}
> will set {{MOUNTS}} to {{/tmp/foo}}
> In that Jira, [~aw] suggested that we change the API to provide a way to
> specify environment variables individually, the same way that Spark does.
> {quote}Rather than fight with a regex why not redefine the API instead?
>
> -Dmapreduce.map.env.MODE=bar
> -Dmapreduce.map.env.IMAGE_NAME=foo
> -Dmapreduce.map.env.MOUNTS=/tmp/foo,/tmp/bar
> ...
> e.g, mapreduce.map.env.[foo]=bar gets turned into foo=bar
> This greatly simplifies the input validation needed and makes it clear what
> is actually being defined.
> {quote}
> The mapreduce properties were dealt with in [MAPREDUCE-7069]. This Jira will
> address the YARN properties.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
