hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Esfandiar Manii (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HADOOP-14722) Azure: BlockBlobInputStream position incorrect after seek
Date Thu, 03 Aug 2017 17:35:00 GMT

    [ 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


Mime
View raw message