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

stack commented on HDFS-14585:
------------------------------

nit: why the change in import order for nonnull and Preconditions? Is it about 
respecting checkstyle import ordering rules?
nit: Best to not make formatting changes that are unrelated to your patch: e.g. 
wrapping exception declaration on blockSeekTo (there are a few of this type of 
change). Formatting bulks up your patch and distract the reviewer.
nit:         ByteBuffer bb = ByteBuffer.wrap(buffer, offset, length); is offset.

Some nice cleanup and duplication removal; e.g. using BB to keep account on the 
read buffer (offsets and length), purge of the HDFS-8703 unused EC version of 
actualGetFromOneDataNode, pulling out long targetEnd = targetStart + 
bytesToRead - 1, etc. 

Is this going to be ok: 1249            tmp.limit(tmp.position() + len); ? The 
EC version of actualGetFromOneDataNode had a checkReadPortions. Should there be 
a check we don't go over the end of the buffer here?

Why do we drop the below in the patch?

1268              updateReadStatistics(readStatistics, nread, reader);          
1269              dfsClient.updateFileSystemReadStats(          
1270                  reader.getNetworkDistance(), nread);

Thanks.






> Backport HDFS-8901 Use ByteBuffer in DFSInputStream#read to branch-2 and 
> branch2.9
> ----------------------------------------------------------------------------------
>
>                 Key: HDFS-14585
>                 URL: https://issues.apache.org/jira/browse/HDFS-14585
>             Project: Hadoop HDFS
>          Issue Type: Task
>            Reporter: Lisheng Sun
>            Assignee: Lisheng Sun
>            Priority: Major
>         Attachments: HDFS-14585.branch-2.9.v1.patch, 
> HDFS-14585.branch-2.v1.patch
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to