no reply patch

Tomash Brechko tomash.brechko at gmail.com
Tue Feb 19 09:40:35 UTC 2008


On Tue, Feb 19, 2008 at 00:44:26 -0800, dormando wrote:
> For sanity I'm going to have to change it to look like this:
> 
>     if (c->noreply) {
>         if (settings.verbose > 1)
>             fprintf(stderr, ">%d NOREPLY %s\n", c->sfd, str);
>         c->noreply = false;
>         conn_set_state(c, conn_read);
>         return;
>     }

Ok, no objections.


> ...and for both of these, the test should be 'if unlikely(c->noreply)' -

It hardly would make any difference, we are not in a tight loop.  You
could also write unlikely(settings.verbose > 1), but I'd say one
should save such optimizations to places there they do make noticeable
speedup.  OTOH, I guess GCC's __builtin_expect() may suggest the
compiler to reverse the order of branches.


> This also needs some more careful documentation warning about the
> occasional bad line potentially throwing clients out of whack. Since
> this is my pet peeve I'll add it myself.

As was mentioned before, the problem is not limited to 'noreply' only.
Send to memcached "SET KEY\r\nVALUE\r\n" (in upper case), and enjoy
two replies instead of one.  With 'norepy' it's just one reply instead
of zero, same problem.  And this can only happen for malformed
command, where error reply is send before the command has been parsed
(memcached couldn't parse it).


-- 
   Tomash Brechko


More information about the memcached mailing list