[
https://issues.apache.org/jira/browse/YARN-4594?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15124248#comment-15124248
]
Colin Patrick McCabe commented on YARN-4594:
--------------------------------------------
Thanks for the review, [~jlowe].
bq. I noticed we're using openat, fchmodat, and unlinkat for the first time. I
suspect most other POSIX-like distributions support this, but I think these
were only recently added to MacOS X (in 10.9 Yosemite). I'm not sure if anyone
uses container-executor for MacOS X (or if container-executor even
compiles/works for MacOS X today). but adding these could break the native
build for those using older MacOS X versions.
Hmm. Correct me if I'm wrong, but I don't think {{container-executor}} is
supported at all on MacOS. I can see places in the code that rely on cgroups,
which is a kernel feature that MacOS just doesn't have (and may never have).
You can see a bunch of code in {{container-executor.c}} dealing with very linux
specific files in {{/proc}}, and so forth. My thinking is that if we ever do
support MacOS in {{container-executor}}, we will support a new enough version
that using the newer POSIX functions is not a problem.
bq. I think this needs to use strerror(-fd):
Fixed
bq. There's no check for an error being encountered by readdir and therefore no
logging if it does occur
Fixed
bq. Sometimes recursive_unlink_helper is returning errno and sometimes it is
returning -errno. For example, the "failed to stat" path will set ret=errno and
return -ret as -errno, but the "failed to unlink" path will set ret=-errno and
thus return errno.
Good catch. Let's return positive error codes everywhere, except in the very
specific case of {{open_helper}} where non-negative returns mean "file
descriptor".
> container-executor fails to remove directory tree when chmod required
> ---------------------------------------------------------------------
>
> Key: YARN-4594
> URL: https://issues.apache.org/jira/browse/YARN-4594
> Project: Hadoop YARN
> Issue Type: Bug
> Components: nodemanager
> Reporter: Colin Patrick McCabe
> Assignee: Colin Patrick McCabe
> Attachments: YARN-4594.001.patch
>
>
> test-container-executor.c doesn't work:
> * It assumes that realpath(/bin/ls) will be /bin/ls, whereas it is actually
> /usr/bin/ls on many systems.
> * The recursive delete logic in container-executor.c fails -- nftw does the
> wrong thing when confronted with directories with the wrong mode (permission
> bits), leading to an attempt to run rmdir on a non-empty directory.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
