[
https://issues.apache.org/jira/browse/YARN-5714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15753981#comment-15753981
]
Remi Catherinot commented on YARN-5714:
---------------------------------------
Hi, thanks for the review too :)
# are you sure about the double % ? i've tested it on windows 7, 8, 8.1 and 10
and double-ing the % does not escape it.
{noformat}
set A=toto
set B=%A%
set C=%%A%%
echo %A% %B% %C%
{noformat}
produce _toto %toto% %toto%_, so i always got _A_'s value, so both _B_ and _C_
depend on _A_.
# I still prefer to keep it the way it is because it is the last line of the
while that does this test. Personnally when i read code that double check
things I tend to think the code is still under dev and not yet production-ready
and then i start to tripple check everything i read. So for me, such double
check would lead in code maintenance/review time increase. if this is really a
blocking issue and does not permit the patch to be accepted, then i'll change
it.
# I've reworked the code to match you algorithm so the comment (and its tipo)
no longer exist
# I've tested it with deps in reverse order and having each env entry depending
on each of its successors and with 100 entries. On my computer it returns in 0
or 16ms (no lesser time precision). My algorithm start to be slower in a
perceptible way with the same test case when we have more than 200 entries. it
hits the second for computation when i have more than ~700 entries. So I've
implemented the new algorithm (more or less, i use recursion to handle the
stack stuff) but i need to rework the tests too because both algorithm do not
exactly order the same way, even if both of them do the job. I'm currently on
it (just to let you know).
> ContainerExecutor does not order environment map
> ------------------------------------------------
>
> Key: YARN-5714
> URL: https://issues.apache.org/jira/browse/YARN-5714
> Project: Hadoop YARN
> Issue Type: Bug
> Components: nodemanager
> Affects Versions: 2.4.1, 2.5.2, 2.7.3, 2.6.4, 3.0.0-alpha1
> Environment: all (linux and windows alike)
> Reporter: Remi Catherinot
> Assignee: Remi Catherinot
> Priority: Trivial
> Labels: oct16-medium
> Attachments: YARN-5714.001.patch, YARN-5714.002.patch,
> YARN-5714.003.patch, YARN-5714.004.patch
>
> Original Estimate: 120h
> Remaining Estimate: 120h
>
> when dumping the launch container script, environment variables are dumped
> based on the order internally used by the map implementation (hash based). It
> does not take into consideration that some env varibales may refer each
> other, and so that some env variables must be declared before those
> referencing them.
> In my case, i ended up having LD_LIBRARY_PATH which was depending on
> HADOOP_COMMON_HOME being dumped before HADOOP_COMMON_HOME. Thus it had a
> wrong value and so native libraries weren't loaded. jobs were running but not
> at their best efficiency. This is just a use case falling into that bug, but
> i'm sure others may happen as well.
> I already have a patch running in my production environment, i just estimate
> to 5 days for packaging the patch in the right fashion for JIRA + try my best
> to add tests.
> Note : the patch is not OS aware with a default empty implementation. I will
> only implement the unix version on a 1st release. I'm not used to windows env
> variables syntax so it will take me more time/research for it.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
