Problem of reclaiming valid items.

[GoogleCodeExporter]

What steps will reproduce the problem?
1.just make a small-sized(with -m <num> and -M option) memcached with full of 
items of exptime=0.
2. and, do send flush command with delay(86400): "flush_all 86400\r\n" 
3. then, add new items into the memcached. The tail item of LRU will be 
reclaimed even if the item is still valid.

What is the expected output? What do you see instead?

Expected result is "out of memory". Because there is no available memory and no 
reclaimable items.

But, The tail item of LRU will be reclaimed even if the item is still valid.
It's a bug I think.

What version of the product are you using? On what operating system?

1.4.11 and below versions. 

Please provide any additional information below.

The functionality of reclaiming flushed items are added memcached by fixing 
issue 183.
But, the fix was not correct. It bring an another bug explained by this issue.

The needed code fix for this bug like followings:
In the following do_item_alloc(), 
the check condition for invalidated items should be changed to the new code.
Because, flush can be requested with delay option.

item *do_item_alloc(char *key, const size_t nkey, const int flags, const 
rel_time_t exptime, const int nbytes) {

    ...

    rel_time_t oldest_live = settings.oldest_live;

    search = tails[id];
    if (search == NULL) {
        it = slabs_alloc(ntotal, id);
    } else if (search->refcount == 0) {
#if 1 // new code
        if ((search->time < oldest_live && oldest_live <= current_time) || // dead by flush
         (search->exptime != 0 && search->exptime < current_time)) {
#else // old code
        if ((search->time < oldest_live) || // dead by flush
         (search->exptime != 0 && search->exptime < current_time)) {
#endif
            STATS_LOCK();
            stats.reclaimed++;
            STATS_UNLOCK();
            itemstats[id].reclaimed++;
            if ((search->it_flags & ITEM_FETCHED) == 0) {
                STATS_LOCK();
                stats.expired_unfetched++;
                STATS_UNLOCK();
                itemstats[id].expired_unfetched++;
            }
            it = search;
            it->refcount = 1;
            slabs_adjust_mem_requested(it->slabs_clsid, ITEM_ntotal(it), ntotal);
            do_item_unlink_nolock(it, hash(ITEM_key(it), it->nkey, 0));
            /* Initialize the item block: */
            it->slabs_clsid = 0;
            it->refcount = 0;
        }
    }

    ...

}

Original issue reported on code.google.com by jhpark...@gmail.com on 2 Feb 2012 at 3:07

[GoogleCodeExporter]

One more minor thing..

In do_item_get() function,
following code is used to check whether the fetched item is valid.

    if (it != NULL) {
        if (settings.oldest_live != 0 && settings.oldest_live <= current_time &&
            it->time <= settings.oldest_live) {
            do_item_unlink(it, hv);
            do_item_remove(it);
            it = NULL;
            if (was_found) {
                fprintf(stderr, " -nuked by flush");
            }
        } else if (it->exptime != 0 && it->exptime <= current_time) {
            do_item_unlink(it, hv);
            do_item_remove(it);
            it = NULL;
            if (was_found) {
                fprintf(stderr, " -nuked by expire");
            }
        } else {
            it->it_flags |= ITEM_FETCHED;
            DEBUG_REFCNT(it, '+');
        }
    }

In the above code, 
"it->time <= settings.oldest_live" is used 
instead of "it->time < settings.oldest_live".
So, it might be better to use the same condition in the code of do_item_alloc().

Original comment by jhpark...@gmail.com on 2 Feb 2012 at 3:07

[GoogleCodeExporter]

ah I mean to file a new bug in the future. I've fixed your reported bug and 
we've already released 1.4.12.

does 1.4.12 look okay to you?

Original comment by dorma...@rydia.net on 2 Feb 2012 at 3:13

[GoogleCodeExporter]

Yes.. 1.4.12 is okay.

Original comment by jhpark...@gmail.com on 2 Feb 2012 at 4:11

[GoogleCodeExporter]

thanks :P Resolving!

Original comment by dorma...@rydia.net on 2 Feb 2012 at 6:43

• Changed state: Fixed