You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by Christopher Friedt <ch...@gmail.com> on 2016/08/17 19:30:45 UTC
UDP Transport
Hi folks,
I'm working on a couple of transports on my GitHub fork of Thrift [1].
There is a UDP Transport ( TUdpSocket, TUdpServerSocket )
implementation in "feature/udp-sockets" [2].
It's still in development, but Works For Me™, so far. For a quick
comparison, see [3].
Some notes:
* Rather than lumping functionality in the base class (TSocket), I
opted to not modify that, and instead added new functionality in
TUdpSocket. This duplicates some code, but it's probably better to do
that than to introduce unnecessary complexity in a base class.
* Rather than using TSocket::getSocketInfo(), I added my own function
to stringify socket addresses that seems more appropriate according to
RFC3986. My function is called TUdpSocket::sockaddrToString().
* Remnants of UDP messages tend to be discarded if the entire message
is not read in one go (at least on my machine). Therefore, I buffer
the entire UDP message when it is first read. This causes some
inconsistencies with the rest of Thrift which expect to be able to
read fragments of a message a few bytes at a time. Poll(2) would fail
on subsequent TSocket::read()s), which is why I shadowed it in
TUdpSocket::read() in such an unusual way.
* I factored-out TGetAddrInfoWrapper into it's own .cpp / .h files, for reuse.
* I factored-out cast_sockopt into it's own header file, for reuse.
* Probably some unnecessary modifications ( private -> protected
fields ) in TServerSocket.h
* Made recv() and send() virtual in TSocket
If anyone is interested in testing, feedback is welcome. Please put my
email in the CC, if you do. I'd be happy to work on getting this code
upstreamed when things are a bit less busy.
Thanks,
C
[1] https://github.com/cfriedt/thrift
[2] https://github.com/cfriedt/thrift/tree/feature/udp-sockets
[3] https://goo.gl/qjCSni
Re: UDP Transport
Posted by Christopher Friedt <ch...@gmail.com>.
Hi Randy,
On Wed, Aug 17, 2016 at 4:01 PM, Randy Abernethy
<ra...@gmail.com> wrote:
> Would be nice to have a UDP Transport! Have you wired it into the cross
> tests? That IDL is sort of the benchmark for interop higher up the stack
I haven't yet wired it into the cross tests, or done anything aside
from a fairly simple hello test (see below). Actually, when I run
"make check" on the master branch in thrift, most of the tests fail
anyway, which is a bit worrisome.
Please feel free to test out the code and report any hiccups to the
ML. Actually, if anyone would like to further integrate UDP tests into
the testing code in thrift, that would be much appreciated.
Thanks!
C
// https://willwarren.com/2012/01/24/creating-a-public-api-with-apache-thrift/
namespace cpp hello
enum HelloLanguage {
ENGLISH,
FRENCH,
SPANISH
}
service Hello {
string say_hello(),
string say_foreign_hello(1: HelloLanguage language),
list<string> say_hello_repeat(1: i32 times),
}
My test class (actually I'm using gtest) has the following two
methods, which should be helpful. Note, that if running unit tests,
since the UDP socket is not connected, the client port and server port
should be different. However, in practise, on separate machines, they
would typically be the same for a P2P topology.
boost::shared_ptr<TSimpleServer> HelloUdpSocketBinaryTest::makeServer() {
return boost::make_shared<TSimpleServer>(
boost::make_shared<HelloProcessor>(boost::make_shared<HelloHandler>()),
boost::shared_ptr<TServerSocket>( (TServerSocket *)
new TUdpServerSocket( server_port, client_port ) ),
boost::make_shared<TTransportFactory>(),
boost::make_shared<TBinaryProtocolFactory>()
);
}
boost::shared_ptr<TTransport> HelloUdpSocketBinaryTest::makeTransport() {
return boost::make_shared<TBufferedTransport>(
boost::shared_ptr<TSocket>( new TUdpSocket( host,
server_port, client_port ) )
);
}
Re: UDP Transport
Posted by Randy Abernethy <ra...@gmail.com>.
Would be nice to have a UDP Transport! Have you wired it into the cross
tests? That IDL is sort of the benchmark for interop higher up the stack
--Randy.
On Wed, Aug 17, 2016 at 12:30 PM, Christopher Friedt <ch...@gmail.com>
wrote:
> Hi folks,
>
> I'm working on a couple of transports on my GitHub fork of Thrift [1].
>
> There is a UDP Transport ( TUdpSocket, TUdpServerSocket )
> implementation in "feature/udp-sockets" [2].
>
> It's still in development, but Works For Me™, so far. For a quick
> comparison, see [3].
>
> Some notes:
>
> * Rather than lumping functionality in the base class (TSocket), I
> opted to not modify that, and instead added new functionality in
> TUdpSocket. This duplicates some code, but it's probably better to do
> that than to introduce unnecessary complexity in a base class.
> * Rather than using TSocket::getSocketInfo(), I added my own function
> to stringify socket addresses that seems more appropriate according to
> RFC3986. My function is called TUdpSocket::sockaddrToString().
> * Remnants of UDP messages tend to be discarded if the entire message
> is not read in one go (at least on my machine). Therefore, I buffer
> the entire UDP message when it is first read. This causes some
> inconsistencies with the rest of Thrift which expect to be able to
> read fragments of a message a few bytes at a time. Poll(2) would fail
> on subsequent TSocket::read()s), which is why I shadowed it in
> TUdpSocket::read() in such an unusual way.
> * I factored-out TGetAddrInfoWrapper into it's own .cpp / .h files, for
> reuse.
> * I factored-out cast_sockopt into it's own header file, for reuse.
> * Probably some unnecessary modifications ( private -> protected
> fields ) in TServerSocket.h
> * Made recv() and send() virtual in TSocket
>
> If anyone is interested in testing, feedback is welcome. Please put my
> email in the CC, if you do. I'd be happy to work on getting this code
> upstreamed when things are a bit less busy.
>
> Thanks,
>
> C
>
> [1] https://github.com/cfriedt/thrift
> [2] https://github.com/cfriedt/thrift/tree/feature/udp-sockets
> [3] https://goo.gl/qjCSni
>
Re: UDP Transport
Posted by Jake Farrell <jf...@apache.org>.
Hey Christopher
Thanks for sharing
-Jake
On Wed, Aug 17, 2016 at 3:30 PM, Christopher Friedt <ch...@gmail.com>
wrote:
> Hi folks,
>
> I'm working on a couple of transports on my GitHub fork of Thrift [1].
>
> There is a UDP Transport ( TUdpSocket, TUdpServerSocket )
> implementation in "feature/udp-sockets" [2].
>
> It's still in development, but Works For Me™, so far. For a quick
> comparison, see [3].
>
> Some notes:
>
> * Rather than lumping functionality in the base class (TSocket), I
> opted to not modify that, and instead added new functionality in
> TUdpSocket. This duplicates some code, but it's probably better to do
> that than to introduce unnecessary complexity in a base class.
> * Rather than using TSocket::getSocketInfo(), I added my own function
> to stringify socket addresses that seems more appropriate according to
> RFC3986. My function is called TUdpSocket::sockaddrToString().
> * Remnants of UDP messages tend to be discarded if the entire message
> is not read in one go (at least on my machine). Therefore, I buffer
> the entire UDP message when it is first read. This causes some
> inconsistencies with the rest of Thrift which expect to be able to
> read fragments of a message a few bytes at a time. Poll(2) would fail
> on subsequent TSocket::read()s), which is why I shadowed it in
> TUdpSocket::read() in such an unusual way.
> * I factored-out TGetAddrInfoWrapper into it's own .cpp / .h files, for
> reuse.
> * I factored-out cast_sockopt into it's own header file, for reuse.
> * Probably some unnecessary modifications ( private -> protected
> fields ) in TServerSocket.h
> * Made recv() and send() virtual in TSocket
>
> If anyone is interested in testing, feedback is welcome. Please put my
> email in the CC, if you do. I'd be happy to work on getting this code
> upstreamed when things are a bit less busy.
>
> Thanks,
>
> C
>
> [1] https://github.com/cfriedt/thrift
> [2] https://github.com/cfriedt/thrift/tree/feature/udp-sockets
> [3] https://goo.gl/qjCSni
>