Currently Memcached gives an error when incrementing large counters:
set foobar 0 0 10
2147483647
STORED
incr foobar 1
2147483648
incr foobar 1
CLIENT_ERROR cannot increment or decrement non-numeric value
I believe this is a bug. Attached is a patch to make counters wrap
around the 2**32 mark, similar to counters in routers. New behavior:
set foobar 0 0 10
4294967294
STORED
incr foobar 1
4294967295
incr foobar 1
0
A CLIENT_ERROR will continue to be returned if a value above 2**32 is
incremented, but the "incr" command will never push the value over that
mark.
A future version of Memcached should probably use a 64-bit counter
instead, but the 32-bit limit is in line with the existing docs. ("The
data for the item is treated as decimal representation of a 32-bit
unsigned integer.")
The patch also adds "const" keywords to arguments of do_add_delta and
mt_add_delta, to be consistent with the header file.
Documentation and tests have been updated.
Evan Miller
IMVU, Inc.
Index: memcached.c
===================================================================
--- memcached.c (revision 606)
+++ memcached.c (working copy)
@@ -1257,15 +1257,15 @@
*
* returns a response string to send back to the client.
*/
-char *do_add_delta(item *it, const int incr, unsigned int delta, char *buf) {
+char *do_add_delta(item *it, const int incr, const unsigned int delta, char *buf) {
char *ptr;
- unsigned int value;
+ uint32_t value;
int res;
ptr = ITEM_data(it);
while ((*ptr != '\0') && (*ptr < '0' && *ptr > '9')) ptr++; // BUG: can't be true
- value = strtol(ptr, NULL, 10);
+ value = strtoul(ptr, NULL, 10);
if(errno == ERANGE) {
return "CLIENT_ERROR cannot increment or decrement non-numeric value";
Index: thread.c
===================================================================
--- thread.c (revision 606)
+++ thread.c (working copy)
@@ -462,7 +462,7 @@
/*
* Does arithmetic on a numeric item value.
*/
-char *mt_add_delta(item *item, int incr, unsigned int delta, char *buf) {
+char *mt_add_delta(item *item, int incr, const unsigned int delta, char *buf) {
char *ret;
pthread_mutex_lock(&cache_lock);
Index: t/incrdecr.t
===================================================================
--- t/incrdecr.t (revision 606)
+++ t/incrdecr.t (working copy)
@@ -1,7 +1,7 @@
#!/usr/bin/perl
use strict;
-use Test::More tests => 13;
+use Test::More tests => 16;
use FindBin qw($Bin);
use lib "$Bin/lib";
use MemcachedTest;
@@ -30,6 +30,15 @@
print $sock "decr num 5\r\n";
is(scalar <$sock>, "0\r\n", "- 5 = 0");
+print $sock "incr num ".(2**32-2)."\r\n";
+is(scalar <$sock>, (2**32-2)."\r\n", "+ ".(2**32-2)." = ".(2**32-2));
+
+print $sock "incr num 1\r\n";
+is(scalar <$sock>, (2**32-1)."\r\n", "+ 1 = ".(2**32-1));
+
+print $sock "incr num 1\r\n";
+is(scalar <$sock>, "0\r\n", "+ 1 = 0");
+
print $sock "decr bogus 5\r\n";
is(scalar <$sock>, "NOT_FOUND\r\n", "can't decr bogus key");
@@ -40,5 +49,3 @@
is(scalar <$sock>, "STORED\r\n", "stored text");
print $sock "incr text 1\r\n";
is(scalar <$sock>, "1\r\n", "hi - 1 = 0");
-
-
Index: doc/protocol.txt
===================================================================
--- doc/protocol.txt (revision 606)
+++ doc/protocol.txt (working copy)
@@ -279,8 +279,8 @@
after the increment/decrement operation was carried out.
Note that underflow in the "decr" command is caught: if a client tries
-to decrease the value below 0, the new value will be 0. Overflow in
-the "incr" command is not checked.
+to decrease the value below 0, the new value will be 0. Overflow in the
+"incr" command will wrap around the 32 bit mark.
Note also that decrementing a number such that it loses length isn't
guaranteed to decrement its returned length. The number MAY be
