[jira] [Comment Edited] (HADOOP-10389) Native RPCv9 client

["Chris Douglas (JIRA)" <ji...@apache.org>]

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

Chris Douglas edited comment on HADOOP-10389 at 6/12/14 8:14 PM:
-----------------------------------------------------------------

[~wheat9], can you be more specific about the code you've found difficult to review and audit, and what could be done to improve its readability? A treatise on buffer overflows and memory leaks will not eliminate them; if Colin were to produce it, how would it help you? Comparing a hypothetical implementation against a partial one by arguing about language features is very indirect. As an example, [~decster]'s questions showed that he had made an effort to understand the code, and Colin's answers provided some insight into the approach. If the code is so impenetrable that review is truly impossible, please consider demonstrating the virtues of C++11 in a competing branch.

[~cmccabe], I looked at the patch attached to this JIRA. Some of it was familiar and/or requires cursory review (makefiles, a queue impl from 1994 \*BSD, vint utility methods), but other files (common/string.*, a splay tree impl from...?, protoc-gen-hrpc.cc) are less obvious, and you didn't explain them until asked. If you can help others follow your progress in the branch, they won't need to ask the "right" questions to understand your approach. This can be in design docs[1], thorough comments[2], and calling out which files are new code (requiring the rigorous audit that [~wheat9] rightly expects), and what has been working for 20+ years. You can solicit better questions.

[1] Is there a design doc for this? I looked on HADOOP-10388, but didn't find one.
[2] For example, one of the few comments in this patch was for {{hrpc_conn_read_alloc}}, which explained esoteric details of the interaction with libuv.


was (Author: chris.douglas):
[~wheat9], can you be more specific about the code you've found difficult to review and audit, and what could be done to improve its readability? A treatise on buffer overflows and memory leaks will not eliminate them; if Colin were to produce it, how would it help you? Comparing a hypothetical implementation against a partial one by arguing about language features is very indirect. As an example, [~decster]'s questions showed that he had made an effort to understand the code, and Colin's answers provided some insight into the approach. If the code is so impenetrable that review is truly impossible, please consider demonstrating the virtues of C++11 in a competing branch.

[~cmccabe], I looked at the patch attached to this JIRA. Some of it was familiar and/or requires cursory review (makefiles, a queue impl from 1994 *BSD, vint utility methods), but other files (common/string.*, a splay tree impl from...?, protoc-gen-hrpc.cc) are less obvious, and you didn't explain them until asked. If you can help others follow your progress in the branch, they won't need to ask the "right" questions to understand your approach. This can be in design docs[1], thorough comments[2], and calling out which files are new code (requiring the rigorous audit that [~wheat9] rightly expects), and what has been working for 20+ years. You can solicit better questions.

[1] Is there a design doc for this? I looked on HADOOP-10388, but didn't find one.
[2] For example, one of the few comments in this patch was for {{hrpc_conn_read_alloc}}, which explained esoteric details of the interaction with libuv.

> Native RPCv9 client
> -------------------
>
>                 Key: HADOOP-10389
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10389
>             Project: Hadoop Common
>          Issue Type: Sub-task
>    Affects Versions: HADOOP-10388
>            Reporter: Binglin Chang
>            Assignee: Colin Patrick McCabe
>         Attachments: HADOOP-10388.001.patch, HADOOP-10389.002.patch, HADOOP-10389.004.patch, HADOOP-10389.005.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.2#6252)