[ 
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: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to