hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Joseph Evans (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-6311) Add support for unix domain sockets to JNI libs
Date Tue, 02 Aug 2011 21:08:27 GMT

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

Robert Joseph Evans commented on HADOOP-6311:
---------------------------------------------

For what it is worth I thought it might be good to review the patch as a second opinion on
its quality and maintainability.  I did not really look at any of the Makefile/Configure changes
as they are mostly auto generated.  The following are the issues that I came up with.

LocalSocket.java remove methods with UnsupportedOperationException unless we plan on implementing
them.

LocalSocketImpl.java may be very slow for large byte array reads and writes.  I'm not sure
how likely this is but if the array is large enough the java splits it into multiple locations
then it might copy the data multiple times to comply with the JNI interface.  It is not a
correctness issues, just potentially a performance issue.

LocalSocketImpl.java Potential race condition between reads/writes and closes (there is inconsistent
locking with fd).

LocalSocketImpl.java remove supportsUrgentData and sendUrgentData unless we plan on implementing
them in the future, similar with getSockAddress()

LocalSocketImpl.java in getOption I understand having a pattern in place to be able to return
different types, but it is always int so can we get rid of the switch or combine the returns
together.

JNIHelp.cpp does not have an apache license in it.

org_apache_hadoop_net_unix_LocalSocketImpl.cpp in make_sockaddr_un and java_opt_to_real I
don't really like it writing to STDERR, it would be better to try and throw an exception instead.

org_apache_hadoop_net_unix_LocalSocketImpl.cpp in Java_org_apache_hadoop_net_unix_LocalSocketImpl_connectLocal
if an Exception occurred when calling jniGetFDFromFileDescriptor then nameUtf8 appears to
be leaked.

org_apache_hadoop_net_unix_LocalSocketImpl.cpp in Java_org_apache_hadoop_net_unix_LocalSocketImpl_bindLocal
unlink is called regardless of namespace, and all errors with it are ignored.

org_apache_hadoop_net_unix_LocalSocketImpl.cpp in JNI_OnLoad still has a reference to android
in an commented out error log/

In the test please address the TODO you left in there about being a timeout exception instead
of an IOException






> Add support for unix domain sockets to JNI libs
> -----------------------------------------------
>
>                 Key: HADOOP-6311
>                 URL: https://issues.apache.org/jira/browse/HADOOP-6311
>             Project: Hadoop Common
>          Issue Type: New Feature
>          Components: native
>    Affects Versions: 0.20.0
>            Reporter: Todd Lipcon
>            Assignee: Todd Lipcon
>         Attachments: 6311-trunk-inprogress.txt, hadoop-6311.txt
>
>
> For HDFS-347 we need to use unix domain sockets. This JIRA is to include a library in
common which adds a o.a.h.net.unix package based on the code from Android (apache 2 license)

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message