[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14649289#comment-14649289 ] Martin Walsh commented on HADOOP-7824: -- Addressed the nits as recommended. Just re-spinning locally to make sure all looks good. I changed ReadaheadPool.java and a number of other files to allow us to utilise the static import, as mentioned in the comments above. e.g. in ReadaheadPool.java: -fd, off, len, NativeIO.POSIX.POSIX_FADV_WILLNEED); +fd, off, len, POSIX_FADV_WILLNEED); Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Fix For: 2.8.0 Attachments: HADOOP-7824.001.patch, HADOOP-7824.002.patch, HADOOP-7824.003.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14649545#comment-14649545 ] Colin Patrick McCabe commented on HADOOP-7824: -- bq. I changed ReadaheadPool.java and a number of other files to allow us to utilise the static import, as mentioned in the comments above. e.g. in ReadaheadPool.java: OK, fair enough. Thanks, Martin. +1 pending Jenkins. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Fix For: 2.8.0 Attachments: HADOOP-7824.001.patch, HADOOP-7824.002.patch, HADOOP-7824.003.patch, HADOOP-7824.004.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14649761#comment-14649761 ] Hadoop QA commented on HADOOP-7824: --- \\ \\ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | pre-patch | 19m 32s | Pre-patch trunk compilation is healthy. | | {color:green}+1{color} | @author | 0m 0s | The patch does not contain any @author tags. | | {color:green}+1{color} | tests included | 0m 0s | The patch appears to include 2 new or modified test files. | | {color:green}+1{color} | javac | 7m 41s | There were no new javac warning messages. | | {color:green}+1{color} | javadoc | 9m 43s | There were no new javadoc warning messages. | | {color:green}+1{color} | release audit | 0m 24s | The applied patch does not increase the total number of release audit warnings. | | {color:red}-1{color} | checkstyle | 1m 55s | The applied patch generated 67 new checkstyle issues (total was 81, now 145). | | {color:green}+1{color} | whitespace | 0m 1s | The patch has no lines that end in whitespace. | | {color:green}+1{color} | install | 1m 20s | mvn install still works. | | {color:green}+1{color} | eclipse:eclipse | 0m 33s | The patch built with eclipse:eclipse. | | {color:green}+1{color} | findbugs | 5m 3s | The patch does not introduce any new Findbugs (version 3.0.0) warnings. | | {color:green}+1{color} | common tests | 22m 20s | Tests passed in hadoop-common. | | {color:green}+1{color} | mapreduce tests | 0m 20s | Tests passed in hadoop-mapreduce-client-shuffle. | | {color:red}-1{color} | hdfs tests | 161m 29s | Tests failed in hadoop-hdfs. | | | | 231m 15s | | \\ \\ || Reason || Tests || | Failed unit tests | hadoop.hdfs.server.namenode.ha.TestStandbyIsHot | \\ \\ || Subsystem || Report/Notes || | Patch URL | http://issues.apache.org/jira/secure/attachment/12748198/HADOOP-7824.004.patch | | Optional Tests | javadoc javac unit findbugs checkstyle | | git revision | trunk / 93d50b7 | | checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/7385/artifact/patchprocess/diffcheckstylehadoop-common.txt | | hadoop-common test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7385/artifact/patchprocess/testrun_hadoop-common.txt | | hadoop-mapreduce-client-shuffle test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7385/artifact/patchprocess/testrun_hadoop-mapreduce-client-shuffle.txt | | hadoop-hdfs test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7385/artifact/patchprocess/testrun_hadoop-hdfs.txt | | Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/7385/testReport/ | | Java | 1.7.0_55 | | uname | Linux asf909.gq1.ygridcore.net 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 | | Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/7385/console | This message was automatically generated. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Fix For: 2.8.0 Attachments: HADOOP-7824.001.patch, HADOOP-7824.002.patch, HADOOP-7824.003.patch, HADOOP-7824.004.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14648160#comment-14648160 ] Colin Patrick McCabe commented on HADOOP-7824: -- Thanks, [~martinw]! It looks really good. Just a few last nits: * {{SET_INT_CONST}}: can we change this to {{SET_INT_OR_RETURN}}? * {{setBooleanConstant}} / {{setIntConstant}} / etc. : can we change these to {{setStaticBoolean}}, {{setStaticInt}}, etc.? Two reasons: * Setting a constant is just weird... by definition, constants can't be set in C, so it feels confusing. These things are really static fields in the Java class, not constants at all. (They're not Java constants-- those can't be modified.) * If a macro performs flow control, like returning from a function, it should include that in the name to avoid confusion. Avoid whitespace changes. For example, we don't need this: {code} - int fd; + int fd; {code} The git history is much nicer when changes are minimal. Related: do we even need to change {{ReadaheadPool.java}}? It seems to just be adding a static import. +1 once those changes are addressed and Jenkins runs again. Thanks for working on this. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Fix For: 2.8.0 Attachments: HADOOP-7824.001.patch, HADOOP-7824.002.patch, HADOOP-7824.003.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14646278#comment-14646278 ] Hadoop QA commented on HADOOP-7824: --- \\ \\ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | pre-patch | 20m 4s | Pre-patch trunk compilation is healthy. | | {color:green}+1{color} | @author | 0m 0s | The patch does not contain any @author tags. | | {color:green}+1{color} | tests included | 0m 0s | The patch appears to include 2 new or modified test files. | | {color:green}+1{color} | javac | 7m 43s | There were no new javac warning messages. | | {color:green}+1{color} | javadoc | 9m 44s | There were no new javadoc warning messages. | | {color:green}+1{color} | release audit | 0m 23s | The applied patch does not increase the total number of release audit warnings. | | {color:red}-1{color} | checkstyle | 1m 57s | The applied patch generated 67 new checkstyle issues (total was 81, now 145). | | {color:green}+1{color} | whitespace | 0m 1s | The patch has no lines that end in whitespace. | | {color:green}+1{color} | install | 1m 22s | mvn install still works. | | {color:green}+1{color} | eclipse:eclipse | 0m 33s | The patch built with eclipse:eclipse. | | {color:green}+1{color} | findbugs | 5m 4s | The patch does not introduce any new Findbugs (version 3.0.0) warnings. | | {color:green}+1{color} | common tests | 22m 25s | Tests passed in hadoop-common. | | {color:green}+1{color} | mapreduce tests | 0m 20s | Tests passed in hadoop-mapreduce-client-shuffle. | | {color:red}-1{color} | hdfs tests | 161m 57s | Tests failed in hadoop-hdfs. | | | | 232m 27s | | \\ \\ || Reason || Tests || | Failed unit tests | hadoop.hdfs.server.namenode.ha.TestStandbyIsHot | \\ \\ || Subsystem || Report/Notes || | Patch URL | http://issues.apache.org/jira/secure/attachment/12747773/HADOOP-7824.003.patch | | Optional Tests | javadoc javac unit findbugs checkstyle | | git revision | trunk / 6374ee0 | | checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/7359/artifact/patchprocess/diffcheckstylehadoop-common.txt | | hadoop-common test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7359/artifact/patchprocess/testrun_hadoop-common.txt | | hadoop-mapreduce-client-shuffle test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7359/artifact/patchprocess/testrun_hadoop-mapreduce-client-shuffle.txt | | hadoop-hdfs test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7359/artifact/patchprocess/testrun_hadoop-hdfs.txt | | Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/7359/testReport/ | | Java | 1.7.0_55 | | uname | Linux asf900.gq1.ygridcore.net 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 | | Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/7359/console | This message was automatically generated. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Fix For: 2.8.0 Attachments: HADOOP-7824.001.patch, HADOOP-7824.002.patch, HADOOP-7824.003.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14644488#comment-14644488 ] Martin Walsh commented on HADOOP-7824: -- So what is the preference then. Leave the patch as is or re-work it to remove the table and set the constants directly in a function as suggested above. As mentioned above, both methods are functionally equivalent. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Fix For: 2.8.0 Attachments: HADOOP-7824.001.patch, HADOOP-7824.002.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14644841#comment-14644841 ] Colin Patrick McCabe commented on HADOOP-7824: -- Hmm. I think it would be easier to read if we set the values directly from a function, if you guys don't have a strong preference. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Fix For: 2.8.0 Attachments: HADOOP-7824.001.patch, HADOOP-7824.002.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14640422#comment-14640422 ] Hadoop QA commented on HADOOP-7824: --- \\ \\ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | pre-patch | 19m 39s | Pre-patch trunk compilation is healthy. | | {color:green}+1{color} | @author | 0m 0s | The patch does not contain any @author tags. | | {color:green}+1{color} | tests included | 0m 0s | The patch appears to include 2 new or modified test files. | | {color:green}+1{color} | javac | 7m 41s | There were no new javac warning messages. | | {color:green}+1{color} | javadoc | 9m 43s | There were no new javadoc warning messages. | | {color:green}+1{color} | release audit | 0m 23s | The applied patch does not increase the total number of release audit warnings. | | {color:red}-1{color} | checkstyle | 2m 1s | The applied patch generated 67 new checkstyle issues (total was 81, now 145). | | {color:green}+1{color} | whitespace | 0m 2s | The patch has no lines that end in whitespace. | | {color:green}+1{color} | install | 1m 22s | mvn install still works. | | {color:green}+1{color} | eclipse:eclipse | 0m 33s | The patch built with eclipse:eclipse. | | {color:green}+1{color} | findbugs | 5m 2s | The patch does not introduce any new Findbugs (version 3.0.0) warnings. | | {color:red}-1{color} | common tests | 22m 2s | Tests failed in hadoop-common. | | {color:green}+1{color} | mapreduce tests | 0m 19s | Tests passed in hadoop-mapreduce-client-shuffle. | | {color:red}-1{color} | hdfs tests | 160m 53s | Tests failed in hadoop-hdfs. | | | | 230m 35s | | \\ \\ || Reason || Tests || | Failed unit tests | hadoop.security.token.delegation.web.TestWebDelegationToken | | | hadoop.hdfs.TestDistributedFileSystem | | | hadoop.hdfs.server.namenode.ha.TestStandbyIsHot | \\ \\ || Subsystem || Report/Notes || | Patch URL | http://issues.apache.org/jira/secure/attachment/12746537/HADOOP-7824.002.patch | | Optional Tests | javadoc javac unit findbugs checkstyle | | git revision | trunk / e202efa | | checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/7335/artifact/patchprocess/diffcheckstylehadoop-common.txt | | hadoop-common test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7335/artifact/patchprocess/testrun_hadoop-common.txt | | hadoop-mapreduce-client-shuffle test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7335/artifact/patchprocess/testrun_hadoop-mapreduce-client-shuffle.txt | | hadoop-hdfs test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7335/artifact/patchprocess/testrun_hadoop-hdfs.txt | | Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/7335/testReport/ | | Java | 1.7.0_55 | | uname | Linux asf900.gq1.ygridcore.net 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 | | Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/7335/console | This message was automatically generated. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Fix For: 2.8.0 Attachments: HADOOP-7824.001.patch, HADOOP-7824.002.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14637465#comment-14637465 ] Colin Patrick McCabe commented on HADOOP-7824: -- Hmm. I don't think there is a lot of cut and paste boilerplate associated with doing it as function calls. The only thing I can think of is the if (failed)... check after each function call. Personally, I prefer seeing that, though, since it makes the control flow clearer to me. And surely using function calls is just as reusable by other Hadoop components as using a table-driven approach? Code being at the top of the file versus the middle of the file seems like an odd argument to make. I don't think it matters where in the file the code is. Mainly, what I don't like about the table-driven approach is that it involves a clunky parallel type system (CT_BOOL, CT_INT, CT_LONG) and a set of somewhat complex macros. I also find it confusing to have a bunch of types that start with const like {{const_type}}, {{const_signature}}, etc but are not {{const}} in the C sense of the word! I think this code may depend on C99 as well... did C89 support { .bool_val=C } and so forth? It seems much simpler to just call a function to do what you want. If you really find it cumbersome to have an if statement after each function call, how about having something like this: {code} jthrowable jthr = NULL; setIntConstant(O_RDONLY, O_RDONLY, jthr); setBooleanConstant(fadvisePossible, HAVE_POSIX_FADVISE, jthr); ... if (jthr) { (*env)-Throw(env, jthr); } {code} And have some code in the functions which just returns and does nothing if jthr is set. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Fix For: 2.8.0 Attachments: HADOOP-7824.001.patch, HADOOP-7824.002.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14637473#comment-14637473 ] Alan Burlison commented on HADOOP-7824: --- I don't have particularly strong preferences either way, it's purely down to personal preference, functionally the two are equivalent. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Fix For: 2.8.0 Attachments: HADOOP-7824.001.patch, HADOOP-7824.002.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14636604#comment-14636604 ] Alan Burlison commented on HADOOP-7824: --- I suggested to Martin that an array of structs would be easiest way of doing this as it avoids lots of cut+paste boilerplate - doing it as explicit code would be 280 lines rather than the 100 or so the current mechanism uses. Plus having it table-driven means it's only a small step to make this code reusable by any Hadoop component that needs to do the same thing. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Attachments: HADOOP-7824.001.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14636594#comment-14636594 ] Martin Walsh commented on HADOOP-7824: -- Thanks for the comments [~cmccabe]. I added the new structures to make it simple to maintain the list of constants being initialised as they would all be in the list at the top of the file. I would argue that modifying that list of consts at the top of the file is simpler and less error prone then modifying the consts_init() function, which should not need to be touched. Additionally changing it as you suggest would bulk up the consts_init() function considerably. However if still prefer me to change it as you suggest, I will do so. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Attachments: HADOOP-7824.001.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14635666#comment-14635666 ] Hadoop QA commented on HADOOP-7824: --- \\ \\ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:red}-1{color} | pre-patch | 21m 22s | Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings. | | {color:green}+1{color} | @author | 0m 0s | The patch does not contain any @author tags. | | {color:green}+1{color} | tests included | 0m 0s | The patch appears to include 2 new or modified test files. | | {color:green}+1{color} | javac | 8m 22s | There were no new javac warning messages. | | {color:green}+1{color} | javadoc | 10m 23s | There were no new javadoc warning messages. | | {color:green}+1{color} | release audit | 0m 22s | The applied patch does not increase the total number of release audit warnings. | | {color:red}-1{color} | checkstyle | 2m 8s | The applied patch generated 67 new checkstyle issues (total was 81, now 145). | | {color:green}+1{color} | whitespace | 0m 2s | The patch has no lines that end in whitespace. | | {color:green}+1{color} | install | 1m 23s | mvn install still works. | | {color:green}+1{color} | eclipse:eclipse | 0m 35s | The patch built with eclipse:eclipse. | | {color:green}+1{color} | findbugs | 5m 30s | The patch does not introduce any new Findbugs (version 3.0.0) warnings. | | {color:green}+1{color} | common tests | 22m 46s | Tests passed in hadoop-common. | | {color:green}+1{color} | mapreduce tests | 0m 19s | Tests passed in hadoop-mapreduce-client-shuffle. | | {color:red}-1{color} | hdfs tests | 161m 5s | Tests failed in hadoop-hdfs. | | | | 235m 16s | | \\ \\ || Reason || Tests || | Failed unit tests | hadoop.hdfs.TestDistributedFileSystem | | | hadoop.hdfs.TestAppendSnapshotTruncate | | | hadoop.hdfs.qjournal.client.TestQuorumJournalManager | \\ \\ || Subsystem || Report/Notes || | Patch URL | http://issues.apache.org/jira/secure/attachment/12746361/HADOOP-7824.001.patch | | Optional Tests | javadoc javac unit findbugs checkstyle | | git revision | trunk / 29cf887b | | Pre-patch Findbugs warnings | https://builds.apache.org/job/PreCommit-HADOOP-Build/7313/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html | | checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/7313/artifact/patchprocess/diffcheckstylehadoop-common.txt | | hadoop-common test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7313/artifact/patchprocess/testrun_hadoop-common.txt | | hadoop-mapreduce-client-shuffle test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7313/artifact/patchprocess/testrun_hadoop-mapreduce-client-shuffle.txt | | hadoop-hdfs test log | https://builds.apache.org/job/PreCommit-HADOOP-Build/7313/artifact/patchprocess/testrun_hadoop-hdfs.txt | | Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/7313/testReport/ | | Java | 1.7.0_55 | | uname | Linux asf907.gq1.ygridcore.net 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 | | Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/7313/console | This message was automatically generated. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Attachments: HADOOP-7824.001.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14635814#comment-14635814 ] Colin Patrick McCabe commented on HADOOP-7824: -- Thanks for posting this, [~martinw]. It looks like a big improvement. Rather than having stuff like this: {code} 64 typedef enum { 65CT_BOOL = 1, 66CT_INT, 67CT_LONG, 68 } const_type; 69 70 struct field { 71char *name; 72const_type type; 73union { 74 jboolean bool_val; 75 jint int_val; 76 jlong long_val; 77} value; 78 }; {code} Why not just have three functions: {code} jthrowable setIntConstant(const char *name, jint val); jthrowable setLongConstant(const char *name, jlong val); jthrowable setBooleanConstant(const char *name, jboolean val); {code} Then you could have: {code} jthr = setIntConstant(O_RDONLY, O_RDONLY); if (jthr != null) { return; } jthr = setBooleanConstant(fadvisePossible, HAVE_POSIX_FADVISE); if (jthr != null) { return; } ... {code} This would be a lot simpler than coming up with all those structures. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Sub-task Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Martin Walsh Labels: hadoop Attachments: HADOOP-7824.001.patch, HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14540533#comment-14540533 ] Colin Patrick McCabe commented on HADOOP-7824: -- Thanks, [~alanburlison]. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Bug Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Todd Lipcon Labels: hadoop Attachments: HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14535591#comment-14535591 ] Colin Patrick McCabe commented on HADOOP-7824: -- Hi [~alanbur], I like this approach. Best of both worlds. Just one comment: would you consider doing {{import static NativeIO.Constants.O_WRONLY}} at the top of these files, to avoid having to type out {{NativeIO.Constants.O_WRONLY}}, etc. later on? {code} 19 #define _GNU_SOURCE {code} Shouldn't be needed, as {{CMakeLists.txt}} will set this. {code} 73if (const_name == NULL) return 0; // JVM throws Exception {code} Should use braces. +1 pending those fixes and pending jenkins Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Bug Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Todd Lipcon Labels: hadoop Attachments: HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14535766#comment-14535766 ] Alan Burlison commented on HADOOP-7824: --- [Switching to new work JIRA account] Sure, I'll submit a patch with the suggested approach above and the extra cleanup you requested. I've just submitted the necessary paperwork to get myself added to Oracle's Apache contributor agreement for Hadoop, that process should take a week or so, after that I'll be able to post a patch. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Bug Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Todd Lipcon Labels: hadoop Attachments: HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14533459#comment-14533459 ] Alan Burlison commented on HADOOP-7824: --- I've prototyped an alternate fix for this, which is to remove 'final' from the public static members used to hold the various O_ constants defined in NativeIO.java but otherwise to leave them alone, then use a JNI_OnLoad hook to update the values of the public static members to the correct platform-specific values. Looking at the classfile disassembly it will be slightly slower than at present - the existing public static final members are inlined to either sipush or ldc of the appropriate constant values whereas if the final modifier is removed (needed to prevent the inlining) then getstatic is used to fetch the value. However it should still be significantly faster than the string-based approach suggested in the latest patch attached to this bug. If this approach is acceptable please let me know and I'll get a patch sorted out. I'm specifically interested in fixing this for Solaris but it should work on all platforms. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Bug Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Todd Lipcon Labels: hadoop Attachments: HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13656741#comment-13656741 ] Matt Foley commented on HADOOP-7824: Changed Target Version to 1.3.0 upon release of 1.2.0. Please change to 1.2.1 if you intend to submit a fix for branch-1.2. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Bug Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Todd Lipcon Labels: hadoop Attachments: HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13462273#comment-13462273 ] Colin Patrick McCabe commented on HADOOP-7824: -- I think we should just do the translation in the JNI code. In general, people expect the jar files to be portable, meaning you can copy them from one PC to another without suffering disaster. This will not be true if we start injecting native constants into the Java code. In contrast, {{libhadoop.so}} is designed to contain the operating-system and architecture-specific parts of the code. Also, do we really want to depend on Perl for our build process? I realize there's already a {{changes2html.pl}} script, but in general Perl seems like another build dependency we'd be better off without. Installing Perl on some platforms will definitely be a hassle-- like Windows. We also don't document our Perl dependency anywhere right now. bq. I suspect that [translating the constants] is going to be a lot slower. I seriously doubt that the performance difference between translating the constants and passing them straight through will even be measurable. If you like, I can do some before and after benchmarks with constant translation and without. Keep in mind, this is a couple of if statements every time we open a file. Considering the other performance issues we have to tackle, this is less than noise-- on par with changing constants because you believe comparisons with 0 are faster, etc. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Bug Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Todd Lipcon Labels: hadoop Attachments: HADOOP-7824.patch, HADOOP-7824.patch, hadoop-7824.txt Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13402763#comment-13402763 ] Todd Lipcon commented on HADOOP-7824: - Instead of generating code at compile time like this, can we do better by creating enums on the Java side, and then translating from the Java enum to the C constants in the JNI layer? See how Errno.java translates to its C counterpart. Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Bug Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0 Environment: Mac OS X, Linux, Solaris, ... Reporter: Dmytro Shteflyuk Assignee: Todd Lipcon Labels: hadoop Attachments: HADOOP-7824.patch, HADOOP-7824.patch Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13402766#comment-13402766 ] Allen Wittenauer commented on HADOOP-7824: -- I suspect that's going to be a lot slower. The C compiler should be able to optimize the heck out of pre-generated code rather than doing it on the fly. Given that the whole reason why this code exists is speed Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Bug Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Todd Lipcon Labels: hadoop Attachments: HADOOP-7824.patch, HADOOP-7824.patch Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7824) Native IO uses wrong constants almost everywhere
[ https://issues.apache.org/jira/browse/HADOOP-7824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13402784#comment-13402784 ] Allen Wittenauer commented on HADOOP-7824: -- (well, that and the fact that Java actively works against you if you want to use OS resources) Native IO uses wrong constants almost everywhere Key: HADOOP-7824 URL: https://issues.apache.org/jira/browse/HADOOP-7824 Project: Hadoop Common Issue Type: Bug Components: native Affects Versions: 0.20.204.0, 0.20.205.0, 1.0.3, 0.23.0, 2.0.0-alpha, 3.0.0 Environment: Mac OS X, Linux, Solaris, Windows, ... Reporter: Dmytro Shteflyuk Assignee: Todd Lipcon Labels: hadoop Attachments: HADOOP-7824.patch, HADOOP-7824.patch Constants like O_CREAT, O_EXCL, etc. have different values on OS X and many other operating systems. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira