hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Haohui Mai (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-10389) Native RPCv9 client
Date Tue, 17 Jun 2014 23:05:13 GMT

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

Haohui Mai commented on HADOOP-10389:

bq.  didn't integrate the existing JNIFS code, are not parsing the config XMLs, don't handle
errors, etc. etc.

Correct me if I'm wrong -- based on my understanding the original patch in HADOOP-10369 implements
none of them above. Both implementation can benefit from other jiras (e.g. HADOOP-10446).

bq. ..., because you didn't implement generation of type-safe RPC wrappers (you just wrote
out everything manually in HdfsImpl::Exists),

This is implemented as C++ code in the original patch. It can be brought in if it is required.

For what is worth, do you agree that the code implements comparable functionality with much
smaller code base?

bq. boost::asio doesn't provide equivalent functionality to libuv. It does wrap a lot of network
stuff, but doesn't handle things like condition variables, signals, etc. etc., ..., For example,
the boost condition variable actually can let your thread wake up without recovering the lock
(it then has to sleep again). 

I'm unsure why this is relevant as I don't see the current code uses condition variables and
signals. My guess is that you might need them to implement RPC with timeout. I'm yet to be
convinced that this is required in the c++ implementation, as {{std::future}} provides a much
better abstraction for that.

bq. It uses exceptions, which are problematic. 

This is not a fundamental part of the API. The current implementation uses the exception-free
variants of the library calls whenever possible. The {{std::system_error}} object carries
the error conditions.

The current implementation of the rpc-send path, however, might throw {{bad_alloc}}. It can
be fixed by limiting the maximum size of the outgoing buffer.

I say it is safe to define {{BOOOST_NO_EXCEPTIONS}} and compile the code with {{-fno-exceptions}}.

Yes, you can use boost for some of that stuff, but the boost versions are often not that good.

Boost in general has a versioning problem which is really bad for libraries. If your library
links against version X of boost, any application linking to you needs to use the same version...
otherwise bad stuff happens. You could link it statically, but it might be too big for that
to work (I've never tried).

bq. The only thing required is {{boost::system::error}}, which have a fairly small foolprints
(~70K on Mac OS X). The size is comparable to the size of {{protobuf-c}} (~75K on Mac OS X).

bq. To me this is not that interesting. There are a lot of clients already out there in various
languages... C, C++, even golang. 

In my opinion the same argument applies to any clients, none of them is superior to the others.
Everybody can write their own clients. Contributions are welcome, but for a community takes
over the code and to maintain it, it is important to ensure that the code can be easily changed,
reviewed, and maintained by other contributors.

This key point, however, remains unaddressed in the the current branch.

bq. If you're interested in something even smaller and more minimal, you can check out this:
https://github.com/toddlipcon/libhrpc/ Though I think that that is for an out-of-date version
of hadoop RPC at this point.

Given the fact that many hadoop RPC clients happily sit on github, and the protocol remains
pretty stable, is it more appropriate to host this project on github if you don't have time
to address the comments?

> 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-alternative.000.patch, HADOOP-10389.002.patch,
HADOOP-10389.004.patch, HADOOP-10389.005.patch

This message was sent by Atlassian JIRA

View raw message