[jira] [Commented] (HADOOP-11935) Provide optional native implementation of stat syscall.

["Chris Nauroth (JIRA)" <ji...@apache.org>]

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

Chris Nauroth commented on HADOOP-11935:
----------------------------------------

The goal of HADOOP-12045 was to enable setting atime explicitly on the local file system using the {{FileSystem#setTimes}} API.  It did not get involved at all with the automatic access time update semantics of any particular local file system, nor did the tests include any assumptions that atime is enabled on the local file system.  I would agree that our tests cannot make assumptions like that about the local file system.

The tests in question just need to be able to set an atime value explicitly, and then verify that the atime was changed on the target instead of the link.  If we can get cross-platform agreement on this basic level of functionality, then I expect we can keep the tests.  There won't be any need for complexity like checking the mount table.  I think we just have a corner case bug in that the test's call to {{FileSystem#getFileLinkStatus}} might inadvertently update atime of the link once again.  The proposed patch solves this by relaxing the assertion to use >=.

HADOOP-12045 was implemented as a pass-through to {{BasicFileAttributeView#setTimes}}, which then appears to be a pass-through to [{{futimes}}|http://man7.org/linux/man-pages/man3/futimes.3.html] in UnixNativeDispatcher.c in the OpenJDK source.  On Windows, it's a pass-through to [{{SetFileTime}}|https://msdn.microsoft.com/en-us/library/windows/desktop/ms724933(v=vs.85).aspx] via WindowsNativeDispatcher.c.

The Linux man page for {{futimes}} says that the function is not specified by a standard and is only implemented for Linux and BSDs.  [~alanburlison], do you know if Solaris has {{futimes}}?  I couldn't find a definitive source on that.  As long as {{BasicFileAttributeView#setTimes}} somehow works correctly onn Solaris, then I expect we're fine.

The tests need to be skipped right now on Windows and Solaris due to a difference in native code implementation supported by the Linux side compared to other platforms.  I described this in more detail in my comments on HADOOP-12045.  HADOOP-11935 is an open issue that would fix this problem by using appropriate NIO APIs to get the {{stat}}/{{lstat}} behavior we need on those platforms.  I don't think we're indiscriminately skipping tests without trying to understand semantics.  We are skipping them temporarily until after resolution of HADOOP-11935.

To summarize, I am +1 for committing patch v02, which retains the tests but corrects for a corner case.  Follow-ups are tracked in separate JIRAs.  I will hold off on committing though in case there is further discussion.

> Provide optional native implementation of stat syscall.
> -------------------------------------------------------
>
>                 Key: HADOOP-11935
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11935
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: fs, native
>            Reporter: Chris Nauroth
>         Attachments: HADOOP-11935-NativeIO-prelim.patch
>
>
> Currently, {{RawLocalFileSystem.DeprecatedRawLocalFileStatus#loadPermissionInfo}} is implemented as forking an {{ls}} command and parsing the output.  This was observed to be a bottleneck in YARN-3491.  This issue proposes an optional native implementation of a {{stat}} syscall through JNI.  We would maintain the existing code as a fallback for systems where the native code is not available.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)