pipelined commands and corking

[Sean Chittenden]

> I just noticed a bug of sorts in memcached:
>
> If you use pipelined commands (does any client?) then the
> corking/uncorking is wrong, and because we never set NODELAY on client
> sockets, the response to the client is Nagel-itized.

libmemcache(3) sets NODELAY on connections, but that's it.  memcache(4) 
is a chatty when busy, but as things stand, I'm not sure how any client 
could handle pipelining correctly until memcache(4) v2 is introduced.  
memcache(3) knows how many keys it sends, but for memcache(3) to work 
correctly on responses, the server would have to send a memcache(4) 
equiv of a 404 for a key that isn't found.

> Simple hack:
>
> conn *conn_new(int sfd, int init_state, int event_flags) {
>     conn *c;
>     int flags = 1;
>
>     /* set NODELAY on all new connections.  there's a bug of sorts in
>        the corking/uncorking of pipelined commands, so this makes sure
>        commands that are outside a cork/uncork block will send 
> immediately */
>
>     setsockopt(sfd, IPPROTO_TCP, TCP_NODELAY, &flags, sizeof(flags));
>
> ....

This is what memcache(3) has done this from day one, but I'm not doing 
any asynchronous sending/reading of responses.

> If anybody (Sean? Avva?) wants to find the proper fix, let me know.
> Or if we should just include the above.

There is no way to do this properly other than the above fix, which 
memcache(3) already has in place.  :)  A *better* fix, however, is 
platform specific, nifty, being phased out of the TCP stack (but being 
replaced by SCTP) and involves ttcp(4) (T/TCP).  I'm half a mind to 
detect rfc1644 extensions and conditionally use sendto(2) to enable 
client side T/TCP support (instead of writev(2)), which would nuke any 
network latency from the stack or tcp(4).  That said, the only OS that 
does have T/TCP support is FreeBSD.  Waiting for SCTP to become 
standardized and widely deployed is probably a better solution, 
however.  *shrug*  Can't win them all.

> Sean, if you're going to proceed with the binary protocol and making
> get_multi go away (to be replaced with pipelined gets), then the above
> is of interest.  (if clients don't advertise their pipeline depth)

 From cvs annotate:

1.1          (sean     07-Dec-04): #ifdef TCP_NODELAY
1.1          (sean     07-Dec-04):     val = 1;
1.1          (sean     07-Dec-04):     if (setsockopt(ms->fd, 
IPPROTO_TCP, TCP_NODELAY, &val, (socklen_t)sizeof(val)) != 0) {
1.1          (sean     07-Dec-04):       
warn("%s:%u\tsetsockopt(TCP_NODELAY)", __FILE__, __LINE__);
1.1          (sean     07-Dec-04):     }
1.1          (sean     07-Dec-04): #endif

:)  Like I said, memcache(3) has been doing it since day one.  -sc

-- 
Sean Chittenden