Per the previous discussion about counter wraparounds, some people
thought that a 64-bit counter could sidestep some of the pain of
adapting client code to deal with a wraparound. It's possible 64-bit
counters could introduce as many problems as they solve; I really don't
know the client libraries well enough to say (let alone everyone's
database schemas, etc.). Regardless, I threw together a patch to support
increments up through 2**64. It is attached.
I'm actually indifferent to whether counters are 32 or 64 bits, but
perhaps someone else can assess the breakage that moving to 64 bits will
wreak. All I know is that wraparound is probably the "correct" behavior,
and I'd like to see one patch or the other integrated. Thoughts?
Evan
Index: memcached.c
===================================================================
--- memcached.c (revision 606)
+++ memcached.c (working copy)
@@ -1201,9 +1201,9 @@
}
static void process_arithmetic_command(conn *c, token_t *tokens, const size_t ntokens, const int incr) {
- char temp[32];
+ char temp[sizeof("18446744073709551615")];
item *it;
- unsigned int delta;
+ uint64_t delta;
char *key;
size_t nkey;
@@ -1230,7 +1230,7 @@
}
}
- delta = strtoul(tokens[2].value, NULL, 10);
+ delta = strtoull(tokens[2].value, NULL, 10);
if(errno == ERANGE) {
out_string(c, "CLIENT_ERROR bad command line format");
@@ -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 uint64_t delta, char *buf) {
char *ptr;
- unsigned int value;
+ uint64_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 = strtoull(ptr, NULL, 10);
if(errno == ERANGE) {
return "CLIENT_ERROR cannot increment or decrement non-numeric value";
@@ -1277,7 +1277,7 @@
if (delta >= value) value = 0;
else value -= delta;
}
- snprintf(buf, 32, "%u", value);
+ sprintf(buf, "%llu", value);
res = strlen(buf);
if (res + 2 > it->nbytes) { /* need to realloc */
item *new_it;
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 uint64_t delta, char *buf) {
char *ret;
pthread_mutex_lock(&cache_lock);
Index: memcached.h
===================================================================
--- memcached.h (revision 606)
+++ memcached.h (working copy)
@@ -222,7 +222,7 @@
bool do_conn_add_to_freelist(conn *c);
char *do_defer_delete(item *item, time_t exptime);
void do_run_deferred_deletes(void);
-char *do_add_delta(item *item, int incr, const unsigned int delta, char *buf);
+char *do_add_delta(item *item, int incr, const uint64_t delta, char *buf);
int do_store_item(item *item, int comm);
conn *conn_new(const int sfd, const int init_state, const int event_flags, const int read_buffer_size, const bool is_udp, struct event_base *base);
@@ -253,7 +253,7 @@
void dispatch_conn_new(int sfd, int init_state, int event_flags, int read_buffer_size, int is_udp);
/* Lock wrappers for cache functions that are called from main loop. */
-char *mt_add_delta(item *item, const int incr, const unsigned int delta, char *buf);
+char *mt_add_delta(item *item, const int incr, const uint64_t delta, char *buf);
void mt_assoc_move_next_bucket(void);
conn *mt_conn_from_freelist(void);
bool mt_conn_add_to_freelist(conn *c);
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 => 17;
use FindBin qw($Bin);
use lib "$Bin/lib";
use MemcachedTest;
@@ -30,6 +30,18 @@
print $sock "decr num 5\r\n";
is(scalar <$sock>, "0\r\n", "- 5 = 0");
+printf $sock "set num 0 0 10\r\n4294967296\r\n";
+is(scalar <$sock>, "STORED\r\n", "stored 2**32");
+
+print $sock "incr num 1\r\n";
+is(scalar <$sock>, "4294967297\r\n", "4294967296 + 1 = 4294967297");
+
+printf $sock "set num 0 0 %d\r\n18446744073709551615\r\n", length("18446744073709551615");
+is(scalar <$sock>, "STORED\r\n", "stored 2**64-1");
+
+print $sock "incr num 1\r\n";
+is(scalar <$sock>, "0\r\n", "(2**64 - 1) + 1 = 0");
+
print $sock "decr bogus 5\r\n";
is(scalar <$sock>, "NOT_FOUND\r\n", "can't decr bogus key");
Index: doc/protocol.txt
===================================================================
--- doc/protocol.txt (revision 606)
+++ doc/protocol.txt (working copy)
@@ -252,7 +252,7 @@
Commands "incr" and "decr" are used to change data for some item
in-place, incrementing or decrementing it. The data for the item is
-treated as decimal representation of a 32-bit unsigned integer. If the
+treated as decimal representation of a 64-bit unsigned integer. If the
current data value does not conform to such a representation, the
commands behave as if the value were 0. Also, the item must already
exist for incr/decr to work; these commands won't pretend that a
@@ -269,7 +269,7 @@
- <key> is the key of the item the client wishes to change
- <value> is the amount by which the client wants to increase/decrease
-the item. It is a decimal representation of a 32-bit unsigned integer.
+the item. It is a decimal representation of a 64-bit unsigned integer.
The response will be one of:
@@ -280,7 +280,7 @@
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.
+the "incr" command will wrap around the 64 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
