Hadoop Common
  1. Hadoop Common
  2. HADOOP-8833

fs -text should make sure to call inputstream.seek(0) before using input stream

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2-alpha
    • Fix Version/s: 2.0.3-alpha
    • Component/s: fs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      From Muddy Dixon on HADOOP-8449:

      Hi
      We found the changes in order of switch and guard block in

      private InputStream forMagic(Path p, FileSystem srcFs) throws IOException
      

      Because of this change, return value of

      codec.createInputStream(i)
      

      is changed if codec exists.

      private InputStream forMagic(Path p, FileSystem srcFs) throws IOException {
          FSDataInputStream i = srcFs.open(p);
      
          // check codecs
          CompressionCodecFactory cf = new CompressionCodecFactory(getConf());
          CompressionCodec codec = cf.getCodec(p);
          if (codec != null) {
            return codec.createInputStream(i);
          }
      
          switch(i.readShort()) {
             // cases
          }
      

      New:

      private InputStream forMagic(Path p, FileSystem srcFs) throws IOException {
          FSDataInputStream i = srcFs.open(p);
      
          switch(i.readShort()) { // <=== index (or pointer) processes!!
            // cases
            default: {
              // Check the type of compression instead, depending on Codec class's
              // own detection methods, based on the provided path.
              CompressionCodecFactory cf = new CompressionCodecFactory(getConf());
              CompressionCodec codec = cf.getCodec(p);
              if (codec != null) {
                return codec.createInputStream(i);
              }
              break;
            }
          }
      
          // File is non-compressed, or not a file container we know.
          i.seek(0);
          return i;
        }
      

      Fix is to use i.seek(0) before we use i anywhere. I missed that.

      1. HADOOP-8833.patch
        4 kB
        Karthik Kambatla
      2. HADOOP-8833.patch
        4 kB
        Tom White
      3. HADOOP-8833.patch
        4 kB
        Harsh J

        Issue Links

          Activity

          Harsh J created issue -
          Harsh J made changes -
          Field Original Value New Value
          Link This issue is broken by HADOOP-8449 [ HADOOP-8449 ]
          Hide
          Harsh J added a comment -

          This should fix it.

          Show
          Harsh J added a comment - This should fix it.
          Harsh J made changes -
          Attachment HADOOP-8833.patch [ 12546011 ]
          Harsh J made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Target Version/s 2.0.2-alpha [ 12322473 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12546011/HADOOP-8833.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1490//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1490//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12546011/HADOOP-8833.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1490//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1490//console This message is automatically generated.
          Hide
          Tom White added a comment -

          +1 on the fix. I noticed that the test doesn't fail without the fix though. This is because BZip2Codec.BZip2CompressionInputStream.readStreamHeader() tolerates a missing (two-byte) header, so BZip2 files happen to work anyway. I've modified the test slightly to test a deflate-compressed file, and this one does fail without the seek fix.

          Show
          Tom White added a comment - +1 on the fix. I noticed that the test doesn't fail without the fix though. This is because BZip2Codec.BZip2CompressionInputStream.readStreamHeader() tolerates a missing (two-byte) header, so BZip2 files happen to work anyway. I've modified the test slightly to test a deflate-compressed file, and this one does fail without the seek fix.
          Tom White made changes -
          Attachment HADOOP-8833.patch [ 12546031 ]
          Hide
          Harsh J added a comment -

          Thanks Tom, I did wonder about that. Then though it to be my maven local repo cause I had first run test with fix installed. thanks for revising the patch. Committing to trunk and branch-2 now, but leaving open for 2.0.2 (gatekeeper has to grant).

          Show
          Harsh J added a comment - Thanks Tom, I did wonder about that. Then though it to be my maven local repo cause I had first run test with fix installed. thanks for revising the patch. Committing to trunk and branch-2 now, but leaving open for 2.0.2 (gatekeeper has to grant).
          Hide
          Harsh J added a comment -

          Oh, first gotta wait for jenkins again.

          Show
          Harsh J added a comment - Oh, first gotta wait for jenkins again.
          Hide
          Karthik Kambatla added a comment -

          Cancelling patch to re-submit and kick Jenkins

          Show
          Karthik Kambatla added a comment - Cancelling patch to re-submit and kick Jenkins
          Karthik Kambatla made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Karthik Kambatla made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Karthik Kambatla made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Karthik Kambatla added a comment -

          Uploading the same patch again.

          Show
          Karthik Kambatla added a comment - Uploading the same patch again.
          Karthik Kambatla made changes -
          Attachment HADOOP-8833.patch [ 12546074 ]
          Karthik Kambatla made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Harsh J added a comment -

          I ran the new test locally - works on a Mac too so apparently no native dependency as I'd originally thought (due to deflate):

          -------------------------------------------------------
           T E S T S
          -------------------------------------------------------
          Running org.apache.hadoop.hdfs.TestDFSShell
          Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 36.699 sec
          

          Committing shortly. Thanks again Tom and Karthik.

          Show
          Harsh J added a comment - I ran the new test locally - works on a Mac too so apparently no native dependency as I'd originally thought (due to deflate): ------------------------------------------------------- T E S T S ------------------------------------------------------- Running org.apache.hadoop.hdfs.TestDFSShell Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 36.699 sec Committing shortly. Thanks again Tom and Karthik.
          Harsh J made changes -
          Target Version/s 2.0.2-alpha [ 12322473 ] 3.0.0 [ 12320357 ]
          Harsh J made changes -
          Target Version/s 3.0.0 [ 12320357 ] 3.0.0, 2.0.3-alpha [ 12320357, 12323273 ]
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #2755 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2755/)
          HADOOP-8833. fs -text should make sure to call inputstream.seek(0) before using input stream. Contributed by Tom White and Harsh J. (harsh) (Revision 1388869)

          Result = SUCCESS
          harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388869
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Display.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2755 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2755/ ) HADOOP-8833 . fs -text should make sure to call inputstream.seek(0) before using input stream. Contributed by Tom White and Harsh J. (harsh) (Revision 1388869) Result = SUCCESS harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388869 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Display.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #2818 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2818/)
          HADOOP-8833. fs -text should make sure to call inputstream.seek(0) before using input stream. Contributed by Tom White and Harsh J. (harsh) (Revision 1388869)

          Result = SUCCESS
          harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388869
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Display.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2818 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2818/ ) HADOOP-8833 . fs -text should make sure to call inputstream.seek(0) before using input stream. Contributed by Tom White and Harsh J. (harsh) (Revision 1388869) Result = SUCCESS harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388869 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Display.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
          Hide
          Harsh J added a comment -

          Committed to trunk. Pending commit to branch-2 (svn up seems to be going too slow here).

          Show
          Harsh J added a comment - Committed to trunk. Pending commit to branch-2 (svn up seems to be going too slow here).
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #2777 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2777/)
          HADOOP-8833. fs -text should make sure to call inputstream.seek(0) before using input stream. Contributed by Tom White and Harsh J. (harsh) (Revision 1388869)

          Result = FAILURE
          harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388869
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Display.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2777 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2777/ ) HADOOP-8833 . fs -text should make sure to call inputstream.seek(0) before using input stream. Contributed by Tom White and Harsh J. (harsh) (Revision 1388869) Result = FAILURE harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388869 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Display.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
          Harsh J made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Target Version/s 3.0.0, 2.0.3-alpha [ 12320357, 12323273 ]
          Fix Version/s 2.0.3-alpha [ 12323273 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1174 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1174/)
          HADOOP-8833. fs -text should make sure to call inputstream.seek(0) before using input stream. Contributed by Tom White and Harsh J. (harsh) (Revision 1388869)

          Result = SUCCESS
          harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388869
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Display.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1174 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1174/ ) HADOOP-8833 . fs -text should make sure to call inputstream.seek(0) before using input stream. Contributed by Tom White and Harsh J. (harsh) (Revision 1388869) Result = SUCCESS harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388869 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Display.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1205 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1205/)
          HADOOP-8833. fs -text should make sure to call inputstream.seek(0) before using input stream. Contributed by Tom White and Harsh J. (harsh) (Revision 1388869)

          Result = SUCCESS
          harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388869
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Display.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1205 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1205/ ) HADOOP-8833 . fs -text should make sure to call inputstream.seek(0) before using input stream. Contributed by Tom White and Harsh J. (harsh) (Revision 1388869) Result = SUCCESS harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388869 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Display.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
          Arun C Murthy made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Harsh J
              Reporter:
              Harsh J
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development