[
https://issues.apache.org/jira/browse/HADOOP-9413?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13615556#comment-13615556
]
Chris Nauroth commented on HADOOP-9413:
---------------------------------------
{quote}
However, as long as the abstractions are solid, I think we are good. Let me
know what you think.
{quote}
Yes, I am definitely in favor of the structure of this patch: providing common
utility wrappers for these methods. I think the only open question is whether
the implementation should revert the logic of HADOOP-8973 like this.
{quote}
Chris already has code that does the expected thing for the scenario in which
the running process is checking whether it has read/write/execute permissions
on a directory.
{quote}
A couple of caveats on this:
# The code I wrote for HADOOP-8973 works only for directories, and we need
these common APIs to handle both files and directories. See Ivan's patches on
HDFS-4610 and YARN-506 for some examples where the codebase needs to get/set
permissions on individual files.
# There was also the question of performance, because my code actually
performed file access and forked new processes to implement the access checks.
These were not problems for HADOOP-8973, because {{DiskChecker}} is used only
for directories and it's used rarely enough that the performance impact would
likely be unnoticeable.
I'm wondering if it's time for us to write a JNI call to {{AccessCheck}}:
http://msdn.microsoft.com/en-us/library/windows/desktop/aa374815(v=vs.85).aspx
This would only be used if (Windows && JDKversion < 7). Otherwise, we expect
the JDK APIs to work.
Obviously, this will take longer to implement. A potential compromise would be
to go with the patch's current implementation of the new functions, but leave
{{DiskChecker}} as is temporarily, just until we get the JNI call written.
That way, the other permission checks in the codebase get pretty close to
working (not 100% correct, but certainly better than the current state), and we
still maintain full correctness for the very important {{DiskChecker}} piece.
Ivan and Bikas, do you have any follow-up thoughts?
> Introduce common utils for File#setReadable/Writable/Executable and
> File#canRead/Write/Execute that work cross-platform
> -----------------------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-9413
> URL: https://issues.apache.org/jira/browse/HADOOP-9413
> Project: Hadoop Common
> Issue Type: Bug
> Reporter: Ivan Mitic
> Assignee: Ivan Mitic
> Fix For: 3.0.0
>
> Attachments: HADOOP-9413.commonfileutils.patch
>
>
> So far, we've seen many unittest and product bugs in Hadoop on Windows
> because Java's APIs that manipulate with permissions do not work as expected.
> We've addressed many of these problems on one-by-one basis (by either
> changing code a bit or disabling the test). While debugging the remaining
> unittest failures we continue to run into the same patterns of problems, and
> instead of addressing them one-by-one, I propose that we expose a set of
> equivalent wrapper APIs that will work well for all platforms.
> Scanning thru the codebase, this will actually be a simple change as there
> are very few places that use File#setReadable/Writable/Executable and
> File#canRead/Write/Execute (5 files in Common, 9 files in HDFS).
> HADOOP-8973 contains additional context on the problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
