Hadoop Common
  1. Hadoop Common
  2. HADOOP-6803

Add native gzip read/write coverage to TestCodec

    Details

    • Type: Test Test
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.22.0
    • Component/s: io
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Looking at ZlibCompressor I noticed that the finished member is never modified, and is therefore always false. This means ZlibCompressor#finished will always return false so CompressorStream#close loops indefinitely in finish:

       
            while (!compressor.finished()) {
              compress();
            }
      

      I modifed TestCodec#testGzipCodecWrite to also cover writing using the native lib and confirmed the hang with jstack. The fix is simple, ZlibCompressor should record when it's been finished.

      1. hadoop-6803-3.patch
        3 kB
        Eli Collins
      2. hadoop-6803-2.patch
        3 kB
        Eli Collins
      3. hadoop-6803-1.patch
        4 kB
        Eli Collins

        Activity

        Konstantin Shvachko made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk #428 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/428/)
        HADOOP-6803. Add native gzip read/write coverage to TestCodec. Contributed by Eli Collins.

        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk #428 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/428/ ) HADOOP-6803 . Add native gzip read/write coverage to TestCodec. Contributed by Eli Collins.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #360 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/360/)
        HADOOP-6803. Add native gzip read/write coverage to TestCodec. Contributed by Eli Collins.

        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #360 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/360/ ) HADOOP-6803 . Add native gzip read/write coverage to TestCodec. Contributed by Eli Collins.
        Tom White made changes -
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        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
        Tom White added a comment -

        +1 Looks good.

        Show
        Tom White added a comment - +1 Looks good.
        Eli Collins made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Eli Collins made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Eli Collins made changes -
        Attachment hadoop-6803-3.patch [ 12452333 ]
        Hide
        Eli Collins added a comment -

        Updated patch resolves with trunk.

        Show
        Eli Collins added a comment - Updated patch resolves with trunk.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12446204/hadoop-6803-2.patch
        against trunk revision 951081.

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

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

        +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/Hadoop-Patch-h1.grid.sp2.yahoo.net/75/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/75/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/75/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/75/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/12446204/hadoop-6803-2.patch against trunk revision 951081. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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/Hadoop-Patch-h1.grid.sp2.yahoo.net/75/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/75/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/75/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/75/console This message is automatically generated.
        Eli Collins made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Eli Collins made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Affects Version/s 0.21.0 [ 12313563 ]
        Fix Version/s 0.21.0 [ 12313563 ]
        Hide
        Eli Collins added a comment -

        I figured it out. In the earlier change I had remove the synchronized modifier from ZlibCompressor#finished which would cause the jvm to miss the update from the jni code.

        Show
        Eli Collins added a comment - I figured it out. In the earlier change I had remove the synchronized modifier from ZlibCompressor#finished which would cause the jvm to miss the update from the jni code.
        Eli Collins made changes -
        Summary ZlibCompressor hangs on close Add native gzip read/write coverage to TestCodec
        Issue Type Bug [ 1 ] Test [ 6 ]
        Eli Collins made changes -
        Attachment hadoop-6803-2.patch [ 12446204 ]
        Hide
        Eli Collins added a comment -

        Attached patch. Reverts the ZlibCompressor change, keeps the test. Thanks Chris.

        Show
        Eli Collins added a comment - Attached patch. Reverts the ZlibCompressor change, keeps the test. Thanks Chris.
        Hide
        Eli Collins added a comment -

        Your right, I forgot that methods not fields have the "native" modifier. The "finish" field is read from the native code and "finished" is updated there so it looks like it should work. What's strange is if I revert the change to ZlibCompressor it passes now, earlier when I first added the test and ran with the native lib it hung with jstack showing the following:

        "main" prio=10 tid=0x0000000040b19000 nid=0x36f0 runnable [0x00007f1257588000]
           java.lang.Thread.State: RUNNABLE
                at org.apache.hadoop.io.compress.zlib.ZlibCompressor.deflateBytesDirect(Native Method)
                at org.apache.hadoop.io.compress.zlib.ZlibCompressor.compress(ZlibCompressor.java:359)
                - locked <0x00007f12495063e0> (a org.apache.hadoop.io.compress.GzipCodec$GzipZlibCompressor)
                at org.apache.hadoop.io.compress.CompressorStream.compress(CompressorStream.java:76)
                at org.apache.hadoop.io.compress.CompressorStream.finish(CompressorStream.java:86)
                at org.apache.hadoop.io.compress.CompressorStream.close(CompressorStream.java:97)
                at sun.nio.cs.StreamEncoder.implClose(StreamEncoder.java:301)
                at sun.nio.cs.StreamEncoder.close(StreamEncoder.java:130)
                - locked <0x00007f12495167c8> (a java.io.OutputStreamWriter)
                at java.io.OutputStreamWriter.close(OutputStreamWriter.java:216)
                at java.io.BufferedWriter.close(BufferedWriter.java:248)
                - locked <0x00007f12495167c8> (a java.io.OutputStreamWriter)
                at org.apache.hadoop.io.compress.TestCodec.testGzipCodecWrite(TestCodec.java:653)
        
        Show
        Eli Collins added a comment - Your right, I forgot that methods not fields have the "native" modifier. The "finish" field is read from the native code and "finished" is updated there so it looks like it should work. What's strange is if I revert the change to ZlibCompressor it passes now, earlier when I first added the test and ran with the native lib it hung with jstack showing the following: "main" prio=10 tid=0x0000000040b19000 nid=0x36f0 runnable [0x00007f1257588000] java.lang. Thread .State: RUNNABLE at org.apache.hadoop.io.compress.zlib.ZlibCompressor.deflateBytesDirect(Native Method) at org.apache.hadoop.io.compress.zlib.ZlibCompressor.compress(ZlibCompressor.java:359) - locked <0x00007f12495063e0> (a org.apache.hadoop.io.compress.GzipCodec$GzipZlibCompressor) at org.apache.hadoop.io.compress.CompressorStream.compress(CompressorStream.java:76) at org.apache.hadoop.io.compress.CompressorStream.finish(CompressorStream.java:86) at org.apache.hadoop.io.compress.CompressorStream.close(CompressorStream.java:97) at sun.nio.cs.StreamEncoder.implClose(StreamEncoder.java:301) at sun.nio.cs.StreamEncoder.close(StreamEncoder.java:130) - locked <0x00007f12495167c8> (a java.io.OutputStreamWriter) at java.io.OutputStreamWriter.close(OutputStreamWriter.java:216) at java.io.BufferedWriter.close(BufferedWriter.java:248) - locked <0x00007f12495167c8> (a java.io.OutputStreamWriter) at org.apache.hadoop.io.compress.TestCodec.testGzipCodecWrite(TestCodec.java:653)
        Hide
        Chris Douglas added a comment -

        Isn't the finished member set by the JNI code?

        Show
        Chris Douglas added a comment - Isn't the finished member set by the JNI code?
        Eli Collins made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Eli Collins added a comment -

        I confirmed the test passes locally with the native lib built. On hudson the native test should warn and bail if the native library is not built when the test is run.

        Show
        Eli Collins added a comment - I confirmed the test passes locally with the native lib built. On hudson the native test should warn and bail if the native library is not built when the test is run.
        Eli Collins made changes -
        Field Original Value New Value
        Attachment hadoop-6803-1.patch [ 12446195 ]
        Hide
        Eli Collins added a comment -

        Patch attached.

        Show
        Eli Collins added a comment - Patch attached.
        Eli Collins created issue -

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development