Large multiget requests randomly broken

Bug #637114 reported by Carlos
26
This bug affects 4 people
Affects Status Importance Assigned to Milestone
memcached (Ubuntu)
Fix Released
High
Unassigned
Lucid
Fix Released
High
Unassigned

Bug Description

== 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.

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

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.

Changed in memcached (Ubuntu):
status: New → Confirmed
importance: Undecided → High
status: Confirmed → Fix Released
Changed in memcached (Ubuntu Lucid):
status: New → Confirmed
importance: Undecided → High
status: Confirmed → Triaged
Revision history for this message
Michael Koziarski (michael-koziarski) wrote :

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.

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Uploaded fix to lucid-proposed.

Changed in memcached (Ubuntu Lucid):
status: Triaged → Fix Committed
description: updated
Revision history for this message
Martin Pitt (pitti) wrote : Please test proposed package

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!

tags: added: verification-needed
Revision history for this message
James Page (james-page) wrote :

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.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in memcached (Ubuntu Lucid):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.