thrift-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bruce Lowekamp <bruce.lowek...@skype.net>
Subject Re: erlang server/client closing connections
Date Mon, 16 Aug 2010 20:56:50 GMT
Hadn't found this before---we have some similar code to improve
performance of the erlang thrift library, but I think this is a bit
more thorough.  Would be nice to see this in trunk.

Bruce


On Fri, Aug 13, 2010 at 2:56 PM, David Reiss <dreiss@facebook.com> wrote:
>> I like that it has -spec's now, does it cleanly pass the dialyzer?
> I don't remember.  Based on my incremental commit messages (which you can
> see at that gitweb link), it looks like I was running dializer while I was
> working on this.
>
>> Also, I know with the current version there are lots of
>> warnings about unused variables, it might be worthwhile to prepend _ to
>> them, so it builds without warning
> Yeah, I think I got those, too.
>
>> The one thing that I'm still curious about is why the client doesn't receive
>> some sort of notification that the server has shutdown.  It must be that
>> gen_tcp doesn't let the client know?
> Not sure.  In C, you don't get any notification until you try to read or write
> (or poll for readability or writability).
>
> --David
>
> On 08/13/2010 02:51 PM, Anthony Molinaro wrote:
>>
>> On Fri, Aug 13, 2010 at 11:49:13AM -0700, David Reiss wrote:
>>> I did a major refactor of the Erlang library that I think might resolve this
>>> issue.  https://issues.apache.org/jira/browse/THRIFT-599  With my patch,
>>> thrift_buffered_transport is no longer a separate process, so there is no
>>> need for a gen_server call.  This patch hasn't been committed yet because
>>> at the time I posted it, Facebook hadn't deployed it in production anywhere.
>>> We have now, though, so if people want, I can check it in.
>>
>> I would not be opposed, I took a quick look through the client code and it
>> seems a lot more streamlined, it sort of leaves it up to the client to
>> spawn a process to handle a client which seems fine.  It probably will help
>> with this case because it honor's recv_timeout in the server, so if I set
>> it to something high it will wait that long.
>>
>> So +1 from me for committing.  I like that it has -spec's now, does it cleanly
>> pass the dialyzer?  Also, I know with the current version there are lots of
>> warnings about unused variables, it might be worthwhile to prepend _ to
>> them, so it builds without warning (this only happens when you use erlc directly
>> the makefiles hide warnings at the moment).
>>
>> The one thing that I'm still curious about is why the client doesn't receive
>> some sort of notification that the server has shutdown.  It must be that
>> gen_tcp doesn't let the client know?
>>
>> Again, patch looks good to me, I'll try it out soon.
>>
>> -Anthony
>>
>

Mime
View raw message