Details
-
Type:
Sub-task
-
Status: Resolved
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: None
-
Component/s: task
-
Labels:None
-
Hadoop Flags:Reviewed
Description
Other native parts of Hadoop will automatically disable snappy support if snappy is not present and -Drequire.snappy is not passed. native-task should do the same. (right now, it fails to build if snappy is missing)
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Do we also need some check in the java side that knows whether the Native code has snappy support built in?
Show
Todd Lipcon
added a comment - Do we also need some check in the java side that knows whether the Native code has snappy support built in?
Thanks, Manu. Looks good, +1
Changes of the new patch:
1. use system provided snappy header files, remove builtin snappy header
2. Java side delegate the codec check to a native function, NativeRuntime.supportCompressionCodec(codecName : String)
Show
Sean Zhong
added a comment - Thanks, Manu. Looks good, +1
Changes of the new patch:
1. use system provided snappy header files, remove builtin snappy header
2. Java side delegate the codec check to a native function, NativeRuntime.supportCompressionCodec(codecName : String)
patch updated. Changes include
1. checks for all compression codecs in the java side whether they have native support built in.
2. remove snappy header files and include system snappy library in native code
Show
Manu Zhang
added a comment - patch updated. Changes include
1. checks for all compression codecs in the java side whether they have native support built in.
2. remove snappy header files and include system snappy library in native code
Todd Lipcon, can you take a look at the new patch?
Show
Sean Zhong
added a comment - Todd Lipcon , can you take a look at the new patch?
Sorry for the delay here. The patch mostly looks good. Just a couple of nits:
+ if (job.getBoolean(MRJobConfig.MAP_OUTPUT_COMPRESS, false) == true) {
style: just use if (job.getBoolean(...)) instead of comparing {{ == true }}.
+ String codec = job.get(MRJobConfig.MAP_OUTPUT_COMPRESS_CODEC); + if (NativeRuntime.supportCompressionCodec(codec.getBytes(Charsets.UTF_8)) == false) {
Same – just use !NativeRuntime.supportCompressionCodec(...) instead of comparing {{ == false}}
Also, English nit: should be named supportsCompressionCodec (note the 's')
+ String message = "Native output collector don't support compression codec " + codec;
Nit: should be "doesn't support" instead of "don't support"
+ *
+ * @param codecName
+ * @return
Remove these empty JavaDoc lines, since the main description of the function is sufficient to understand it.
Show
Todd Lipcon
added a comment - Sorry for the delay here. The patch mostly looks good. Just a couple of nits:
+ if (job.getBoolean(MRJobConfig.MAP_OUTPUT_COMPRESS, false ) == true ) {
style: just use if (job.getBoolean(...)) instead of comparing {{ == true }}.
+ String codec = job.get(MRJobConfig.MAP_OUTPUT_COMPRESS_CODEC);
+ if (NativeRuntime.supportCompressionCodec(codec.getBytes(Charsets.UTF_8)) == false ) {
Same – just use !NativeRuntime.supportCompressionCodec(...) instead of comparing {{ == false}}
Also, English nit: should be named supportsCompressionCodec (note the 's')
+ String message = "Native output collector don't support compression codec " + codec;
Nit: should be "doesn't support" instead of "don't support"
+ *
+ * @param codecName
+ * @ return
Remove these empty JavaDoc lines, since the main description of the function is sufficient to understand it.
thanks, Todd. I've updated the patch accordingly and also fixed "don't support" in other places.
Show
Manu Zhang
added a comment - thanks, Todd. I've updated the patch accordingly and also fixed "don't support" in other places.
+1, looks good to me. I'll commit this momentarily
Show
Todd Lipcon
added a comment - +1, looks good to me. I'll commit this momentarily
Actually it looks like this doesn't apply against tip of the branch. Is this patch based on another in-flight patch, Manu?
Show
Todd Lipcon
added a comment - Actually it looks like this doesn't apply against tip of the branch. Is this patch based on another in-flight patch, Manu?
sorry, my local branch was not updated. Recreated a patch against latest branch.
Show
Manu Zhang
added a comment - sorry, my local branch was not updated. Recreated a patch against latest branch.
Committed to branch at r1616115. Thanks!
Show
Sean Zhong
added a comment - Committed to branch at r1616115. Thanks!
commited to branch MR-2841 at r1616115
Show
Sean Zhong
added a comment - commited to branch MR-2841 at r1616115
SUCCESS: Integrated in Hadoop-Yarn-trunk #679 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/679/)
MAPREDUCE-5976. native-task: should not fail to build if snappy is missing (Manu Zhang) (seanzhong: rev 432f641bc2f6c7fdb67da578233cbe07fdfbf1ea)
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/lib/TestReadBuffer.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeMapOutputCollectorDelegator.java
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/Compressions.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestIFile.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/org_apache_hadoop_mapred_nativetask_NativeRuntime.h
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestCompressions.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy-c.h
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/NativeRuntimeJniImpl.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy.h
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeRuntime.java
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/SnappyCodec.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/util/SnappyUtil.java
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/CMakeLists.txt
- hadoop-mapreduce-project/CHANGES.
MAPREDUCE-2841.txt
Show
Hudson
added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #679 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/679/ )
MAPREDUCE-5976 . native-task: should not fail to build if snappy is missing (Manu Zhang) (seanzhong: rev 432f641bc2f6c7fdb67da578233cbe07fdfbf1ea)
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/lib/TestReadBuffer.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeMapOutputCollectorDelegator.java
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/Compressions.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestIFile.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/org_apache_hadoop_mapred_nativetask_NativeRuntime.h
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestCompressions.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy-c.h
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/NativeRuntimeJniImpl.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy.h
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeRuntime.java
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/SnappyCodec.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/util/SnappyUtil.java
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/CMakeLists.txt
hadoop-mapreduce-project/CHANGES. MAPREDUCE-2841 .txt
FAILURE: Integrated in Hadoop-Mapreduce-trunk #1895 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1895/)
MAPREDUCE-5976. native-task: should not fail to build if snappy is missing (Manu Zhang) (seanzhong: rev 432f641bc2f6c7fdb67da578233cbe07fdfbf1ea)
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy.h
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/Compressions.cc
- hadoop-mapreduce-project/CHANGES.
MAPREDUCE-2841.txt - hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/util/SnappyUtil.java
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/org_apache_hadoop_mapred_nativetask_NativeRuntime.h
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/NativeRuntimeJniImpl.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestCompressions.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy-c.h
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeRuntime.java
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/CMakeLists.txt
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/SnappyCodec.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeMapOutputCollectorDelegator.java
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/lib/TestReadBuffer.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestIFile.cc
Show
Hudson
added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1895 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1895/ )
MAPREDUCE-5976 . native-task: should not fail to build if snappy is missing (Manu Zhang) (seanzhong: rev 432f641bc2f6c7fdb67da578233cbe07fdfbf1ea)
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy.h
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/Compressions.cc
hadoop-mapreduce-project/CHANGES. MAPREDUCE-2841 .txt
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/util/SnappyUtil.java
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/org_apache_hadoop_mapred_nativetask_NativeRuntime.h
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/NativeRuntimeJniImpl.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestCompressions.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy-c.h
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeRuntime.java
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/CMakeLists.txt
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/SnappyCodec.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeMapOutputCollectorDelegator.java
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/lib/TestReadBuffer.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestIFile.cc
SUCCESS: Integrated in Hadoop-Hdfs-trunk #1870 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1870/)
MAPREDUCE-5976. native-task: should not fail to build if snappy is missing (Manu Zhang) (seanzhong: rev 432f641bc2f6c7fdb67da578233cbe07fdfbf1ea)
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/NativeRuntimeJniImpl.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeMapOutputCollectorDelegator.java
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/Compressions.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy-c.h
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestCompressions.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeRuntime.java
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestIFile.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/org_apache_hadoop_mapred_nativetask_NativeRuntime.h
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/CMakeLists.txt
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy.h
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/util/SnappyUtil.java
- hadoop-mapreduce-project/CHANGES.
MAPREDUCE-2841.txt - hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/SnappyCodec.cc
- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/lib/TestReadBuffer.cc
Show
Hudson
added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #1870 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1870/ )
MAPREDUCE-5976 . native-task: should not fail to build if snappy is missing (Manu Zhang) (seanzhong: rev 432f641bc2f6c7fdb67da578233cbe07fdfbf1ea)
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/NativeRuntimeJniImpl.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeMapOutputCollectorDelegator.java
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/Compressions.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy-c.h
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestCompressions.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/NativeRuntime.java
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/TestIFile.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/lib/org_apache_hadoop_mapred_nativetask_NativeRuntime.h
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/CMakeLists.txt
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/snappy.h
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/java/org/apache/hadoop/mapred/nativetask/util/SnappyUtil.java
hadoop-mapreduce-project/CHANGES. MAPREDUCE-2841 .txt
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/SnappyCodec.cc
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/test/lib/TestReadBuffer.cc
Thanks, Manu,
The patch looks good. +1