hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Nauroth (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-9481) Broken conditional logic with HADOOP_SNAPPY_LIBRARY
Date Sun, 21 Apr 2013 03:09:16 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-9481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13637457#comment-13637457
] 

Chris Nauroth commented on HADOOP-9481:
---------------------------------------

Ivan, thank you for the further explanation.  I understand the problem now.

I think a simpler fix would be to add {{#include "org_apache_hadoop.h"}} as the first line
of SnappyCompressor.c and SnappyDecompressor.c.  Can you check if that would work?

Right now, this gets included transitively through {{#include "org_apache_hadoop_io_compress_snappy.h"}},
but that's too late.  Including it explicitly as the first line would guarantee that {{UNIX}}
or {{WINDOWS}} gets defined before any other header or code processing.  This also would guarantee
that config.h gets included for {{UNIX}}, and therefore {{HADOOP_SNAPPY_LIBRARY}} would be
defined.  This is the same approach used in NativeIO.c.

In general, both the .h and .c files are structured such that they expect {{UNIX}}, {{WINDOWS}},
{{HADOOP_SNAPPY_LIBRARY}}, and other build configuration dependencies to be defined before
the preprocessor handles anything else.  Therefore, I think it's best that we always {{#include
"org_apache_hadoop.h"}} as the first line in any file.

BTW, I have build environments for both Linux and Windows ready to go, so I can volunteer
to test the patch cross-platform after we resolve this feedback.  I don't have any test jobs
ready to go that use Snappy, so I can't fully verify that, but I assume you already tested
that part before submitting the patch.

Thank you for addressing this!

                
> Broken conditional logic with HADOOP_SNAPPY_LIBRARY
> ---------------------------------------------------
>
>                 Key: HADOOP-9481
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9481
>             Project: Hadoop Common
>          Issue Type: Bug
>    Affects Versions: 3.0.0
>            Reporter: Vadim Bondarev
>            Priority: Minor
>         Attachments: HADOOP-9481-trunk--N1.patch
>
>
> The problem is a regression introduced by recent fix https://issues.apache.org/jira/browse/HADOOP-8562
.
> That fix makes some improvements for Windows platform, but breaks native code work on
Unix.
> Namely, let's see the diff HADOOP-8562 of the file hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/compress/snappy/SnappyCompressor.c
:  
> {noformat}
> --- hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/compress/snappy/SnappyCompressor.c
> +++ hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/compress/snappy/SnappyCompressor.c
> @@ -16,12 +16,18 @@
>   * limitations under the License.
>   */
> -#include <dlfcn.h>
> +
> +#if defined HADOOP_SNAPPY_LIBRARY
> +
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#ifdef UNIX
> +#include <dlfcn.h>
>  #include "config.h"
> +#endif // UNIX
> +
>  #include "org_apache_hadoop_io_compress_snappy.h"
>  #include "org_apache_hadoop_io_compress_snappy_SnappyCompressor.h"
> @@ -81,7 +87,7 @@ JNIEXPORT jint JNICALL Java_org_apache_hadoop_io_compress_snappy_SnappyCompresso
>    UNLOCK_CLASS(env, clazz, "SnappyCompressor");
>    if (uncompressed_bytes == 0) {
> -    return 0;
> +    return (jint)0;
>    }
>    // Get the output direct buffer
> @@ -90,7 +96,7 @@ JNIEXPORT jint JNICALL Java_org_apache_hadoop_io_compress_snappy_SnappyCompresso
>    UNLOCK_CLASS(env, clazz, "SnappyCompressor");
>    if (compressed_bytes == 0) {
> -    return 0;
> +    return (jint)0;
>    }
>    /* size_t should always be 4 bytes or larger. */
> @@ -109,3 +115,5 @@ JNIEXPORT jint JNICALL Java_org_apache_hadoop_io_compress_snappy_SnappyCompresso
>    (*env)->SetIntField(env, thisj, SnappyCompressor_uncompressedDirectBufLen, 0);
>    return (jint)buf_len;
>  }
> +
> +#endif //define HADOOP_SNAPPY_LIBRARY
> {noformat}
> Here we see that all the class implementation got enclosed into "if defined HADOOP_SNAPPY_LIBRARY"
directive, and the point is that "HADOOP_SNAPPY_LIBRARY" is *not* defined. 
> This causes the class implementation to be effectively empty, what, in turn, causes the
UnsatisfiedLinkError to be thrown in the runtime upon any attempt to invoke the native methods
implemented there.
> The actual intention of the authors of HADOOP-8562 was (as we suppose) to invoke "include
config.h", where "HADOOP_SNAPPY_LIBRARY" is defined. But currently it is *not* included because
it resides *inside* "if defined HADOOP_SNAPPY_LIBRARY" block.
> Similar situation with "ifdef UNIX", because UNIX or WINDOWS variables are defined in
"org_apache_hadoop.h", which is indirectly included through "include "org_apache_hadoop_io_compress_snappy.h"",
and in the current code this is done *after* code "ifdef UNIX", so in the current code the
block "ifdef UNIX" is *not* executed on UNIX.
> The suggested patch fixes the described problems by reordering the "include" and "if"
preprocessor directives accordingly, bringing the methods of class org.apache.hadoop.io.compress.snappy.SnappyCompressor
back to work again.
> Of course, Snappy native libraries must be installed to build and invoke snappy native
methods.
> (Note: there was a mistype in commit message: 8952 written in place of 8562: 
> HADOOP-8952. Enhancements to support Hadoop on Windows Server and Windows Azure environments.
Contributed by Ivan Mitic, Chuan Liu, Ramya Sunil, Bikas Saha, Kanna Karanam, John Gordon,
Brandon Li, Chris Nauroth, David Lao, Sumadhur Reddy Bolli, Arpit Agarwal, Ahmed El Baz, Mike
Liddell, Jing Zhao, Thejas Nair, Steve Maine, Ganeshan Iyer, Raja Aluri, Giridharan Kesavan,
Ramya Bharathi Nimmagadda.
>    git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1453486 13f79535-47bb-0310-9956-ffa450edef68
> )

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message