Closed (fixed)
Project:
Memcache API and Integration
Version:
5.x-1.x-dev
Component:
Code
Priority:
Critical
Category:
Task
Assigned:
Reporter:
Created:
15 Aug 2008 at 14:18 UTC
Updated:
9 Feb 2009 at 21:30 UTC
Jump to comment: Most recent file
Comments
Comment #1
chx commentedComment #2
chx commentedHere is why this patch is good: imagine you have N frontends and a memcache pool and you cache the results of a slow query. Now, when that object expires then you will suddenly run N pieces of slow query. With this patch you will run said query before the object can expire and you are guaranteed to run only one.
Edit: to achieve this, we will return one and exactly one FALSE 30 seconds before the object can expire so that the requester will recreate and recache the object.
Comment #3
robertdouglass commentedThanks for finally providing the patch!
Comment #4
chx commentedThis is an even better version. The previous version had a problem: if the object expired then you still could get a stampede. So now all the items are permanent and once the item has expired, one and only one process will get a chance to re-build and re-set the object. Ample comments are provided because it's not entirely trivial but IMO it's rather elegant.
Comment #5
jeremy commentedFor any content that expires with a TTL, I like this very much. But this doesn't help with any content that expires with a call to cache_clear_all, does it?
Comment #6
jeremy commentedThe cache_clear_all issue is secondary -- this patch is really just trying to address items that expire after a specific ttl. In my opinion, this is very clean and should be committed. Does anyone else want to weigh in on it?
Silence will be construed as a yes vote.
Comment #7
robertdouglass commentedI've followed this with chx from near the beginning of the idea. Thumbs up from me.
Comment #8
jeremy commented@chx: Do we really want to hold the lock for 30 seconds? Why do we need to hold the lock longer than 5 seconds? Perhaps we should turn this into a define() which can make it even more obvious what's going on?
Comment #9
chx commentedJeremy: whatever. I can't think of a scenario where you would cache an object for less than 30s and then again a rebuild process that takes more than 30s is horrible... I can't say the same thing for 5s -- if a rebuild takes 5.5s, well we are not happy but it's not the end of the world.
Comment #10
jeremy commentedChx: How is the attached? I compromised per our irc discussion to 15 seconds, and made it a define. I also reworded some comments (but it's quite late as I do this).
Comment #11
chx commentedI rather dislike the word "session", you wanted "request". "we do not return the cached object, instead causing it to be rebuilt" what about "return a FALSE isntead of the cache object causing it to be rebuilt"?
Comment #12
david straussCACHE_STAMPEDE_SEMAPHORE should probably be a variable to be more easily override-able.
I dislike telling memcache that the expiration for items is infinite. I'm guessing it hurts the ability for memcache to effectively prune a full cache.
Comment #13
jeremy commentedCommitted to 6.x-1.x-dev. Will backport to 5.x-dev as well.
Comment #14
jeremy commentedPer David's request, made memcache_stampede_semaphore a variable (in 6.x-1.x-dev).
Comment #15
jeremy commentedBackported this patch and committed to 5.x-1.x-dev.
Comment #16
jeremy commentedNote, David and I discussed his concern about memcache's LRU, and we confirmed that the lifetime of the cached item does not affect memcache's LRU algorithm's ability to prune content when it fills up.