joseluisll commented on PR #8798:
URL: https://github.com/apache/hadoop/pull/8798#issuecomment-6033172407
Thanks, walking the tree with `walkFileTree` and ignoring
`NoSuchFileException` is a good fix for the race. Two things:
**1. The ownership check also applies to the top-level dir, so validation
can silently become a no-op.**
`preVisitDirectory(root)` returns `SKIP_SUBTREE` when
`Files.getOwner(root).getName()` isn't equal to
`System.getProperty("user.name")`. On Windows the owner name is `DOMAIN\user`,
so the two never match. I reproduced this with the classes from this PR, on a
local dir containing `filecache/10/job.jar` with read denied by ACL:
```
user.name = nmuser
Files.getOwner(root) = EXAMPLEDOMAIN\nmuser
isReadable(job.jar) = false
validateSubsAccessibility(dir) = null (disk considered healthy)
SubAccessibilityVisitor("EXAMPLEDOMAIN\nmuser") walk: Can not read
C:\hadoop\nm-local\filecache\10\job.jar
```
With the owner string matching, the unreadable file is reported. With the
real `user.name` it's silently skipped. On Linux the same skip should happen
for local dirs pre-created as e.g. `root:hadoop 0775`. Could you always check
the root itself and apply the ownership skip only to entries below it?
**2. Is skipping `usercache` under LinuxContainerExecutor intended?**
With LCE, `usercache/<user>` is owned by the application user, so the
ownership skip excludes the whole `usercache` subtree. This includes `appcache`
and the private `filecache`, and the default depth of 3 is justified by those
same dirs. That leaves little beyond the public `filecache` and `nmPrivate`
being validated, which seems to cut against what YARN-11703 set out to cover.
If it is intended, please say so in the `yarn-default.xml` description.
Nits: `max-depth` isn't validated, so a negative value throws an uncaught
`IllegalArgumentException: 'maxDepth' is negative` at NodeManager startup
without naming the property; `Collectors` and `DiskChecker` imports are now
unused (checkstyle); and please revert the unrelated import reordering.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
