Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.20.3, 0.21.1, 0.22.0
    • Fix Version/s: 0.22.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      IFile assumes that when it has a codec it can always get a compressor. This fails when mapred.compress.map.output is true but the native libraries are not installed, resulting in an NPE:

      java.lang.NullPointerException
      at org.apache.hadoop.mapred.IFile$Writer.<init>(IFile.java:102)
      at org.apache.hadoop.mapred.MapTask$MapOutputBuffer.sortAndSpill(MapTask.java:1198)
      at org.apache.hadoop.mapred.MapTask$MapOutputBuffer.flush(MapTask.java:1091)
      at org.apache.hadoop.mapred.MapTask.runOldMapper(MapTask.java:359)
      at org.apache.hadoop.mapred.MapTask.run(MapTask.java:307)
      at org.apache.hadoop.mapred.Child.main(Child.java:170)
      

      Let's make IFile handle this case by logging and using non-compressed streams.l

      1. mapred-1784-2.patch
        5 kB
        Eli Collins
      2. mapred-1784-1.patch
        2 kB
        Eli Collins

        Activity

        Hide
        Eli Collins added a comment -

        Patch attached. Confirmed that running pi with mapred.compress.map.output and no native libraries now works.

        Show
        Eli Collins added a comment - Patch attached. Confirmed that running pi with mapred.compress.map.output and no native libraries now works.
        Hide
        Tom White added a comment -

        If the user has asked for the output to be compressed but it can't get a compressor then is a warning sufficient? It probably is, since it doesn't impact correctness, only efficiency. Do others agree?

        Eli, is it possible to add a unit test for this?

        Show
        Tom White added a comment - If the user has asked for the output to be compressed but it can't get a compressor then is a warning sufficient? It probably is, since it doesn't impact correctness, only efficiency. Do others agree? Eli, is it possible to add a unit test for this?
        Hide
        Eli Collins added a comment -

        I made it a warning since GzipCodec can function w/o native libraries (kind of, it provides java input and output streams, eg GzipOutputStream, though it returns null for createCompressor) and didn't want this to stop something that should otherwise work (ie the user specifying org.apache.hadoop.io.compress.GzipCodec doesn't imply native libraries must be used). However it does seem like they should be notified that they requested a codec that can't be created. How about CodecPool#getCompressor throws an exception instead of returning null indicating it couldn't get a compressor for the given codec (because codec.createCompressor() returned null)?

        I could add a test that covers this method in TestCodec. I think adding a test to TestReduceTask that uses a Gzip codec would also cover this.

        Show
        Eli Collins added a comment - I made it a warning since GzipCodec can function w/o native libraries (kind of, it provides java input and output streams, eg GzipOutputStream, though it returns null for createCompressor) and didn't want this to stop something that should otherwise work (ie the user specifying org.apache.hadoop.io.compress.GzipCodec doesn't imply native libraries must be used). However it does seem like they should be notified that they requested a codec that can't be created. How about CodecPool#getCompressor throws an exception instead of returning null indicating it couldn't get a compressor for the given codec (because codec.createCompressor() returned null)? I could add a test that covers this method in TestCodec. I think adding a test to TestReduceTask that uses a Gzip codec would also cover this.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12444268/mapred-1784-1.patch
        against trunk revision 943372.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

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

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/183/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/183/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/183/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/183/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/12444268/mapred-1784-1.patch against trunk revision 943372. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/183/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/183/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/183/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/183/console This message is automatically generated.
        Hide
        Tom White added a comment -

        Marking as open since it still needs a test.

        Show
        Tom White added a comment - Marking as open since it still needs a test.
        Hide
        Eli Collins added a comment -

        I merged HDFS-727 from trunk into branch-0.20, ran the libhdfs test and committed.

        Show
        Eli Collins added a comment - I merged HDFS-727 from trunk into branch-0.20, ran the libhdfs test and committed.
        Hide
        Eli Collins added a comment -

        Ignore the last comment, wrong jira.

        Show
        Eli Collins added a comment - Ignore the last comment, wrong jira.
        Hide
        Eli Collins added a comment -

        Patch attached. Preserves the behavior of the last patch of logging rather than NPE. Adds new class TestIFile which fails w/o this change, passes with it.

        Show
        Eli Collins added a comment - Patch attached. Preserves the behavior of the last patch of logging rather than NPE. Adds new class TestIFile which fails w/o this change, passes with it.
        Hide
        Tom White added a comment -

        I've just committed this. Thanks Eli!

        Show
        Tom White added a comment - I've just committed this. Thanks Eli!
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #541 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/541/)
        MAPREDUCE-1784. IFile should check for null compressor. Contributed by Eli Collins.

        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #541 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/541/ ) MAPREDUCE-1784 . IFile should check for null compressor. Contributed by Eli Collins.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #643 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk/643/)

        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #643 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk/643/ )

          People

          • Assignee:
            Eli Collins
            Reporter:
            Eli Collins
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development