[
https://issues.apache.org/jira/browse/HDFS-8901?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15458196#comment-15458196
]
SammiChen commented on HDFS-8901:
---------------------------------
1. {{TestPread}} failure is fixed.
2.
{quote}
Looking back at the discussion history, my previous [suggestion |
https://issues.apache.org/jira/browse/HDFS-8901?focusedCommentId=15435642&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15435642]
to move it to {{BlockReaderUtil}} was misleading. Sorry about that.
So my concern here is not about correctness but about code structure.
{{BlockReaderUtil}} was designed to contain static util / helper methods for
different block readers. That's why its scope was within its package. We
shouldn't make it public unnecessarily. If we need a {{ByteBuffer}} version of
{{readAll}}, I think we should add the API to the {{BlockReader}} interface.
{quote}
Sounds very reasonable. A new API {{BlockReaderl#readAll(ByteBuffer buf, int
len)}} is introduced to replace the previous one in {{BlockReaderUtil}}
3.
{quote}
# In general, I feel that dealing with {{ByteBuffer}} causes a lot of
complexities in understanding the expected {{position}} and {{limit}} before
and after method calls. If possible, it'd be very helpful to specify those
expectations in Javadocs. For example, {{copyTo}} can specify that it will take
care of flipping {{target}} to a ready-to-read state.
{quote}
Note is added to {{copyTo}} as a heads-up that {{target}} will be of
read-to-read state after the call. And I agree with the complexities opinion.
4. also fixed some old checkstyle issues.
Thanks Zhe! I think we are very close now.
> Use ByteBuffer in striping positional read
> ------------------------------------------
>
> Key: HDFS-8901
> URL: https://issues.apache.org/jira/browse/HDFS-8901
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Reporter: Kai Zheng
> Assignee: SammiChen
> Attachments: HDFS-8901-v10.patch, HDFS-8901-v17.patch,
> HDFS-8901-v18.patch, HDFS-8901-v2.patch, HDFS-8901-v3.patch,
> HDFS-8901-v4.patch, HDFS-8901-v5.patch, HDFS-8901-v6.patch,
> HDFS-8901-v7.patch, HDFS-8901-v8.patch, HDFS-8901-v9.patch,
> HDFS-8901.v11.patch, HDFS-8901.v12.patch, HDFS-8901.v13.patch,
> HDFS-8901.v14.patch, HDFS-8901.v15.patch, HDFS-8901.v16.patch,
> initial-poc.patch
>
>
> Native erasure coder prefers to direct ByteBuffer for performance
> consideration. To prepare for it, this change uses ByteBuffer through the
> codes in implementing striping position read. It will also fix avoiding
> unnecessary data copying between striping read chunk buffers and decode input
> buffers.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
