[ https://issues.apache.org/jira/browse/HDFS-8319?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14559508#comment-14559508 ]
Jing Zhao commented on HDFS-8319: --------------------------------- Thanks for the review, [~walter.k.su]. Replied your comments inline below: bq. evaluated value is int, casting is useless. Should be "long cellStart = 1L * cell.idxInInternalBlk * cellSize + cell.offset;" The casting is just moved to the calculation. So this change is unnecessary I guess. bq. We knew it's completed. So get() is useless. get() is necessary to get the ExecutionException during the runtime. bq. So need to remove i<=dataBlkNum in line 614. I think this is a temporary workaround from HDFS-7678 according to Zhe's [comment|https://issues.apache.org/jira/browse/HDFS-7678?focusedCommentId=14535803&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14535803]. I will add a TODO for this. bq. you only call prepareDecodeInputs once, and only copy curStripeBuf to decodeInputs once I think this was originally a bug in pread before the patch. The current patch tries to fix this issue by moving the data copy code to the {{decode}} function. bq. Pread create block reader at actualGetFromOneDataNode. It's a problem of PositionStripeReader version's readChunk(). It creates too much readers. This is a great catch. With the new AlignedStripe logic from HDFS-7678 we may create too many readers. We can create a separate jira to improve this. bq. We only have to keep one of them. To address the above improvement we may finally use the same readChunk function for stateful read and pread. But this jira mainly changes the stateful read code to make it unified with pread. Thus I guess we can do this along with the pread reader improvement in a separate jira. bq. Need to copy the decoded result back. The pread code already does this. For stateful read I thought I could avoid extra data copy by passing the slices of the curStripeBuf to the decode function. But looks like some decoder may change the input so extra data copy may be necessary. I will fix this later. I will address all the other comments in my next patch. Thanks again for the review, Walter. > Erasure Coding: support decoding for stateful read > -------------------------------------------------- > > Key: HDFS-8319 > URL: https://issues.apache.org/jira/browse/HDFS-8319 > Project: Hadoop HDFS > Issue Type: Sub-task > Reporter: Jing Zhao > Assignee: Jing Zhao > Attachments: HDFS-8319.001.patch > > > HDFS-7678 adds the decoding functionality for pread. This jira plans to add > decoding to stateful read. -- This message was sent by Atlassian JIRA (v6.3.4#6332)