[ 
https://issues.apache.org/jira/browse/YARN-10607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17279200#comment-17279200
 ] 

Jim Brennan commented on YARN-10607:
------------------------------------

Thanks [~ebadger].  This looks good overall, but I have one comment.  The unit 
test you added only runs on Windows, which I think is correct, but the code in 
ContainerLaunch.sanitizeEnv() runs on        
 windows as well.  I'm not sure this feature really makes sense on windows, and 
in particular this line is certainly not correct for windows:
{noformat}
Apps.addToEnvironment(environment, Environment.PATH.name(),
            "$PATH", File.pathSeparator);
{noformat}
My suggestion is to put the force-path code inside a {{!Shell.WINDOWS}} check.
We might want to update the documentation as well to note it is ignored on 
windows.

> User environment is unable to prepend PATH when mapreduce.admin.user.env also 
> sets PATH
> ---------------------------------------------------------------------------------------
>
>                 Key: YARN-10607
>                 URL: https://issues.apache.org/jira/browse/YARN-10607
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: Eric Badger
>            Assignee: Eric Badger
>            Priority: Major
>         Attachments: YARN-10607.001.patch, YARN-10607.002.patch, 
> YARN-10607.003.patch
>
>
> When using the tarball approach to ship relevant Hadoop jars to containers, 
> it is helpful to set {{mapreduce.admin.user.env}} to something like 
> {{PATH=./hadoop-tarball:\{\{PATH\}\}}} to make sure that all of the Hadoop 
> binaries are on the PATH. This way you can call {{hadoop}} instead of 
> {{./hadoop-tarball/hadoop}}. The intention here is to force prepend 
> {{./hadoop-tarball}} and then append the set {{PATH}} afterwards. But if a 
> user would like to override the appended portion of {{PATH}} in their 
> environment, they are unable to do so. This is because {{PATH}} ends up 
> getting parsed twice. Initially it is set via {{mapreduce.admin.user.env}} to 
> {{PATH=./hadoop-tarball:$SYS_PATH}}}. In this case {{SYS_PATH}} is what I'll 
> refer to as the normal system path. E.g. {{/usr/local/bin:/usr/bin}}, etc.
> After this, the user env parsing happens. For example, let's say the user 
> sets their {{PATH}} to {{PATH=.:$PATH}}. We have already parsed {{PATH}} from 
> the admin.user.env. Then we go to parse the user environment and find the 
> user also specified {{PATH}}. So {{$PATH}} ends up getting getting expanded 
> to {{./hadoop-tarball:$SYS_PATH}}, which leads to the user's {{PATH}} being 
> {{PATH=.:./hadoop-tarball:$SYS_PATH}}. We then append this to {{PATH}}, which 
> has already been set in the environment map via the admin.user.env. So we 
> finally end up with 
> {{PATH=./hadoop-tarball:$SYS_PATH:.:./hadoop-tarball:$SYS_PATH}}. 
> This normally isn't a huge deal, but if you want to ship a version of 
> python/perl/etc. that clashes with the one that is already there in 
> {{SYS_PATH}}, you will need to refer to it by its full path. Since in the 
> above example, {{.}} doesn't appear until after {{$SYS_PATH}}. This is a pain 
> and it should be possible to prepend its {{PATH}} to override the 
> system/container {{SYS_PATH}}, even when also forcefully prepending to 
> {{PATH}} with you hadoop tarball.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to