[
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:45 PM:
-------------------------------------------------------
Thanks Colin for the work.
*1.*
This comment is for {{ExternalBlockReader}}:
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 replica length? Since from the calculation of {{skip}} and {{available}}
in {{ExternalBlockReader}} , it indicates it assumes it's the block replica
length.
For example, if client wants to read 5 bytes starting from pos 100, then in
{{ExternalBlockReader}}, the block replica is only 5 bytes, it's not correct.
*2.*
{code}
/**
* Set the length of the replica which is visible to this client. If bytes
* are added later, they will not be visible to the ReplicaAccessor we are
* building. In order to see more of the replica, the client must re-open
* this HDFS file. The visible length provides an upper bound, but not a
* lower one. If the replica is deleted or truncated, fewer bytes may be
* visible than specified here.
*/
public abstract ReplicaAccessorBuilder setVisibleLength(long visibleLength);
{code}
Here it says the visible length is the upper bound, if so, how can we use it to
calculate the {{available()}}.
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.
> 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)
