CAS is broken
Chris Goffinet
goffinet at yahoo-inc.com
Mon Nov 12 06:46:23 UTC 2007
The mutex part is handled right now using the threads code, every item
allocation has a mutex around it, so this should be safe by calling
inside the item_allocation.
Right now every item does have its own cas id. If you propose the
following scenario:
set foo -> id 0
gets foo -> id 0
delete foo
set foo -> id 0
cas foo -> sent using 0
That would succeed when it should of failed. The second set should of
caused it to be 1.
Chris Goffinet
goffinet at yahoo-inc.com
On Nov 11, 2007, at 10:39 PM, Tomash Brechko wrote:
> On Sun, Nov 11, 2007 at 14:37:07 -0800, Dustin Sallings wrote:
>> Can you take a look at this and see if it makes any more sense?
>
>
>> diff --git a/items.c b/items.c
>> --- a/items.c
>> +++ b/items.c
>> @@ -37,6 +38,15 @@ void item_init(void) {
>> tails[i] = NULL;
>> sizes[i] = 0;
>> }
>> +}
>> +
>> +/* Get the next CAS id for a new item. */
>> +uint64_t get_cas_id() {
>> + static uint64_t cas_id;
>> + if(cas_id >= MAX_CAS_ID) {
>> + cas_id = 0;
>> + }
>> + return ++cas_id;
>> }
>
> Seems like you want to have a global CAS counter. This would require
> you to have a mutex on it. Wouldn't it be better to have a CAS local
> to each item? Becase all we need is an unique (key,CAS) pair, we
> don't have to have each CAS to be unique by itself. Besides, this
> will decrease the chance of wraparound to effectively zero.
>
>
>> @@ -129,6 +139,7 @@ item *do_item_alloc(char *key, const siz
>> it->it_flags = 0;
>> it->nkey = nkey;
>> it->nbytes = nbytes;
>> + it->cas_id = get_cas_id();
>
> Then this would be it->cas_id = 0;, 'gets' will return current value,
> and 'cas' will do ++it->cas_id; (no need to have MAX_CAS_ID, C
> guarantees no error on wraparound of unsigned types, not to say that
> it will never happen).
>
>
> --
> Tomash Brechko
More information about the memcached
mailing list