[
https://issues.apache.org/jira/browse/HDFS-9133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14906000#comment-14906000
]
Yi Liu edited comment on HDFS-9133 at 9/24/15 1:19 PM:
-------------------------------------------------------
Thanks Colin for the work.
*1.*
This comment is for {{ExternalBlockReader}}, not only for the patch:
There is some mismatched between {{ExternalBlockReader}} and other BlockReaders
{code}
ExternalBlockReader(ReplicaAccessor accessor, long visibleLength,
long startOffset) {
this.accessor = accessor;
this.visibleLength = visibleLength;
this.pos = startOffset;
}
{code}
{code}
return new ExternalBlockReader(accessor, length, startOffset);
{code}
{code}
/**
* Number of bytes to read. -1 indicates no limit.
*/
private long length = -1;
/**
* The offset within the block to start reading at.
*/
private long startOffset;
{code}
For other block readers, the {{length}} means the number of bytes to read, but
in {{ExternalBlockReader}}, it means "visible length" which assumes it's the
block length? Since from the calculation of {{skip}} and {{available}} in
{{ExternalBlockReader}} , it indicates it assumes it's the block length, or at
least assume {{startOffset}} is 0.
was (Author: hitliuyi):
Thanks Colin for the work.
*1.*
This comment is for {{ExternalBlockReader}}, not only for the patch:
There is some mismatched between {{ExternalBlockReader}} and other BlockReaders
{code}
ExternalBlockReader(ReplicaAccessor accessor, long visibleLength,
long startOffset) {
this.accessor = accessor;
this.visibleLength = visibleLength;
this.pos = startOffset;
}
{code}
{code}
return new ExternalBlockReader(accessor, length, startOffset);
{code}
{code}
/**
* Number of bytes to read. -1 indicates no limit.
*/
private long length = -1;
/**
* The offset within the block to start reading at.
*/
private long startOffset;
{code}
For other block readers, the {{length}} means the number of bytes to read, but
in {{ExternalBlockReader}}, it means "visible length" which assumes it's the
block length? Since from the calculation of {{skip}} and {{available}} in
{{ExternalBlockReader}} , it indicates it assumes it's the block length, or at
least assume {{startOffset}} is 0.
*2.*
I think if reach the end of block, we should also update the {{pos}}, otherwise
the calculation in {{available()}} is wrong.
> ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF
> -----------------------------------------------------------------------------
>
> Key: HDFS-9133
> URL: https://issues.apache.org/jira/browse/HDFS-9133
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: hdfs-client
> Affects Versions: 2.8.0
> Reporter: Colin Patrick McCabe
> Assignee: Colin Patrick McCabe
> Attachments: HDFS-9133.001.patch
>
>
> ExternalBlockReader and ReplicaAccessor need to return -1 on read when at
> EOF, as per the JavaDoc in BlockReader.java.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
