[
https://issues.apache.org/jira/browse/HDFS-8905?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15008161#comment-15008161
]
Rakesh R commented on HDFS-8905:
--------------------------------
Thanks [~drankye] for the good work. Adding few thoughts, please take a look at
it.
# {{ByteBufferStrategy#read}} - The proposed patch uses readBuffer.slice() to
avoid re-positioning buf to original state, could you please add java comments
conveying the same. I'm wondering {{ByteArrayStrategy#read}} doesn't have the
logic of setting back to the original state in case of failure, should we?
# I could see {{offset += length;}} and {{offset += nRead;}} are both updating
the offset variable. The first one is a kind of copy and is a public API. Will
this affect the other read(blockReader, length) in any case. I'm just thinking
about any relation be be maintained between these two read API calls which
updates the {{offset}} variable.
{code}
ByteArrayStrategy:
+ public int read(ByteBuffer src, int length) {
+ ByteBuffer dup = src.duplicate();
+ dup.get(buf, offset, length);
+ offset += length;
{code}
{code}
ByteArrayStrategy:
+ public int read(BlockReader blockReader, int length) throws IOException {
+ int nRead = blockReader.read(buf, offset, length);
+ if (nRead > 0) {
+ updateReadStatistics(readStatistics, nRead, blockReader);
+ offset += nRead;
+ } else {
+ DFSClient.LOG.warn("Zero bytes read");
+ }
{code}
# Also, please fix the checkstyle warnings. It seems these are not introduced
by your patch. Since you are touching this part would be good to fix it:)
> Refactor DFSInputStream#ReaderStrategy
> --------------------------------------
>
> Key: HDFS-8905
> URL: https://issues.apache.org/jira/browse/HDFS-8905
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: erasure-coding
> Reporter: Kai Zheng
> Assignee: Kai Zheng
> Attachments: HDFS-8905-HDFS-7285-v1.patch, HDFS-8905-v2.patch
>
>
> DFSInputStream#ReaderStrategy family don't look very good. This refactors a
> little bit to make them make more sense.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
