[
https://issues.apache.org/jira/browse/HDFS-7291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14186292#comment-14186292
]
Chris Nauroth commented on HDFS-7291:
-------------------------------------
Thanks for the updated patch, Xiaoyu. The comments in {{NativeIO}} and
{{Storage}} do a great job of explaining the whole situation. This mostly
looks good. I have just a few more comments:
* NativeIO.java: Instead of {{org.apache.commons.io.IOUtils#closeQuietly}}, I
recommend using {{org.apache.hadoop.io.IOUtils#cleanup}}, and passing the
{{LOG}} to it. The difference is that we'll get a log message on the rare
chance that close fails. Sometimes that extra log message can be helpful.
* Also in NativeIO.java, the return value of {{FileChannel#transferTo}} is
ignored. Is that intentional? Is it not necessary to check the actual number
of bytes written and loop because we expect the destination {{FileChannel}}
never uses non-blocking I/O? If so, then it would be worthwhile to add a
comment explaining that the return value is ignored intentionally.
* NativeIO.c: The function compiles on Linux right now, because it has void
return type. However, I think it's preferable to have a code path for Linux
that calls {{THROW}} with a "not supported" message. We expect this code will
never get called on Linux. Right now, if someone introduced a bug that caused
Linux to call it, then it wouldn't cause an immediate error, but it wouldn't
actually do the copy, which could cause confusion.
> Persist in-memory replicas with appropriate unbuffered copy API on POSIX and
> Windows
> ------------------------------------------------------------------------------------
>
> Key: HDFS-7291
> URL: https://issues.apache.org/jira/browse/HDFS-7291
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode
> Affects Versions: 2.6.0
> Reporter: Xiaoyu Yao
> Assignee: Xiaoyu Yao
> Attachments: HDFS-7291.0.patch, HDFS-7291.1.patch, HDFS-7291.2.patch
>
>
> HDFS-7090 changes to persist in-memory replicas using unbuffered IO on Linux
> and Windows. On Linux distribution, it relies on the sendfile() API between
> two file descriptors to achieve unbuffered IO copy. According to Linux
> document at http://man7.org/linux/man-pages/man2/sendfile.2.html, this is
> only supported on Linux kernel 2.6.33+.
> As pointed by Haowei in the discussion below, FileChannel#transferTo already
> has support for native unbuffered IO on POSIX platform. On Windows, JDK 6/7/8
> has not implemented native unbuffered IO yet. We change to use
> FileChannel#transfer for POSIX and our own native wrapper of CopyFileEx on
> Windows for unbuffered copy.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
