Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-9133

ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: hdfs-client
    • Labels:
      None
    • Target Version/s:

      Description

      ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF, as per the JavaDoc in BlockReader.java.

      1. HDFS-9133.001.patch
        5 kB
        Colin P. McCabe

        Activity

        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #419 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/419/)
        HDFS-9133. ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f)

        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #419 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/419/ ) HDFS-9133 . ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2386 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2386/)
        HDFS-9133. ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f)

        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2386 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2386/ ) HDFS-9133 . ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2359 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2359/)
        HDFS-9133. ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f)

        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2359 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2359/ ) HDFS-9133 . ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Yarn-trunk #1181 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1181/)
        HDFS-9133. ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f)

        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #1181 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1181/ ) HDFS-9133 . ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #448 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/448/)
        HDFS-9133. ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f)

        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #448 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/448/ ) HDFS-9133 . ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #442 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/442/)
        HDFS-9133. ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #442 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/442/ ) HDFS-9133 . ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #8521 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8521/)
        HDFS-9133. ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8521 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8521/ ) HDFS-9133 . ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) (lei: rev 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
        Hide
        eddyxu Lei (Eddy) Xu added a comment -

        LGTM. +1.

        Thanks for the effort Colin P. McCabe and Yi Liu.

        Show
        eddyxu Lei (Eddy) Xu added a comment - LGTM. +1. Thanks for the effort Colin P. McCabe and Yi Liu .
        Hide
        cmccabe Colin P. McCabe added a comment -

        I filed HDFS-9147 to fix the naming and documentation of the ExternalBlockReader length field.

        Show
        cmccabe Colin P. McCabe added a comment - I filed HDFS-9147 to fix the naming and documentation of the ExternalBlockReader length field.
        Hide
        cmccabe Colin P. McCabe added a comment -

        Thanks Colin for the work. The patch itself looks good, +1.

        Thanks

        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.

        You are correct... we should not be treating this as the visibleLength of the block, since it's really the number of bytes in between the end of the block and the current offset within the block. Let me file another jira to fix this.

        Show
        cmccabe Colin P. McCabe added a comment - Thanks Colin for the work. The patch itself looks good, +1. Thanks 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. You are correct... we should not be treating this as the visibleLength of the block, since it's really the number of bytes in between the end of the block and the current offset within the block. Let me file another jira to fix this.
        Hide
        hitliuyi Yi Liu added a comment - - edited

        Thanks Colin for the work. The patch itself looks good, +1.

        I have two small comments not related to the patch, but for ExternalBlockReader:

        1.
        There is some mismatched between ExternalBlockReader and other BlockReaders

        ExternalBlockReader(ReplicaAccessor accessor, long visibleLength,
                              long startOffset) {
            this.accessor = accessor;
            this.visibleLength = visibleLength;
            this.pos = startOffset;
          }
        
        return new ExternalBlockReader(accessor, length, startOffset);
        
        /**
           * 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;
        

        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.

        /**
           * 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);
        

        Here it says the visible length is the upper bound, if so, how can we use it to calculate the available().

        Show
        hitliuyi Yi Liu added a comment - - edited Thanks Colin for the work. The patch itself looks good, +1. I have two small comments not related to the patch, but for ExternalBlockReader : 1. There is some mismatched between ExternalBlockReader and other BlockReaders ExternalBlockReader(ReplicaAccessor accessor, long visibleLength, long startOffset) { this .accessor = accessor; this .visibleLength = visibleLength; this .pos = startOffset; } return new ExternalBlockReader(accessor, length, startOffset); /** * 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; 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. /** * 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); Here it says the visible length is the upper bound, if so, how can we use it to calculate the available() .
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 20m 4s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        +1 javac 8m 1s There were no new javac warning messages.
        +1 javadoc 10m 25s There were no new javadoc warning messages.
        +1 release audit 0m 26s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 2m 58s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 31s mvn install still works.
        +1 eclipse:eclipse 0m 35s The patch built with eclipse:eclipse.
        +1 findbugs 4m 39s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 3m 16s Pre-build of native portion
        -1 hdfs tests 194m 38s Tests failed in hadoop-hdfs.
        +1 hdfs tests 0m 31s Tests passed in hadoop-hdfs-client.
            247m 8s  



        Reason Tests
        Failed unit tests hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks
          hadoop.hdfs.TestRollingUpgrade



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12762029/HDFS-9133.001.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 1f707ec
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12647/artifact/patchprocess/testrun_hadoop-hdfs.txt
        hadoop-hdfs-client test log https://builds.apache.org/job/PreCommit-HDFS-Build/12647/artifact/patchprocess/testrun_hadoop-hdfs-client.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12647/testReport/
        Java 1.7.0_55
        uname Linux asf908.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12647/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 20m 4s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 8m 1s There were no new javac warning messages. +1 javadoc 10m 25s There were no new javadoc warning messages. +1 release audit 0m 26s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 2m 58s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 31s mvn install still works. +1 eclipse:eclipse 0m 35s The patch built with eclipse:eclipse. +1 findbugs 4m 39s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 16s Pre-build of native portion -1 hdfs tests 194m 38s Tests failed in hadoop-hdfs. +1 hdfs tests 0m 31s Tests passed in hadoop-hdfs-client.     247m 8s   Reason Tests Failed unit tests hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks   hadoop.hdfs.TestRollingUpgrade Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12762029/HDFS-9133.001.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 1f707ec hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12647/artifact/patchprocess/testrun_hadoop-hdfs.txt hadoop-hdfs-client test log https://builds.apache.org/job/PreCommit-HDFS-Build/12647/artifact/patchprocess/testrun_hadoop-hdfs-client.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12647/testReport/ Java 1.7.0_55 uname Linux asf908.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12647/console This message was automatically generated.

          People

          • Assignee:
            cmccabe Colin P. McCabe
            Reporter:
            cmccabe Colin P. McCabe
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development