[
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
