Large multiget requests randomly broken

[Carlos]

== SRU REPORT ==

IMPACT: users who try to use memcached with large multi-gets will get a cache miss every time.

TEST CASE:

* install php5-cli and php5-memcached
* run this code using 'php':

$m = new Memcached();
$m->addServer('localhost', 11211);
$keys = array();
for ($i=0; $i<50000; $i++) {
  $key = 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::foo::' . $i;
  $keys[] = $key;
}
$result = $m->getMulti($keys, $cas);
var_dump($result, $cas);
var_dump($m->getResultCode());

* Result if affected by bug will be:
bool(false)
NULL
int(19)

* Result if fixed will be:

array(0) {
}
array(0) {
}
int(0)

REGRESSION POTENTIAL: This change makes the patch *more* like the upstream fix, which has been in every version of memcached since Ubuntu 10.10, and has not caused regressions.

======

Binary package hint: memcached

* Ubuntu release.

  Description: Ubuntu 10.04 LTS
  Release: 10.04

* How to reproduce the bug?

Send several large multiget requests to a Ubuntu 1.4.2 Memcached server, and you'll notice that some of them will break, and other will return only a subset of the requested keys. You can use the following PHP test code:

$m = new Memcached();
$m->addServer('localhost', 11211);
$keys = array();
for ($i=0; $i<50000; $i++) {
  $key = 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::foo::' . $i;
  $keys[] = $key;
}
$result = $m->getMulti($keys, $cas);
var_dump($result, $cas);
var_dump($m->getResultCode());

* How to fix de bug?

Rebuilding the server without the fix-issue-102-segfault & fix-ubuntu-ftbfs patches fixes that bug for us.

[Clint Byrum (clint-fewbar)]

Carlos, thanks for taking the time to file this bug report.

I was able to confirm this on lucid only. It is working fine in Maverick and Natty.

Since its hard to know that this is affecting you, and memcached can potentially be used as a sort of data store, I'm marking it as High Importance.

[Michael Koziarski (michael-koziarski)]

The cause of this is a broken back port of the upstream change set:

https://github.com/memcached/memcached/commit/d9cd01ede97f4145af9781d448c62a3318952719

That changeset uses strncmp whereas the .diff file uses strcmp

++ if (ptr - c->rcurr > 100 ||
++ strcmp(ptr, "get ") && strcmp(ptr, "gets ")) {
++ conn_set_state(c, conn_closing);
++ return 1;
++ }

Given that the string being compared contains either "get HUGE KEY" or "gets HUGE KEYS" the check in the ubuntu diff will *always* return false.

Simply correcting that back port appears to solve the problem for us.

[Clint Byrum (clint-fewbar)]

Uploaded fix to lucid-proposed.

[Martin Pitt (pitti)]

Hello Carlos, or anyone else affected,

Accepted memcached into lucid-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[James Page (james-page)]

Hi

With the package in -proposed I got consistent results when executing the test script for reproducing the issue:

array(0) {
}
array(0) {
}
int(0)

I was able to confirm the issue prior to testing as well.

Thanks.

[Launchpad Janitor (janitor)]

This bug was fixed in the package memcached - 1.4.2-1ubuntu4

---------------
memcached (1.4.2-1ubuntu4) lucid-proposed; urgency=low

  * debian/patches/fix-issue-102-segfault.patch: use strncmp
    the way the upstream code does. Prevents multigets from
    failing. (LP: #637114)
  * debian/patchex/fix-ubuntu-ftbfs.patch: dropped
 -- Clint Byrum <email address hidden> Tue, 29 Nov 2011 22:57:21 -0800