[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14988603#comment-14988603 ] Kai Zheng commented on HADOOP-11887: Thanks Colin a lot for the more review and the commit is very helpful for subsequent tasks. Yes, I can also address refinements in HADOOP-11996 and HADOOP-11540 if any, since the both are laid on this. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-HDFS-7285-v6.patch, HADOOP-11887-v1.patch, > HADOOP-11887-v10, HADOOP-11887-v2.patch, HADOOP-11887-v3.patch, > HADOOP-11887-v4.patch, HADOOP-11887-v5.patch, HADOOP-11887-v5.patch, > HADOOP-11887-v7.patch, HADOOP-11887-v8.patch, HADOOP-11887-v9.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14988535#comment-14988535 ] Colin Patrick McCabe commented on HADOOP-11887: --- Thanks for this, [~drankye]. It looks good to me. +1. I'll commit tomorrow if there's no more comments. It's probably easier to do any refinements in a follow-on JIRA. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-HDFS-7285-v6.patch, HADOOP-11887-v1.patch, > HADOOP-11887-v10, HADOOP-11887-v2.patch, HADOOP-11887-v3.patch, > HADOOP-11887-v4.patch, HADOOP-11887-v5.patch, HADOOP-11887-v5.patch, > HADOOP-11887-v7.patch, HADOOP-11887-v8.patch, HADOOP-11887-v9.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14986809#comment-14986809 ] Hadoop QA commented on HADOOP-11887: | (/) *{color:green}+1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 7s {color} | {color:blue} docker + precommit patch detected. {color} | | {color:blue}0{color} | {color:blue} patch {color} | {color:blue} 0m 8s {color} | {color:blue} The patch file was not named according to hadoop's naming conventions. Please see https://wiki.apache.org/hadoop/HowToContribute for instructions. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s {color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s {color} | {color:green} The patch appears to include 1 new or modified test files. {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 3m 13s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 27s {color} | {color:green} trunk passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 22s {color} | {color:green} trunk passed with JDK v1.7.0_79 {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 59s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 0s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m 0s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 0s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 0s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 0s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 18s {color} | {color:green} the patch passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 18s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 18s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 10s {color} | {color:green} the patch passed with JDK v1.7.0_79 {color} | | {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 10s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 10s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 54s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 0s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s {color} | {color:green} Patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m 2s {color} | {color:green} The patch has no ill-formed XML file. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m 0s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 0s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 0s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 24s {color} | {color:green} Patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 23m 45s {color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-03 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12770249/HADOOP-11887-v10 | | JIRA Issue | HADOOP-11887 | | Optional Tests | asflicense javac javadoc mvninstall unit xml compile cc findbugs checkstyle | | uname | Linux fa7e6168ed38 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 | /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-1a9afee/precommit/personality/hadoop.sh | | git r
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14978288#comment-14978288 ] Kai Zheng commented on HADOOP-11887: Hi Steve, we use the same set of building options to include ISA-L library on Windows like for Linux platform. The added text in BUILDING.txt follows existing conventions, like for snappy, openssl and etc. One thing I noted is, we don't mention dll stuffs (like hadoop.dll) for Windows, when mentioning so stuffs (like libhadoop.so). Please let me know if this sounds good or not. Thanks. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-HDFS-7285-v6.patch, HADOOP-11887-v1.patch, > HADOOP-11887-v2.patch, HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, > HADOOP-11887-v5.patch, HADOOP-11887-v5.patch, HADOOP-11887-v7.patch, > HADOOP-11887-v8.patch, HADOOP-11887-v9.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14978092#comment-14978092 ] Steve Loughran commented on HADOOP-11887: - does the diff to BUILDING.TXT cover windows too? > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-HDFS-7285-v6.patch, HADOOP-11887-v1.patch, > HADOOP-11887-v2.patch, HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, > HADOOP-11887-v5.patch, HADOOP-11887-v5.patch, HADOOP-11887-v7.patch, > HADOOP-11887-v8.patch, HADOOP-11887-v9.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14977566#comment-14977566 ] Hadoop QA commented on HADOOP-11887: | (/) *{color:green}+1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 11s {color} | {color:blue} docker + precommit patch detected. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s {color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s {color} | {color:green} The patch appears to include 1 new or modified test files. {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 3m 24s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 57s {color} | {color:green} trunk passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 39s {color} | {color:green} trunk passed with JDK v1.7.0_79 {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m 2s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 0s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m 0s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 0s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 0s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 0s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 50s {color} | {color:green} the patch passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 50s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 50s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 37s {color} | {color:green} the patch passed with JDK v1.7.0_79 {color} | | {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 37s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 37s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m 1s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 0s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s {color} | {color:green} Patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m 2s {color} | {color:green} The patch has no ill-formed XML file. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m 0s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 0s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 0s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 25s {color} | {color:green} Patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 26m 0s {color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-10-28 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12769111/HADOOP-11887-v9.patch | | JIRA Issue | HADOOP-11887 | | Optional Tests | asflicense javac javadoc mvninstall unit xml cc findbugs checkstyle compile | | uname | Linux c3a757f74cd5 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 | /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-2392ab4/dev-support/personality/hadoop.sh | | git revision | trunk / 68ce93c | | Default Java | 1.7.0_79 | | Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79 | | JDK v1.7.0_79 Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/79
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14976394#comment-14976394 ] Hadoop QA commented on HADOOP-11887: | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 6s {color} | {color:blue} docker + precommit patch detected. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s {color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s {color} | {color:green} The patch appears to include 1 new or modified test files. {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 3m 15s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 22s {color} | {color:green} trunk passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 3s {color} | {color:green} trunk passed with JDK v1.7.0_79 {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 14s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 13s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 34s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 51s {color} | {color:green} trunk passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 1s {color} | {color:green} trunk passed with JDK v1.7.0_79 {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m 34s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 14s {color} | {color:green} the patch passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 14s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 14s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 5s {color} | {color:green} the patch passed with JDK v1.7.0_79 {color} | | {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 5s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 5s {color} | {color:green} the patch passed {color} | | {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m 13s {color} | {color:red} Patch generated 1 new checkstyle issues in hadoop-common-project/hadoop-common (total was 7, now 7). {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 13s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s {color} | {color:green} Patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m 2s {color} | {color:green} The patch has no ill-formed XML file. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 46s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 55s {color} | {color:green} the patch passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 1s {color} | {color:green} the patch passed with JDK v1.7.0_79 {color} | | {color:red}-1{color} | {color:red} unit {color} | {color:red} 6m 5s {color} | {color:red} hadoop-common in the patch failed with JDK v1.8.0_60. {color} | | {color:red}-1{color} | {color:red} unit {color} | {color:red} 6m 19s {color} | {color:red} hadoop-common in the patch failed with JDK v1.7.0_79. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 23s {color} | {color:green} Patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 43m 34s {color} | {color:black} {color} | \\ \\ || Reason || Tests || | JDK v1.7.0_79 Failed junit tests | hadoop.security.TestShellBasedIdMapping | | | hadoop.metrics2.sink.TestFileSink | \\ \\ || Subsystem || Report/Notes || | Docker | Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-10-27 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12768960/HADOOP-11887-v8.patch | | JIRA Issue | HADOOP-11887 | | Optional Tests | asflicense javac javadoc mvnin
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14976340#comment-14976340 ] Kai Zheng commented on HADOOP-11887: Sorry looks like I missed some changes for Windows when rebased. Will fix this and test for Windows tommorrow. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-HDFS-7285-v6.patch, HADOOP-11887-v1.patch, > HADOOP-11887-v2.patch, HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, > HADOOP-11887-v5.patch, HADOOP-11887-v5.patch, HADOOP-11887-v7.patch, > HADOOP-11887-v8.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14976271#comment-14976271 ] Kai Zheng commented on HADOOP-11887: Colin, would you help review this one more time? Thanks! > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-HDFS-7285-v6.patch, HADOOP-11887-v1.patch, > HADOOP-11887-v2.patch, HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, > HADOOP-11887-v5.patch, HADOOP-11887-v5.patch, HADOOP-11887-v7.patch, > HADOOP-11887-v8.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14975939#comment-14975939 ] Hadoop QA commented on HADOOP-11887: | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 7s {color} | {color:blue} docker + precommit patch detected. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s {color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s {color} | {color:green} The patch appears to include 1 new or modified test files. {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 3m 6s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 23s {color} | {color:green} trunk passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 9s {color} | {color:green} trunk passed with JDK v1.7.0_79 {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 17s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 14s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 41s {color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 53s {color} | {color:green} trunk passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 1s {color} | {color:green} trunk passed with JDK v1.7.0_79 {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m 27s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 16s {color} | {color:green} the patch passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 16s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 16s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 6s {color} | {color:green} the patch passed with JDK v1.7.0_79 {color} | | {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 6s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 6s {color} | {color:green} the patch passed {color} | | {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m 13s {color} | {color:red} Patch generated 5 new checkstyle issues in hadoop-common-project/hadoop-common (total was 7, now 12). {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 13s {color} | {color:green} the patch passed {color} | | {color:red}-1{color} | {color:red} whitespace {color} | {color:red} 0m 0s {color} | {color:red} The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix. {color} | | {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m 1s {color} | {color:green} The patch has no ill-formed XML file. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 43s {color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 52s {color} | {color:green} the patch passed with JDK v1.8.0_60 {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 2s {color} | {color:green} the patch passed with JDK v1.7.0_79 {color} | | {color:red}-1{color} | {color:red} unit {color} | {color:red} 20m 41s {color} | {color:red} hadoop-common in the patch failed with JDK v1.8.0_60. {color} | | {color:red}-1{color} | {color:red} unit {color} | {color:red} 6m 25s {color} | {color:red} hadoop-common in the patch failed with JDK v1.7.0_79. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 23s {color} | {color:green} Patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 58m 20s {color} | {color:black} {color} | \\ \\ || Reason || Tests || | JDK v1.7.0_79 Failed junit tests | hadoop.metrics2.sink.TestFileSink | | | hadoop.metrics2.sink.TestFileSink | | JDK v1.7.0_79 Timed out junit tests | org.apache.hadoop.http.TestHttpServerLifecycle | \\ \\ || Subsystem || Report/Notes || | Docker | Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-10-27 | | JIRA Patch URL | https://issues.apache.org/jira/secure/a
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14973543#comment-14973543 ] Kai Zheng commented on HADOOP-11887: Will rebase this on trunk and update the patch. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-HDFS-7285-v6.patch, HADOOP-11887-v1.patch, > HADOOP-11887-v2.patch, HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, > HADOOP-11887-v5.patch, HADOOP-11887-v5.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14726452#comment-14726452 ] Colin Patrick McCabe commented on HADOOP-11887: --- bq. Thanks Colin for this another round of review and very good comments. I addressed most of them and the left one is about how to invoke the unit test. I agree with you that current way isn't good as it links the test codes into the production binary. So I will wrap the test codes into an executable program as I previously did. OK. bq. The questions in this way are: 1) how to tell the situation in the POM file that ISA-L isn't available so the test shouldn't be invoked. How about: we always run the test but in the test program we check if ISA-L is available or not, if not available it will exit (0) in the beginning, which wouldn't be regarded as failure. That would be fine. I suppose you would need to use dlopen / dlsym to load the symbols in that case. You would also need to build the test program even when ISA-L was not installed. If that's tricky, then perhaps you could add a shell code snippet in the pom.xml that checks to see if the test binary exists before it runs it. bq. The other problem is, how to come up the test program on Windows platform? It looks pretty heavy to write up a new native (test) program for Windows. Is it acceptable if we only support this test program for *nix platforms? Please help confirm these, thanks! It should be OK as long as it doesn't break the Windows build. Thanks, [~drankye]. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, HADOOP-11887-v5.patch, > HADOOP-11887-v5.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14724960#comment-14724960 ] Kai Zheng commented on HADOOP-11887: Thanks Colin for this another round of review and very good comments. I addressed most of them and the left one is about how to invoke the unit test. I agree with you that current way isn't good as it links the test codes into the production binary. So I will wrap the test codes into an executable program as I previously did. The questions in this way are: 1) how to tell the situation in the POM file that ISA-L isn't available so the test shouldn't be invoked. Previously I came up a new tool for the purpose but it didn't work in your side. How about: we always run the test but in the test program we check if ISA-L is available or not, if not available it will exit (0) in the beginning, which wouldn't be regarded as failure. 2) The other problem is, how to come up the test program on Windows platform? It looks pretty heavy to write up a new native (test) program for Windows. Is it acceptable if we only support this test program for *nix platforms? Please help confirm these, thanks! > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, HADOOP-11887-v5.patch, > HADOOP-11887-v5.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14720772#comment-14720772 ] Colin Patrick McCabe commented on HADOOP-11887: --- Sorry for the hiatus. I was on vacation recently. It looks like the two most recent patch files have the same name, {{HADOOP-11887-v5.patch}}. I am reviewing the August 10 one. This looks good overall! Thanks for updating the bundle.snappy, bundle.openssl etc checks in {{hadoop-project-dist/pom.xml}} to use safe shell string comparisons. {{hadoop-common-project/hadoop-common/src/CMakeLists.txt}}: The D variable has been removed in trunk, you have to use SRC now here. {{ErasureCodeNative.java}}: Could rephrase "ISA-L isn't supported in the building" as "libhadoop was built without ISA-L support" {{ErasureCodeNative.java}}: In the JavaDoc comment, {{Is the native ISA-L library loaded & initialized? Throw exception if not.}}, the ampersand is likely to cause problems with Javadoc. It might be easier to replace it with "and" (or you could escape it... I think JavaDoc uses HTML escapes or something like that.) {{io/erasurecode/erasure_code_test.c}}: It's nice that we can now run the test through Java, but it looks like this test code is getting linked into {{libhadoop.so}} and would be deployed in production. We generally avoid pulling in test code to production where possible, because it increases code size and may pull in dependencies or add extra APIs, etc. You should be able to use the native unit test stuff to avoid this. See {{hadoop-hdfs-project/hadoop-hdfs/pom.xml}} -- specifically the section on {{test_libhdfs_threaded}} and {{test_native_mini_dfs}} for an example of this. Looks good, should be ready to go aside from that! > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, HADOOP-11887-v5.patch, > HADOOP-11887-v5.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14680311#comment-14680311 ] Alan Burlison commented on HADOOP-11887: Hello, I'm on holiday from August 10th returning on August 17th, I'll reply to you when I'm back. If it's something urgent, please contact bonnie.cor...@oracle.com Thanks, -- Alan Burlison -- > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, HADOOP-11887-v5.patch, > HADOOP-11887-v5.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14648571#comment-14648571 ] Kai Zheng commented on HADOOP-11887: Thanks Colin for all the detailed comments!! Kindly let me address them next week and update then. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, HADOOP-11887-v5.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14648303#comment-14648303 ] Colin Patrick McCabe commented on HADOOP-11887: --- Thanks, [~drankye]. {code} + * Use -Disal.prefix to specify a nonstandard location for the libisal +library files. You do not need this option if you have installed isal. {code} Should be "you do not need this option if you have installed ISAL to the system library path." {code} +${D}/io/erasurecode/coder/erasure_code_native.c) + + +endif (ISAL_LIBRARY) {code} Nitpick: it's weird to have two blank lines here? {{CMakeLists.txt}}: {{REQUIRE_ISAL}} is not verified. I was able to build without ISAL installed and {{-Drequire.isal}} on the maven command line, and there was no error. {code} + static { +if (NativeCodeLoader.isNativeCodeLoaded() && +NativeCodeLoader.buildSupportsIsal()) { + try { +loadLibrary(); +nativeIsalLoaded = true; + } catch (Throwable t) { +LOG.error("Failed to load ISA-L", t); + } +} + } + ... + /** + * Is the native ISA-L library loaded & initialized? Throw exception if not. + */ + public static void checkNativeCodeLoaded() { +if (!nativeIsalLoaded) { + throw new RuntimeException("Native ISA-L library not available: " + + "this version of libhadoop was built without " + + "ISA-L support."); +} + } {code} Hmm. It seems that {{checkNativeCodeLoaded}} will throw an exception stating that "this verison of libhadoop was built without ISA-L support" even if libhadoop.so *was* built with ISA-L support, but we can't find the ISA-L library. Rather than having a boolean, you should have a {{loadingFailureReason}} that gets initialized in a static block. Look at {{DomainSocket.java}} or {{OpensslAesCtrCryptoCodec.java}} to see how to do this. Then you can throw an exception with this loading failure reason in that check function. {{NativeLibraryChecker.java}}: thanks for adding support here. It would be nice to distinguish between a hadoop build that doesn't support ISA-L, and one that can't find {{libisal.so}}. Take a look at how openssl does this: {code} if (OpensslCipher.getLoadingFailureReason() != null) { openSslDetail = OpensslCipher.getLoadingFailureReason(); openSslLoaded = false; } else { openSslDetail = OpensslCipher.getLibraryName(); openSslLoaded = true; } {code} This is nice because {{hadoop checknative}} will display exactly *why* openssl can't be loaded (i.e. no build support, can't find library, library version incompatible, etc.) rather than just saying "not loaded" and leaving admins to guess why. Also, you should display the full path to the isal-l library, not just a constant string. Example output: {code} Native library checking: hadoop: true /h/lib/native/libhadoop.so.1.0.0 zlib:true /lib64/libz.so.1 snappy: true /usr/local/lib64/libsnappy.so.1 {code} This may be important if I am getting a version that is different than what I thought (for example, perhaps I thought I was getting {{/opt/snappy/libsnappy.so.1}}, but I am actually getting {{/usr/local/lib64/libsnappy.so.1}}) Take a look at {{SnappyCompressor.c}} to see how to get the actual library path. {code} +if [ "${bundle.isal}" = "true" ] ; then + cd "${isal.lib}" + $$TAR *isa* | (cd $${TARGET_DIR}/; $$UNTAR) +fi {code} Should be {{if \[ "x$\{bundle.isal\}" = x"true" \] ;}} to avoid the shell getting a syntax error when {{bundle.isal}} is not set. There are a few cases where you have a space at the end of a line, which may trigger checkstyle warnings. Thanks > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, HADOOP-11887-v5.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14638037#comment-14638037 ] Kai Zheng commented on HADOOP-11887: Thanks [~cmccabe] for the comments and suggestions. I just updated the patch, it's tested on Windows as well. bq. you need to coordinate with the build folks to make sure that ISA-L is installed on all our Jenkins machines. Good idea, when the library is equipped in the building boxes, we can give this patch a chance to Jenkins build and ensure its quality. I will do that. Thanks. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch, HADOOP-11887-v5.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14634139#comment-14634139 ] Colin Patrick McCabe commented on HADOOP-11887: --- Thanks, [~drankye]. Take a look at {{TestCryptoStreamsWithOpensslAesCtrCryptoCodec.java}} for an example of testing if a specific native library is loaded and failing the build if not. Of course, prior to adding a test like this, you need to coordinate with the build folks to make sure that ISA-L is installed on all our Jenkins machines. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14629255#comment-14629255 ] Kai Zheng commented on HADOOP-11887: 1. Thanks Colin for the suggestions about ISA-L configurations the approach. I agree it's too early to consider support of multiple libraries here and good to focus on the ISA-L support for now. I will change the building configurations as you proposed. 2. Yes I will add new entry for ISA-L in the command "hadoop checknative". 3. As I discussed off-line with you, "hadoop checknative" might be heavy to be used in the unit test process (in Maven pom file) since the tool may be half-baked at the invoking time. It's also too heavy to add a new tool just for the guarding of the executing of the native test program, so I will remove it. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14628449#comment-14628449 ] Colin Patrick McCabe commented on HADOOP-11887: --- bq. I totally got your concern. I'm using the general name erasurecode instead of isal directly because I wish the overall work won't couple with ISA-L too tightly. In future other native libraries like Jerasure or even hardware based ones could also be supported as well without too much change. I'm thinking that the native APIs defined in erasure_code.h should be general enough so other native libraries could also be easily mapped to it, thus when building, other libraries could also be passed to via the mentioned options. Note require.erasurecode is used to enable it, and if enabled, erasurecode.prefix should be specified to provide the library place; If not enabled (by default for now), the building should go as normally, and the result won't contain any erasure code related symbols. The logic is similar to existing codes like for snappy library. I think you are confusing two different things: how to configure ISA-L, and supporting multiple different erasure encoding libraries. ISA-L configuration includes: * Where to find it (\-Disal.prefix, \-Disal.lib) * Whether to bundle it (\-Dbundle.isal) * Whether to fail the build if it is not found (\-Dbundle.isal) These things should have "ISA-L" in the name since they pertain only to that library, and not to any other libraries. Naming them "erasurecode" rather than "isal" will actually make it hard to support more than one erasure encoding library in the future, since we would only have one set of configuration knobs for both libraries, whereas we would need at least two. If you want to support multiple erasure encoding libraries, you will need some kind of codec interface. This is in addition to however you would configure the other libraries, not in replacement of it. In any case, I think it would be unwise to try to write a plugin interface until we have support for at least one other erasure encoding library. Let's keep the scope of this JIRA focused just on ISA-L. If people want to come back later and add more libraries, they certainly can. bq. You found another place I need to change. Yes I need to add an entry for erasrue code in the tool. The question here is, I'm wondering if it can serve the purpose of the new tool here, because executing of hadoop checknative may need some configuration or tweak to make it work, the new tool can directly run just after it's out, so can be used in maven unit tests cleanly. I understand introducing a new tool just for ONE native test may be too heavy, if you agree, maybe we could go simple, say, if no native library is available, the native test program could just exit with a warning message? Do we need more native tests anyway in future? If so the checking with the new tool may sound more reasonable. Please don't add a new tool just for this. Add support to {{hadoop checknative}}. If "hadoop checknative... need\[s\] some configuration or tweak to make it work" then the admin should know that their libraries are not being properly found. This is important information. Thanks > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14626033#comment-14626033 ] Kai Zheng commented on HADOOP-11887: Hi [~cmccabe], I have some understanding about the following two major questions and need to discuss with you further. bq. I can see you're using -Drequire.erasurecode, -Derasurecode.prefix, etc. for the build parameters. This might create confusion, though. Users might ask if erasure encoding is disabled if they build without the -Derasurecode... options. Given that the name of the library is ISA-L, maybe we can have -Drequire.ISAL, -DISAL.prefix, etc.? Seems more straightforward. I totally got your concern. I'm using the general name {{erasurecode}} instead of {{isal}} directly because I wish the overall work won't couple with ISA-L too tightly. In future other native libraries like Jerasure or even hardware based ones could also be supported as well without too much change. I'm thinking that the native APIs defined in {{erasure_code.h}} should be general enough so other native libraries could also be easily mapped to it, thus when building, other libraries could also be passed to via the mentioned options. Note {{require.erasurecode}} is used to enable it, and if enabled, {{erasurecode.prefix}} should be specified to provide the library place; If not enabled (by default for now), the building should go as normally, and the result won't contain any erasure code related symbols. The logic is similar to existing codes like for snappy library. bq. I think we should avoid adding a new test program for this, and instead add this functionality to checknative. You found another place I need to change. Yes I need to add an entry for erasrue code in the tool. The question here is, I'm wondering if it can serve the purpose of the new tool here, because executing of {{hadoop checknative}} may need some configuration or tweak to make it work, the new tool can directly run just after it's out, so can be used in maven unit tests cleanly. I understand introducing a new tool just for ONE native test may be too heavy, if you agree, maybe we could go simple, say, if no native library is available, the native test program could just exit with a warning message? Do we need more native tests anyway in future? If so the checking with the new tool may sound more reasonable. Would you let know your thoughts? Thanks again. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14626004#comment-14626004 ] Kai Zheng commented on HADOOP-11887: Thanks [~ste...@apache.org] for the ideas. bq. native work like this really does need the windows stuff done at the same time. Otherwise we can assume that build will break the moment this patch goes in. It's indeed, when I go through the building on Windows recently, I have to change and fix some places to make it compile on Windows platform. Great I have got it done, and will be able to provide update soon. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14625997#comment-14625997 ] Kai Zheng commented on HADOOP-11887: Sorry for the very late update. I'm going to refine the work according to the above review comments and provide new patch soon. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14591204#comment-14591204 ] Kai Zheng commented on HADOOP-11887: Thanks [~cmccabe] for the more detailed review and great suggestions! I will re-work on this some time later addressing all the comments received so far. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14591085#comment-14591085 ] Colin Patrick McCabe commented on HADOOP-11887: --- Thanks for waiting. I can see you're using {{-Drequire.erasurecode}}, {{-Derasurecode.prefix}}, etc. for the build parameters. This might create confusion, though. Users might ask if erasure encoding is disabled if they build without the {{-Derasurecode...}} options. Given that the name of the library is ISA-L, maybe we can have {{-Drequire.ISAL}}, {{-DISAL.prefix}}, etc.? Seems more straightforward. {code} +add_executable(check_native_lib +${T}/util/check_native_lib.c +) {code} I can see why you wanted this new program, to tell you what was enabled and what wasn't. But in Hadoop, this functionality is provided by {{hadoop checknatve}}. Using {{checknative}} is a much better way of doing it since it means that cluster administrators can figure out whether they have deployed a version of {{libhadoop.so}} with or without any given feature. I think we should avoid adding a new test program for this, and instead add this functionality to {{checknative}}. Here is the current output of {{hadoop checknative}}: {code} cmccabe@keter:~/> hadoop checknative 15/06/17 18:12:34 INFO bzip2.Bzip2Factory: Successfully loaded & initialized native-bzip2 library system-native 15/06/17 18:12:34 INFO zlib.ZlibFactory: Successfully loaded & initialized native-zlib library Native library checking: hadoop: true /home/cmccabe/hadoop4/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/lib/native/libhadoop.so.1.0.0 zlib:true /lib64/libz.so.1 snappy: true /usr/local/lib64/libsnappy.so.1 lz4: true revision:99 bzip2: true /usr/lib64/libbz2.so.1 openssl: true /usr/lib64/libcrypto.so {code} It would be very helpful to see an ISAL line in there, I think. {code} 49 typedef unsigned char u8; {code} Should use the standard {{uint8_t}} type here from {{stdint.h}} {code} 278 memset(src_in_err, 0, TEST_SOURCES); {code} Would be nice to use {{sizeof(src_in_err)}} here in case that size ever changes. thanks, [~drankye]. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14578126#comment-14578126 ] Colin Patrick McCabe commented on HADOOP-11887: --- Sorry about being slow to review on this. Hadoop summit is this week and I am kind of swamped. Great to know that ISA-L will soon be portable to many OSes. I will try to do a review on this in the next few days > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14575630#comment-14575630 ] Alan Burlison commented on HADOOP-11887: Greg, Kai thanks for the updates. Let me know if you want any Solaris testing done on the new version when it is out. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14575407#comment-14575407 ] Kai Zheng commented on HADOOP-11887: Thanks Greg for looking at this and giving the comments! > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14575393#comment-14575393 ] Greg Tucker commented on HADOOP-11887: -- Next release will use autoconf and should clear up these porting issues. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14575391#comment-14575391 ] Greg Tucker commented on HADOOP-11887: -- Alan, Sorry about the confusion. The make test target for ISA-L is just a bit too inclusive now and tries to build and run all the arch-specific tests on any machine. This unfortunately includes tests that explicitly call a version with AVX that will certainly fail on a machine without AVX support. This doesn't mean that the multi-binary functions don't to the right thing and run an appropriate version. We will be sure in the next release that make test doesn't try to run all these cross-arch tests. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14574307#comment-14574307 ] Alan Burlison commented on HADOOP-11887: The Makefile issues were that GNU and Solaris strip use different command-line arguments and that on Solaris it is necessary to explicitly specify -m32 or -m64 on the compiler compiler command-line to specify if a 32 or 64 bit library is required. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14574286#comment-14574286 ] Kai Zheng commented on HADOOP-11887: Thanks Alan for the good report! I will involve the ISA-L team to handle this and update here. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14574284#comment-14574284 ] Kai Zheng commented on HADOOP-11887: Thanks [~steve_l] for the review and comments! I will update the patch addressing them, and make the patch also build on Windows. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14572836#comment-14572836 ] Alan Burlison commented on HADOOP-11887: I've pulled the ISA-L source down and built it on Solaris x86. It needs a bit of Makefile tweaking but it builds as a LP64 library and 'make test' and 'make perf' succeed. One caveat: on an older machine 'make test' dumps core with SIGILL, presumably because the library isn't checking if all the instructions it uses are actually available. This could potentially cause problems on Linux & Windows as well, as it's a CPU feature rather than an OS one. The machine that all the tests pass on has the following instruction set extensions: amd64: avx xsave pclmulqdq aes sse4.2 sse4.1 ssse3 popcnt tscp ahf cx16 sse3 sse2 sse fxsr mmx cmov amd_sysc cx8 tsc fpu efs f16c rdrand i386: avx xsave pclmulqdq aes sse4.2 sse4.1 ssse3 popcnt tscp ahf cx16 sse3 sse2 sse fxsr mmx cmov sep cx8 tsc fpu efs f16c rdrand The one that some of the tests fail on has these: amd64: xsave sse4.1 ssse3 ahf cx16 sse3 sse2 sse fxsr mmx cmov amd_sysc cx8 tsc fpu i386: xsave sse4.1 ssse3 ahf cx16 sse3 sse2 sse fxsr mmx cmov sep cx8 tsc fpu The failing tests are: gf_vect_mul_avx_test gf_vect_dot_prod_avx_test > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14572498#comment-14572498 ] Steve Loughran commented on HADOOP-11887: - Build commentary # {{pom.xml}} L672that {{}} should't be using {{}}, its better to use a list of {{}} elements. Why? Less brittle to spaces in the paths # put ant.contrib version into the hadoop-project/pom.xml # {{hadoop-project/pom.xml}}. there must be only one {{}} declaration; that entry was already broken. Merge them to one declaration. # native work like this really does need the windows stuff done at the same time. Otherwise we can assume that build will break the moment this patch goes in. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14568372#comment-14568372 ] Kai Zheng commented on HADOOP-11887: Hi [~cmccabe], would you have more review for this? Thanks. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14562546#comment-14562546 ] Kai Zheng commented on HADOOP-11887: The test results on Linux: 1. When ISA-L is available: {noformat} [INFO] [INFO] --- maven-antrun-plugin:1.7:run (native_tests) @ hadoop-common --- [WARNING] Parameter tasks is deprecated, use target instead [INFO] Executing tasks main: [exec] CRC 16384 bytes @ 512 bytes per checksum X 100 iterations = 1.47 [exec] CRC 16384 bytes @ 512 bytes per checksum X 100 iterations = 14.83 [exec] /home/drankye/workspace/hadoop3/hadoop-common-project/hadoop-common/target/native/test_bulk_crc32: SUCCESS. [exec] HADOOP_ERASURECODE_LIBRARY is available as:libisal.so [exec] erasure_code_test [exec] done EC tests: Pass [INFO] Executed tasks [INFO] [INFO] BUILD SUCCESS {noformat} 2. When ISA-L isn't available: {noformat} [INFO] --- maven-antrun-plugin:1.7:run (native_tests) @ hadoop-common --- [WARNING] Parameter tasks is deprecated, use target instead [INFO] Executing tasks main: [exec] /home/drankye/workspace/hadoop3/hadoop-common-project/hadoop-common/target/native/test_bulk_crc32: SUCCESS. [exec] CRC 16384 bytes @ 512 bytes per checksum X 100 iterations = 1.5 [exec] CRC 16384 bytes @ 512 bytes per checksum X 100 iterations = 15.2 [exec] HADOOP_ERASURECODE_LIBRARY isn't available [exec] Result: 1 [echo] Erasure code native library isn't available, test skipped: 1 [INFO] Executed tasks [INFO] [INFO] BUILD SUCCESS {noformat} No tests on Windows platform yet. Could I open separate issue to build, test and fix for Windows? > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14562456#comment-14562456 ] Kai Zheng commented on HADOOP-11887: I'm not feeling perfect in the update because in the following change it used the deprecated {{task}} instead of {{target}}. I have no better idea to make it work. Colin how do you like this? Thanks. {code} - - - - - + ${skipTests} + + + + + + - + + + + + + + + + + + + +ant-contrib +ant-contrib +20020829 + + {code} > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch, HADOOP-11887-v4.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14559921#comment-14559921 ] Colin Patrick McCabe commented on HADOOP-11887: --- The run-test macro in the ant build is clever. However, it seems that these unit tests will now fail if the erasure encoding library is not present. Can you add a check to this so that the native unit test binary is not executed if it doesn't exist? +1 aside from that > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch, > HADOOP-11887-v3.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14552040#comment-14552040 ] Kai Zheng commented on HADOOP-11887: Hi [~cmccabe], I will update the patch here as you suggested. Meanwhile, would you help also take a look at the initial patches attached in HADOOP-11996 that bases on this work and HADOOP-11540 that comes up the resultant native erasure coders? I wish with the two patches it may be more clear how this work will be utilized, and makes sense overall in such organizing, but I'm not very sure. Your suggestion will be valuable and appreciated. Thanks in advance. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14535553#comment-14535553 ] Kai Zheng commented on HADOOP-11887: Thanks Colin for the comments. I'll update the patch following the good ideas. Just to make sure before proceed: bq. Suggest renaming this simply to LOAD_DYNAMIC_SYMBOL I used a different name in order to avoid conflict with the one defined in {{org_apache_hadoop.h}}, though I thought it should be ok to re-define the macro locally in the file. So yes I will simply rename the macro name to LOAD_DYNAMIC_SYMBOL. bq. Do you want to add the new unit test to the set of native unit tests? It's a good idea and why not. The test codes should be conditionally run into regarding the ISA-L library available or not, and there are two ways: 1) Run the executable anyway, but if ISA-L not available and loading it failed, the test program exits 0 (maybe reporting a warning message?). Easier to do this in the c program. 2) Check the library first, if not available, then not execute the test program at all. Not clear to me yet how to do this in pom.xml. Would you suggest and help? Thanks. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14535359#comment-14535359 ] Colin Patrick McCabe commented on HADOOP-11887: --- Thanks for updating the patch. {code} 106 char* load_erasurecode_lib() { 107 static void* libec = NULL; 108 static char errMsg[1000]; 109 110 if (libec != NULL) { 111 return NULL; 112 } {code} I really would prefer not to be using global buffers for error messages. How about having the caller pass in a buffer and a size, and using snprintf to add an error message if necessary? For example, {code} void do_stuff(char *err, size_t err_len) { if (problem with the foo) { snprintf(err, err_len, "problem with the foo") return; } err[0] = '\0'; // no error } {code} {code} 63 /* A helper macro to dlsym the requisite dynamic symbol in NON-JNI env. */ 64 #define LOAD_DYNAMIC_SYMBOL_NON_JNI(func_ptr, handle, symbol) \ 65if ((func_ptr = my_dlsym(handle, symbol)) == NULL) { \ 66 return "Failed to load symbol" symbol; \ 67} {code} Even in a JNI environment, {{dlopen}} and {{dlsym}} still work the same way. Our JNI code does use dlopen and dlsym in many cases to avoid a hard dependency between {{libhadoop.so}} and the libraries that it makes use of. Suggest renaming this simply to {{LOAD_DYNAMIC_SYMBOL}}. Do you want to add the new unit test to the set of native unit tests? See the pom.xml for hadoop-hdfs for an example: {code} native_tests test run ${skipTests} {code} > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14530207#comment-14530207 ] Kai Zheng commented on HADOOP-11887: Hi [~cmccabe], I updated the patch according to your review comments. It avoided changing into {{org_apache_hadoop.h}} by providing its own versions in {{erasure_code.c}}. I would really love to keep the tests from ISA-L library to verify the integration before bunmping into Java side. All your other points are carefully addressed. Would you help review one more time? Thanks! > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch, HADOOP-11887-v2.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14529604#comment-14529604 ] Kai Zheng commented on HADOOP-11887: Thanks Yi for taking care of this. Thanks Colin for the review and great comments. bq. Why are these variables global? Why is load_erasure_code_lib returning a value that no functions are checking? Yes I should and could avoid these global variables. {{load_erasure_code_lib}} doesn't report exception but returns error that's to be checked by the caller. The caller may just display the error (in pure native tool) or throw exception with the error (in JNI Java). This patch was introducing ISA-L and setting up the basic things to prepare for native erasure coders (Ref. HADOOP-11540), and the main codes using this setup and calling the function will be attached elsewhere. bq. In general, we shouldn't have every function calling load_erasure_code_lib. Instead, a static block in the java file should call load_erasure_code_lib once on startup. If the function fails, it needs to raise a JNI exception. I agree and will avoid calling the function in every exposed API. In native coders (like patch to be attached in HADOOP-11540) the Java code does as you suggested. bq. I would really prefer not to change org_apache_hadoop.h here just for the sake of a unit test. Let's just not call these functions if we aren't using JNI. OK, maybe I could move the changes to it out of there. I put the change there in case there could be similar situations like this (invoking a native call in non-JNI environment) in future. bq. Should this default to true? I don't think any Linux distros provide this library, so it's important to bundle it. I agree. ISA-L isn't something very general, and bundling it would be easier in most cases. Will update the patch accordingly. Thanks. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14529061#comment-14529061 ] Colin Patrick McCabe commented on HADOOP-11887: --- {code} 25 #cmakedefine HADOOP_ERASURECODE_LIBRARY "libisal.so" {code} This should be set based on the name we found in the {{CMakeLists.txt}}. {code} 76 static void *libec = NULL; 77 char errMsg[1000]; 78 79 char* load_erasure_code_lib() { {code} Why are these variables global? Why is {{load_erasure_code_lib}} returning a value that no functions are checking? In general, we shouldn't have every function calling {{load_erasure_code_lib}}. Instead, a static block in the java file should call {{load_erasure_code_lib}} once on startup. If the function fails, it needs to raise a JNI exception. I would really prefer not to change {{org_apache_hadoop.h}} here just for the sake of a unit test. Let's just not call these functions if we aren't using JNI. {code} 44 false {code} Should this default to true? I don't think any Linux distros provide this library, so it's important to bundle it. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14527736#comment-14527736 ] Yi Liu commented on HADOOP-11887: - Hi [~cmccabe] and [~andrew.wang], do you have time to review this, appreciate if you guys can help? Since you are native experts :) > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-11887) Introduce Intel ISA-L erasure coding library for the native support
[ https://issues.apache.org/jira/browse/HADOOP-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14520689#comment-14520689 ] Yi Liu commented on HADOOP-11887: - Thanks Kai for the work, I will take look at it in following few days. > Introduce Intel ISA-L erasure coding library for the native support > --- > > Key: HADOOP-11887 > URL: https://issues.apache.org/jira/browse/HADOOP-11887 > Project: Hadoop Common > Issue Type: Sub-task > Components: io >Reporter: Kai Zheng >Assignee: Kai Zheng > Attachments: HADOOP-11887-v1.patch > > > This is to introduce Intel ISA-L erasure coding library for the native > support, via dynamic loading mechanism (dynamic module, like *.so in *nix and > *.dll on Windows). -- This message was sent by Atlassian JIRA (v6.3.4#6332)