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

ByteRangeInputStream used in webhdfs does not override available()

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.7.1
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: webhdfs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      ByteRangeInputStream has to override the method available() , the super class implementation returns 0. Clients using the method available() to rely on available bytes information will end up with errors during reads if WebhdfsFileSystem is used.

      1. HDFS-8885.000.patch
        2 kB
        Shradha Revankar
      2. HDFS-8885.001.patch
        0.9 kB
        Shradha Revankar
      3. HDFS-8885.002.patch
        4 kB
        Shradha Revankar

        Activity

        Hide
        srevanka Shradha Revankar added a comment -

        Attaching the patch for review.

        Show
        srevanka Shradha Revankar added a comment - Attaching the patch for review.
        Hide
        srevanka Shradha Revankar added a comment -

        Attaching the patch again with UTF-8 encoding

        Show
        srevanka Shradha Revankar added a comment - Attaching the patch again with UTF-8 encoding
        Hide
        cnauroth Chris Nauroth added a comment -

        Hello Shradha Revankar. Thank you for posting the patch. Here are a few notes.

        1. A ByteRangeInputStream starts out in SEEK status, which indicates the underlying HTTP stream needs to be opened. If a caller calls available before any read calls, then the result won't be correct, because it has not yet determined the file length. I recommend calling getInputStream first, which lazily opens the underlying HTTP stream if necessary.
        2. A similar situation occurs if a caller calls seek immediately before available. Again, calling getInputStream first would handle this case correctly.
        3. It's typical for available to throw an IOException if called after the stream has been closed. Once again, a call to getInputStream would cover this case, because that method is already implemented to throw an IOException if the stream is in CLOSED state.
        4. Can you please look into adding unit tests in TestByteRangeInputStream?
        Show
        cnauroth Chris Nauroth added a comment - Hello Shradha Revankar . Thank you for posting the patch. Here are a few notes. A ByteRangeInputStream starts out in SEEK status, which indicates the underlying HTTP stream needs to be opened. If a caller calls available before any read calls, then the result won't be correct, because it has not yet determined the file length. I recommend calling getInputStream first, which lazily opens the underlying HTTP stream if necessary. A similar situation occurs if a caller calls seek immediately before available . Again, calling getInputStream first would handle this case correctly. It's typical for available to throw an IOException if called after the stream has been closed. Once again, a call to getInputStream would cover this case, because that method is already implemented to throw an IOException if the stream is in CLOSED state. Can you please look into adding unit tests in TestByteRangeInputStream ?
        Hide
        srevanka Shradha Revankar added a comment -

        Hi Chris Nauroth,
        Thank you for reviewing this, getInputStream() is called in the constructor of ByteRangeInputStream, so 1 and 2 should not occur. Do you still think this should be called again in avaliable?
        3. Agree, will add this check.
        4.I will add testcases, sorry about not doing it in the first patch.

        Show
        srevanka Shradha Revankar added a comment - Hi Chris Nauroth , Thank you for reviewing this, getInputStream() is called in the constructor of ByteRangeInputStream , so 1 and 2 should not occur. Do you still think this should be called again in avaliable ? 3. Agree, will add this check. 4.I will add testcases, sorry about not doing it in the first patch.
        Hide
        cnauroth Chris Nauroth added a comment -

        ...getInputStream() is called in the constructor of ByteRangeInputStream...

        Yes, thank you. I missed this while reviewing. That means we don't need to worry about case 1.

        Case 2 is still a potential issue if a caller calls seek right before available. If the file length has changed between calls on the stream (such as due to a concurrent writer), the value returned by available would be inaccurate. Because of case 2 and case 3, I still recommend calling getInputStream.

        Show
        cnauroth Chris Nauroth added a comment - ... getInputStream() is called in the constructor of ByteRangeInputStream ... Yes, thank you. I missed this while reviewing. That means we don't need to worry about case 1. Case 2 is still a potential issue if a caller calls seek right before available . If the file length has changed between calls on the stream (such as due to a concurrent writer), the value returned by available would be inaccurate. Because of case 2 and case 3, I still recommend calling getInputStream .
        Hide
        srevanka Shradha Revankar added a comment -

        Incorporated Chri's review comments and added testcases in the new patch.

        Show
        srevanka Shradha Revankar added a comment - Incorporated Chri's review comments and added testcases in the new patch.
        Hide
        cnauroth Chris Nauroth added a comment -

        +1 for patch v002 pending a fresh Jenkins run. I have submitted it for a new run.

        Show
        cnauroth Chris Nauroth added a comment - +1 for patch v002 pending a fresh Jenkins run. I have submitted it for a new run.
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 15m 53s 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 7m 52s There were no new javac warning messages.
        +1 javadoc 10m 15s There were no new javadoc warning messages.
        +1 release audit 0m 25s The applied patch does not increase the total number of release audit warnings.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 30s mvn install still works.
        +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse.
        +1 native 3m 13s Pre-build of native portion
        +1 hdfs tests 163m 34s Tests passed in hadoop-hdfs.
        +1 hdfs tests 0m 30s Tests passed in hadoop-hdfs-client.
            203m 50s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12752710/HDFS-8885.002.patch
        Optional Tests javadoc javac unit
        git revision trunk / 09c64ba
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12264/artifact/patchprocess/testrun_hadoop-hdfs.txt
        hadoop-hdfs-client test log https://builds.apache.org/job/PreCommit-HDFS-Build/12264/artifact/patchprocess/testrun_hadoop-hdfs-client.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12264/testReport/
        Java 1.7.0_55
        uname Linux asf904.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/12264/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 15m 53s 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 7m 52s There were no new javac warning messages. +1 javadoc 10m 15s There were no new javadoc warning messages. +1 release audit 0m 25s The applied patch does not increase the total number of release audit warnings. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 30s mvn install still works. +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse. +1 native 3m 13s Pre-build of native portion +1 hdfs tests 163m 34s Tests passed in hadoop-hdfs. +1 hdfs tests 0m 30s Tests passed in hadoop-hdfs-client.     203m 50s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12752710/HDFS-8885.002.patch Optional Tests javadoc javac unit git revision trunk / 09c64ba hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12264/artifact/patchprocess/testrun_hadoop-hdfs.txt hadoop-hdfs-client test log https://builds.apache.org/job/PreCommit-HDFS-Build/12264/artifact/patchprocess/testrun_hadoop-hdfs-client.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12264/testReport/ Java 1.7.0_55 uname Linux asf904.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/12264/console This message was automatically generated.
        Hide
        ajisakaa Akira Ajisaka added a comment -

        +1

        Show
        ajisakaa Akira Ajisaka added a comment - +1
        Hide
        ajisakaa Akira Ajisaka added a comment -

        Committed this to trunk and branch-2. Thanks Shradha Revankar for creating the patch and thanks Chris Nauroth for the reviews.

        Show
        ajisakaa Akira Ajisaka added a comment - Committed this to trunk and branch-2. Thanks Shradha Revankar for creating the patch and thanks Chris Nauroth for the reviews.
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #8393 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8393/)
        HDFS-8885. ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6)

        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8393 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8393/ ) HDFS-8885 . ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #339 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/339/)
        HDFS-8885. ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6)

        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #339 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/339/ ) HDFS-8885 . ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #346 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/346/)
        HDFS-8885. ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #346 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/346/ ) HDFS-8885 . ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Yarn-trunk #1075 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1075/)
        HDFS-8885. ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6)

        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #1075 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1075/ ) HDFS-8885 . ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2288 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2288/)
        HDFS-8885. ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2288 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2288/ ) HDFS-8885 . ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #329 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/329/)
        HDFS-8885. ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6)

        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #329 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/329/ ) HDFS-8885 . ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2267 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2267/)
        HDFS-8885. ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6)

        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2267 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2267/ ) HDFS-8885 . ByteRangeInputStream used in webhdfs does not override available(). Contributed by Shradha Revankar. (aajisaka: rev c92e31bd659e95c8baa0f3b2bf0cd7f6f72278e6) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/ByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestByteRangeInputStream.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        cnauroth Chris Nauroth added a comment -

        Shradha Revankar, thank you for the patch. Akira Ajisaka, thank you for the code review and commit.

        Show
        cnauroth Chris Nauroth added a comment - Shradha Revankar , thank you for the patch. Akira Ajisaka , thank you for the code review and commit.

          People

          • Assignee:
            srevanka Shradha Revankar
            Reporter:
            srevanka Shradha Revankar
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development