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