Needs work
Project:
Cache Router
Version:
6.x-1.x-dev
Component:
Code
Priority:
Critical
Category:
Bug report
Assigned:
Reporter:
Created:
31 Oct 2008 at 08:09 UTC
Updated:
28 Jan 2010 at 21:00 UTC
At the moment we have fairly primitive functions we can use:
get()
set()
delete()
flush()
When looking at the drupal core code we infact have 6 functionality:
get()
set()
delete()
flush()
flush_temporary()
flush_wildcard()
I propose actually making this functions which would make it alot easier creating the different flush types. This will also fix alot the troubles we have with the wildcard code at the moment. I set this on review so you, slantview, will have a look at this. If you agree I can make the patch for you.
Comments
Comment #1
R.Muilwijk commentedComment #2
andypostReally useful - but I think it's better to separate flush & flush_wildcard because flush and flush_temp are same
Comment #3
slantview commentedWell, my only concern is that we are over-complicating something that could be a simple solution. Although I do want to keep the existing functionality of core, I also want to keep this something we can expand in the future.
Personally, I feel that CacheRouter's functionality of breaking out delete() and flush() into two modules is _more_ correct that the cache_clear_all which is a joke of a function overridden to do too many things. If you look at other cache systems, get, set, flush and delete are the only four functions you need. I added the ability to do $cache->delete('something:*', $table) because I wanted to match Drupal's core functionality, but it seems like something we shouldn't need if the cache is working correctly.
So since I am already doing it in delete, i suppose we could break out $cache->delete('something:*', $bin) into $cache->flush_wildcard('something:', $bin), but I really don't see the point. The functionality is already there.
Comment #4
R.Muilwijk commentedAs long as it's clear to do the various drupal core types of deleting. The exact method of it is not really my concern.
So if the four options:
- clear one cid
$cache->delete('something', $bin)
- clear wildcard
$cache->delete('something:*', $bin)
- clear temporary
$cache->flush($cache_flush (integer timestamp))
- flush cache
$cache->flush();
are implemented like that it's fine for me. However in beta-4 this was not the case. If you can confirm this are the four delete/flushing options available now you can set this to fixed.
Comment #5
R.Muilwijk commentedComment #6
R.Muilwijk commentedAt the moment a total flush is done doing a delete see:
so $cache->flush() is always just deleting the temporary caches?
Comment #7
andypostAt this time $cache->flush() deletes everything and should be implemented with timestamp parameter
Comment #8
andyposttake a look at bug #227228: cache_clear_all and cache_get fail to clear caches when Minimum cache lifetime is on
problem with cache_flush when cache_lifetime is not 0 - cleared only first cache
Suppose we can make cache_flush per bin - then eliminate this bug
every flush initiated by cache_clear_all with $cid=NULL so we need {prefix}{bin}_flush key to store flush time
When flush executed cleared only TEMP entries and expired (maybe useful for file-engine)
Delayed flush (when cache_lifetime !=0) in original cache.inc processed in cache_get() - in this situation we need separate logic for TEMP and PERM data (both of them looks like permanent but only temp goes cleared)
Summary
If cache_lifetime > 0 then we need flush parameter per bin and check in cache_get or hook_flush_caches implementation
Comment #9
R.Muilwijk commented@andypost comment #7 and #8, are not true
Comment #10
R.Muilwijk commentedTo illustrate db.php:
However $flush is never given because the CacheRouter class does not support it. So always < time() is deleted.
Comment #11
andypost@R.Muilwijk
I wrote #7 #8 about cacherouter not drupal
In DB.php engine logiс from core
but current realization of CR is different and should be fixed
I seen your patch for apc - it looks much promising
So this issue about delete-flush in CR, I wrote some thoughts about in #8
Comment #12
andypostto illustrate full flush from core
I found no ability to implement this in memcache apc eacc because there's no enumeration of entries
Possible solution is to have some system backend to enumerate stored keys so memcachedb looks promising
IMO, Functions
1) delete key
2) delete key + mask
3) flush whole bin
flush by time should be implemented for engines which does not support automatic expiration
Comment #13
slantview commentedComment #14
andypostFirst we should fix $flush parameter before Release right now only Db-engine use this parameter by the wrong way
@slantview as fixed #266588: Obey "expire" value on cache_clear_all(), like Drupal core
let's put $flush = NULL parameter into Cache class and use it when walking through lookup array for clearing expired values
Comment #15
andypostSuppose before release all flush should look the same