Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-12972

Lz4Compressor#getLibraryName returns the wrong version number

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: 2.6.0
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: native
    • Labels:
      None
    • Target Version/s:

      Description

      HADOOP-11184 updated lz4 to "r123", but hadoop checknative -a still prints "revision:99".

      $ hadoop checknative -a
      16/03/29 11:42:40 INFO bzip2.Bzip2Factory: Successfully loaded & initialized native-bzip2 library system-native
      16/03/29 11:42:40 INFO zlib.ZlibFactory: Successfully loaded & initialized native-zlib library
      Native library checking:
      hadoop:  true /opt/cloudera/parcels/CDH-5.8.0-1.cdh5.8.0.p0.1209/lib/hadoop/lib/native/libhadoop.so.1.0.0
      zlib:    true /lib64/libz.so.1
      snappy:  true /opt/cloudera/parcels/CDH-5.8.0-1.cdh5.8.0.p0.1209/lib/hadoop/lib/native/libsnappy.so.1
      lz4:     true revision:99
      bzip2:   true /lib64/libbz2.so.1
      openssl: true /usr/lib64/libcrypto.so
      

        Activity

        Hide
        jzhuge John Zhuge added a comment -

        Starting from lz4 release "r132", lz4 -h will show:

        *** LZ4 CLI 64-bits r132 (lib 1.7.2), by Yann Collet ***
        

        The lib version "1.7.2" can correlate with Hadoop hadoop checknative -a lz4 output "revision:10301" (i.e., 1.3.1).

        Show
        jzhuge John Zhuge added a comment - Starting from lz4 release "r132", lz4 -h will show: *** LZ4 CLI 64-bits r132 (lib 1.7.2), by Yann Collet *** The lib version "1.7.2" can correlate with Hadoop hadoop checknative -a lz4 output "revision:10301" (i.e., 1.3.1).
        Hide
        jzhuge John Zhuge added a comment -

        Fix lz4 cli to display library version. The pull request https://github.com/Cyan4973/lz4/pull/194 has been merged.

        In that pull request, also add an api LZ4_versionString to return version string in the format of MAJOR.MINOR.RELEASE.

        Show
        jzhuge John Zhuge added a comment - Fix lz4 cli to display library version. The pull request https://github.com/Cyan4973/lz4/pull/194 has been merged. In that pull request, also add an api LZ4_versionString to return version string in the format of MAJOR.MINOR.RELEASE .
        Hide
        jzhuge John Zhuge added a comment -

        Hi Colin P. McCabe,

        Agree with you on that the library version should be printed since we only bundles the library code, thus ok with your patch.

        programs/lz4cli.c is in github.com:Cyan4973/lz4.git. In order to correlate Hadoop lz4 library with OS lz4 command, things get a little messy because there are actually 3 "versions": git tag, cli version, and library version. Here is a table of these 3 columns from github.com:Cyan4973/lz4.git:

        $ lz4vers 
               tag   cliver   libver
          lz4-r130     r128    1.7.0
              r116   v1.1.5    1.1.3
              r117   v1.1.5    1.1.3
              r118   v1.2.0    1.2.0
              r119   v1.2.0    1.2.0
              r120   v1.2.0    1.3.0
              r121   v1.2.0    1.3.0
              r122     r122    1.3.0
              r123     r122    1.3.1
              r124     r122    1.4.0
              r125     r125    1.4.1
              r126     r126    1.5.0
              r127     r126    1.5.0
              r128     r128    1.6.0
              r129     r128    1.7.0
              r130     r128    1.7.0
              r131     r128    1.7.1
           rc129v0     r128    1.7.0
        

        This seems to be the rules of versioning: cli and lib versions can be bumped independently; if one of them is bumped, tag is bumped; if neither of them is bumped, tag may still be bumped.

        IMO, the correct fix is for lz4 -h to display lib version in addition to cli version. Created https://github.com/Cyan4973/lz4/issues/192.

        Show
        jzhuge John Zhuge added a comment - Hi Colin P. McCabe , Agree with you on that the library version should be printed since we only bundles the library code, thus ok with your patch. programs/lz4cli.c is in github.com:Cyan4973/lz4.git . In order to correlate Hadoop lz4 library with OS lz4 command, things get a little messy because there are actually 3 "versions": git tag, cli version, and library version. Here is a table of these 3 columns from github.com:Cyan4973/lz4.git : $ lz4vers tag cliver libver lz4-r130 r128 1.7.0 r116 v1.1.5 1.1.3 r117 v1.1.5 1.1.3 r118 v1.2.0 1.2.0 r119 v1.2.0 1.2.0 r120 v1.2.0 1.3.0 r121 v1.2.0 1.3.0 r122 r122 1.3.0 r123 r122 1.3.1 r124 r122 1.4.0 r125 r125 1.4.1 r126 r126 1.5.0 r127 r126 1.5.0 r128 r128 1.6.0 r129 r128 1.7.0 r130 r128 1.7.0 r131 r128 1.7.1 rc129v0 r128 1.7.0 This seems to be the rules of versioning: cli and lib versions can be bumped independently; if one of them is bumped, tag is bumped; if neither of them is bumped, tag may still be bumped. IMO, the correct fix is for lz4 -h to display lib version in addition to cli version. Created https://github.com/Cyan4973/lz4/issues/192 .
        Hide
        cmccabe Colin P. McCabe added a comment -

        Hi John Zhuge,

        The output of the "lz4" command on Ubuntu isn't relevant. Hadoop uses its own bundled version of the lz4 source code, stored in ./hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/compress/lz4/lz4.c. I don't even have the lz4 command installed on my system, and I don't know what the lz4cli.c source code file you are referencing is (it sounds like something in a 3rd party package that Hadoop doesn't use.)

        You can see the way that the version is calculated here in ./hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/compress/lz4/lz4.h

        #define LZ4_VERSION_MAJOR    1    /* for major interface/format changes  */
        #define LZ4_VERSION_MINOR    3    /* for minor interface/format changes  */
        #define LZ4_VERSION_RELEASE  1    /* for tweaks, bug-fixes, or development */
        #define LZ4_VERSION_NUMBER (LZ4_VERSION_MAJOR *100*100 + LZ4_VERSION_MINOR *100 + LZ4_VERSION_RELEASE)
        

        So a version number of 10301 corresponds to major = 1, minor = 3, release = 1. It seems fairly easy to read. If you want to improve this so that it prints out things in dot-dot-dot format, that would be a useful patch. But it doesn't seem strictly necessary.

        Show
        cmccabe Colin P. McCabe added a comment - Hi John Zhuge , The output of the "lz4" command on Ubuntu isn't relevant. Hadoop uses its own bundled version of the lz4 source code, stored in ./hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/compress/lz4/lz4.c . I don't even have the lz4 command installed on my system, and I don't know what the lz4cli.c source code file you are referencing is (it sounds like something in a 3rd party package that Hadoop doesn't use.) You can see the way that the version is calculated here in ./hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/compress/lz4/lz4.h #define LZ4_VERSION_MAJOR 1 /* for major interface /format changes */ #define LZ4_VERSION_MINOR 3 /* for minor interface /format changes */ #define LZ4_VERSION_RELEASE 1 /* for tweaks, bug-fixes, or development */ #define LZ4_VERSION_NUMBER (LZ4_VERSION_MAJOR *100*100 + LZ4_VERSION_MINOR *100 + LZ4_VERSION_RELEASE) So a version number of 10301 corresponds to major = 1, minor = 3, release = 1. It seems fairly easy to read. If you want to improve this so that it prints out things in dot-dot-dot format, that would be a useful patch. But it doesn't seem strictly necessary.
        Hide
        jzhuge John Zhuge added a comment -

        If we want to match Hadoop lz4 library version with OS lz4 command version, shall we make the following change instead of Patch 001?

        Java_org_apache_hadoop_io_compress_lz4_Lz4Compressor_getLibraryName(
          JNIEnv *env, jclass class
          ) {
          return (*env)->NewStringUTF(env, "r123");
        }
        

        If we take this approach, we must add comments reminding whoever upgrades LZ4 revision to update this hardcoded value.

        Show
        jzhuge John Zhuge added a comment - If we want to match Hadoop lz4 library version with OS lz4 command version, shall we make the following change instead of Patch 001? Java_org_apache_hadoop_io_compress_lz4_Lz4Compressor_getLibraryName( JNIEnv *env, jclass class ) { return (*env)->NewStringUTF(env, "r123" ); } If we take this approach, we must add comments reminding whoever upgrades LZ4 revision to update this hardcoded value.
        Hide
        jzhuge John Zhuge added a comment -

        Thanks Colin P. McCabe for the fix. Here is the test output of the patch:

        $ hadoop checknative
        16/03/30 00:13:19 INFO bzip2.Bzip2Factory: Successfully loaded & initialized native-bzip2 library system-native
        16/03/30 00:13:19 INFO zlib.ZlibFactory: Successfully loaded & initialized native-zlib library
        Native library checking:
        hadoop:  true /opt/cloudera/parcels/CDH-5.8.0-1.cdh5.8.0.p0.26/lib/hadoop/lib/native/libhadoop.so.1.0.0
        zlib:    true /lib/x86_64-linux-gnu/libz.so.1
        snappy:  false 
        lz4:     true revision:10301
        bzip2:   true /lib/x86_64-linux-gnu/libbz2.so.1
        openssl: true /usr/lib/x86_64-linux-gnu/libcrypto.so
        

        Ubuntu 14.04 lz4 -v output:

        $ lz4 -h
        *** LZ4 Compression CLI 64-bits r114, by Yann Collet (Apr 14 2014) ***
        Usage :
              lz4 [arg] [input] [output]
        ...
        

        The hadoop checknative shows library version "revision:10301", while Linux lz4 -h shows cli version "r114". There is no way to match library version LZ4_VERSION_NUMBER in lz.h and cli version LZ4_VERSION in lz4cli.c without the source code if it is 1-1 mapping.

        Show
        jzhuge John Zhuge added a comment - Thanks Colin P. McCabe for the fix. Here is the test output of the patch: $ hadoop checknative 16/03/30 00:13:19 INFO bzip2.Bzip2Factory: Successfully loaded & initialized native -bzip2 library system- native 16/03/30 00:13:19 INFO zlib.ZlibFactory: Successfully loaded & initialized native -zlib library Native library checking: hadoop: true /opt/cloudera/parcels/CDH-5.8.0-1.cdh5.8.0.p0.26/lib/hadoop/lib/ native /libhadoop.so.1.0.0 zlib: true /lib/x86_64-linux-gnu/libz.so.1 snappy: false lz4: true revision:10301 bzip2: true /lib/x86_64-linux-gnu/libbz2.so.1 openssl: true /usr/lib/x86_64-linux-gnu/libcrypto.so Ubuntu 14.04 lz4 -v output: $ lz4 -h *** LZ4 Compression CLI 64-bits r114, by Yann Collet (Apr 14 2014) *** Usage : lz4 [arg] [input] [output] ... The hadoop checknative shows library version "revision:10301", while Linux lz4 -h shows cli version "r114". There is no way to match library version LZ4_VERSION_NUMBER in lz.h and cli version LZ4_VERSION in lz4cli.c without the source code if it is 1-1 mapping.
        Hide
        cmccabe Colin P. McCabe added a comment -

        committed to 2.8, thanks

        Show
        cmccabe Colin P. McCabe added a comment - committed to 2.8, thanks
        Hide
        drankye Kai Zheng added a comment -

        The patch looks good. The reported cc warnings are not relevant and already covered elsewhere in HADOOP-12955.

        Show
        drankye Kai Zheng added a comment - The patch looks good. The reported cc warnings are not relevant and already covered elsewhere in HADOOP-12955 .
        Hide
        andrew.wang Andrew Wang added a comment -

        LGTM +1, thanks Colin and John

        Show
        andrew.wang Andrew Wang added a comment - LGTM +1, thanks Colin and John
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 19s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s 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 mvninstall 11m 24s trunk passed
        +1 compile 12m 49s trunk passed with JDK v1.8.0_74
        +1 compile 8m 19s trunk passed with JDK v1.7.0_95
        +1 mvnsite 1m 11s trunk passed
        +1 mvneclipse 0m 16s trunk passed
        +1 mvninstall 0m 48s the patch passed
        +1 compile 9m 21s the patch passed with JDK v1.8.0_74
        -1 cc 44m 22s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 3 new + 18 unchanged - 3 fixed = 21 total (was 21)
        +1 cc 9m 21s the patch passed
        +1 javac 9m 21s the patch passed
        +1 compile 7m 56s the patch passed with JDK v1.7.0_95
        -1 cc 52m 18s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 4 new + 27 unchanged - 4 fixed = 31 total (was 31)
        +1 cc 7m 56s the patch passed
        +1 javac 7m 56s the patch passed
        +1 mvnsite 1m 6s the patch passed
        +1 mvneclipse 0m 16s the patch passed
        +1 whitespace 0m 0s Patch has no whitespace issues.
        -1 unit 9m 53s hadoop-common in the patch failed with JDK v1.8.0_74.
        -1 unit 9m 7s hadoop-common in the patch failed with JDK v1.7.0_95.
        -1 asflicense 0m 21s Patch generated 3 ASF License warnings.
        73m 28s



        Reason Tests
        JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker
        JDK v1.7.0_95 Failed junit tests hadoop.metrics2.impl.TestGangliaMetrics
        JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:fbe3e86
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12795899/HADOOP-12972.001.patch
        JIRA Issue HADOOP-12972
        Optional Tests asflicense compile cc mvnsite javac unit
        uname Linux 7a03f956d562 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
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / f2aec4e
        Default Java 1.7.0_95
        Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
        cc root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/diff-compile-cc-root-jdk1.8.0_74.txt
        cc root-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/diff-compile-cc-root-jdk1.7.0_95.txt
        unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt
        unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
        unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
        JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/testReport/
        asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/patch-asflicense-problems.txt
        modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/console
        Powered by Apache Yetus 0.2.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 11m 24s trunk passed +1 compile 12m 49s trunk passed with JDK v1.8.0_74 +1 compile 8m 19s trunk passed with JDK v1.7.0_95 +1 mvnsite 1m 11s trunk passed +1 mvneclipse 0m 16s trunk passed +1 mvninstall 0m 48s the patch passed +1 compile 9m 21s the patch passed with JDK v1.8.0_74 -1 cc 44m 22s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 3 new + 18 unchanged - 3 fixed = 21 total (was 21) +1 cc 9m 21s the patch passed +1 javac 9m 21s the patch passed +1 compile 7m 56s the patch passed with JDK v1.7.0_95 -1 cc 52m 18s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 4 new + 27 unchanged - 4 fixed = 31 total (was 31) +1 cc 7m 56s the patch passed +1 javac 7m 56s the patch passed +1 mvnsite 1m 6s the patch passed +1 mvneclipse 0m 16s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. -1 unit 9m 53s hadoop-common in the patch failed with JDK v1.8.0_74. -1 unit 9m 7s hadoop-common in the patch failed with JDK v1.7.0_95. -1 asflicense 0m 21s Patch generated 3 ASF License warnings. 73m 28s Reason Tests JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker JDK v1.7.0_95 Failed junit tests hadoop.metrics2.impl.TestGangliaMetrics JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12795899/HADOOP-12972.001.patch JIRA Issue HADOOP-12972 Optional Tests asflicense compile cc mvnsite javac unit uname Linux 7a03f956d562 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f2aec4e Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 cc root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/diff-compile-cc-root-jdk1.8.0_74.txt cc root-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/diff-compile-cc-root-jdk1.7.0_95.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/testReport/ asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8957/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.

          People

          • Assignee:
            cmccabe Colin P. McCabe
            Reporter:
            jzhuge John Zhuge
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development