You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Esfandiar Manii (JIRA)" <ji...@apache.org> on 2017/08/03 17:35:00 UTC
[jira] [Comment Edited] (HADOOP-14722) Azure: BlockBlobInputStream
position incorrect after seek
[ https://issues.apache.org/jira/browse/HADOOP-14722?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16113101#comment-16113101 ]
Esfandiar Manii edited comment on HADOOP-14722 at 8/3/17 5:34 PM:
------------------------------------------------------------------
BlockBlobInputStream.java: L92-94: streamPosition - streamBufferLength + streamBufferPosition, can this become negative?
BlockBlobInputStream.java: L133: don't we need to nullify streamBuffer too?
BlockBlobInputStream.java: L321-323: Why dont you throw the exception right at the beginning?
BlockBlobInputStream.java: L314: Overall I am not a big fan of having nested if and elses because its making code more complicated that needed. lets just return instead of creating else.
For example
public synchronized long skip(long n) throws IOException {
checkState();
long skipped;
if (blobInputStream != null) {
skipped = blobInputStream.skip(n);
streamPosition += skipped;
return skipped;
}
if (n < 0 || n > streamLength - streamPosition) {
throw new IndexOutOfBoundsException("skip range");
}
if (streamBuffer == null) {
streamPosition += n;
return n;
}
if (n < streamBufferLength - streamBufferPosition) {
streamBufferPosition += (int) n;
} else {
streamBufferPosition = 0;
streamBufferLength = 0;
streamPosition = getPos() + n;
}
return skipped;
}
BlockBlobInputStream.java: L330: I'd suggest create a private method which clears the buffer and get rid of all the custom streamBufferPosition = 0; streamBufferLength = 0 and etc.
was (Author: esmanii):
BlockBlobInputStream.java: L92-94: streamPosition - streamBufferLength + streamBufferPosition, can this become negative?
BlockBlobInputStream.java: L133: don't we need to nullify streamBuffer too?
BlockBlobInputStream.java: L321-323: Why dont you throw the exception right at the beginning?
BlockBlobInputStream.java: L314: Overall I am not a big fan of having nested if and elses because its making code more complicated that needed. lets just return instead of creating else.
For example
public synchronized long skip(long n) throws IOException {
checkState();
long skipped;
if (blobInputStream != null) {
skipped = blobInputStream.skip(n);
streamPosition += skipped;
return skipped;
}
if (n < 0 || n > streamLength - streamPosition) {
throw new IndexOutOfBoundsException("skip range");
}
if (streamBuffer == null) {
streamPosition += n;
return n;
}
if (n < streamBufferLength - streamBufferPosition) {
streamBufferPosition += (int) n;
} else {
streamBufferPosition = 0;
streamBufferLength = 0;
streamPosition = getPos() + n;
}
return skipped;
}
BlockBlobInputStream.java: L330: I'd suggest clear a private method which clears the buffer and get rid of all the custom streamBufferPosition = 0; streamBufferLength = 0 and etc.
> Azure: BlockBlobInputStream position incorrect after seek
> ---------------------------------------------------------
>
> Key: HADOOP-14722
> URL: https://issues.apache.org/jira/browse/HADOOP-14722
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
> Reporter: Thomas Marquardt
> Assignee: Thomas Marquardt
> Attachments: HADOOP-14722-001.patch, HADOOP-14722-002.patch
>
>
> The seek, skip, and getPos methods of BlockBlobInputStream do not correctly account for the stream's internal buffer. This results in invalid stream positions.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org