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 <[email protected]> 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 >> >
