Allow expired items to make use of stampede protection; ensure that memcache optionally behaves like DB cache

This also occurs for block_cache where CACHE_TEMPORARY is always used for cache_set, memcache converts CACHE_TEMPORARY to 1 month or so but then *does not* flush these entries when cache_clear_all(NULL, 'cache_block') is called by system_cron (for the same reasons as above).
The result is that blocks are cached for 1 month and survive a cron run.

I'm surprised that there is not further discussion here. Does anyone have a recommendation for how to handle the CACHE_TEMPORARY issue? My thought at the very least is that CACHE_TEMPORARY should be trated as no more than one day by memcache, or should be configurable. 1 month is just absurd. It makes the block cache truly useless for blocks containing any kind of dynamic content. What about and admin setting to choose the duration of CACHE_TEMPORTARY employed by memcache?

This patch fixes the issue and adds test coverage to confirm that memcache responds in the same way as DrupalDatabaseCache when calling cache_clear_all(NULL, $bin).

I'm having the same issue. I have some data stored in cache temp which gets retrieved on cron runs. We just had a bug report saying that the data hadn't been updated in months, turns out this is the issue! Will report with results from patch

Can confirm this fixes my issues perfectly, cheers!

Just a note, this is not specific to cache_form but anything using CACHE_TEMPORARY

We are having the exact same issue. I'm going to try the patch.

I created a custom module with CACHE_TEMPORARY and without memcache it works as expected. With memcache (on prod), it doesn't get cleared.

#3 Worked for our custom module using CACHE_TEMPORARY. Everything seems to clear as expected.

• Patch applies cleanly to latest memcache API HEAD
• New test passes
• Patch looks good and works for above users and me

+1 rtbc

Fixing a typo of "exirable" in the test comments.

Well spotted!

Setting back to needs review and interdiff attached for patch in #11 vs #3 to speed things up.

This issue seems to be the underlying cause of a problem on my site. We have some time-based content which should be updated throughout the day, and over night however it was only getting updated sporadically throughout the day. I've traced the issue to the handling of cache_clear_all(NULL,$bin) being a NO-OP on memcache. The reason content was sporadically being updated (as opposed to never) during the day was that any call to "node_form_submit()" would then call "cache_clear_all()" with no parameters which seems to be handled differently than "cache_clear_all(NULL,$bin)".

My fix until this patch gets further along is to call "cache_clear_all()" during every cron run.

Setting back to RTBC. It's only a minor comment change and still applies cleanly to 7.x-dev.

Thanks! This is an important fix. Patch committed:
http://drupalcode.org/project/memcache.git/commit/5c52cff

I came across this while reviewing changes since 7.x-1.0.

It looks like the committed fix doesn't address invalidating CACHE_TEMPORARY in a way that supports stampede protection.

Also, I was thinking whether a second array key is really needed for this. Aside from some slight overhead, I think the code could be more understandable if the original $expire was what was stored in the cache item, and that include the CACHE_TEMPORARY value. The current use of $cache->expire to start calculating an expiration, which then transitions to $memcache_expire to calculate a later time for stampede protection, doesn't read very well IMO.

I'd propose solely using $memcache_expire variable to calculate each of the different cache expiration cases (REQUEST_TIME+30days for temporary, x2 for permanent, etc), and passing through the original timestamp.

I might have an initial revision done tonight, but if not I'd appreciate any feedback on the idea.

I'm happy to take a look at your patch, marking this as active to reflect this; please update to needs review once you attach your patch.

This may be related to a bug I found elsewhere: #2366709: memcache_requirements() returns 'failed to store then retrieve data' on Windows

While trying to refactor this for my recommendations in #18 (retain original $expires parameter into the cache object instead of adding $cache->temporary, and support stampede protection for expired entries), I ran into a dilemma. The memcache module has traditionally not lined up with the API's definition of cache_set, namely, that an expired item behaves the same as CACHE_TEMPORARY (still valid until "general cache wipe"). This may be considered a limitation of how DB caching works (that it requires garbage collection), but the fact remains that cache_set with memcache module doesn't behave the way the API says it should. I don't know what is preferred, so I added a variable that allows the module to function the same way as described in the API, but leaving the traditional behavior as the default. Also, if stampede protection is disabled and the traditional behavior (exact timestamp expirations) is kept, I changed it so that the exact expiration is passed to memcached. This was the behavior before stampede protection was added, and for those not using stampede protection, it means a slightly smaller cache and less cache hits retrieved across the wire which just end up failing valid().

This fails 3 tests in cache clearing, but these tests already were failing for me in 7.x-1.x (I suspect it's due to a REQUEST_TIME/time() discrepancy).

I can split this into multiple patches if that is preferred.

Updating status now that a patch is available.

Changing summary

Rerolled.

Thanks, applied!