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

Zhe Zhang commented on HDFS-8905:
---------------------------------

Thanks Kai for the work. 

IIUC the patch contains 2 refactors: 
# Changing {{doRead}} to {{read}}, with different signatures. I like the new 
signatures of the method. Carrying {{offset}} and {{targetLength}} in the 
{{ByteArrayStrategy}} class also seems a good change. But I prefer the old name 
{{doRead}} to distinguish from user-visible {{read}} in the class. Also, the 
below change doesn't seem necessary. I think all callers of {{read}} has logic 
to interpret zero return.
{code}
+      } else {
+        DFSClient.LOG.warn("Zero bytes read");
+      }
{code}
# Changing {{copyFrom}} to {{read}}. Similar to above, I think {{read}} has a 
specific meaning in the {{DFSInputStream}} class and should probably not be 
used in too many ways. We should also keep (or revise) the Javadoc for 
{{copyFrom}}.

Another concern is that {{DFSInputStream}} in trunk is getting quite different 
from branch-2 now. This will make backporting harder. We should probably merge 
non-EC changes to branch-2. [~jingzhao] Let me know if you agree.

I will review the {{DFSStripedInputStream}} changes pending the above decision.

> 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)

Reply via email to