[
https://issues.apache.org/jira/browse/HDFS-8901?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15456710#comment-15456710
]
Zhe Zhang commented on HDFS-8901:
---------------------------------
Thanks for the update [~Sammi].
# {{TestPread}} failure looks related. It was not failing for v16 patch.
# bq. I thought the {{BlockReaderUtil.readAll}} implementation just
demonstrates the idea, that is leverage BlockReader#read(ByteBuffer buf) to
read data into buf.
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.
# 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.
Otherwise LGTM. +1 once the above are addressed / clarified. Great work Sammi
and Kai!
> 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-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: [email protected]
For additional commands, e-mail: [email protected]
