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

R.Muilwijk’s picture

Priority: Normal » Critical
andypost’s picture

Really useful - but I think it's better to separate flush & flush_wildcard because flush and flush_temp are same

  function flush($flush = NULL) {
    if (is_null($flush)) {
      $flush = time();
    }
slantview’s picture

Status: Needs review » Needs work

Well, 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.

R.Muilwijk’s picture

As 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.

R.Muilwijk’s picture

Status: Needs work » Needs review
R.Muilwijk’s picture

At the moment a total flush is done doing a delete see:

    if ($wildcard) {
      if ($cid == '*') {
        $cache->delete('*', $table);
      }
      else {
        $cache->delete($cid . '*', $table);
      }
    }
    else {
      $cache->delete($cid, $table);
    }

so $cache->flush() is always just deleting the temporary caches?

andypost’s picture

At this time $cache->flush() deletes everything and should be implemented with timestamp parameter

andypost’s picture

take 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

R.Muilwijk’s picture

@andypost comment #7 and #8, are not true

R.Muilwijk’s picture

To illustrate db.php:

  function flush($flush) {
    $flush = empty($flush) ? time() : $flush;
    $this->content = array();
    db_query("DELETE FROM {". $this->name ."} WHERE expire != %d AND expire < %d", CACHE_PERMANENT, $flush);
  }

However $flush is never given because the CacheRouter class does not support it. So always < time() is deleted.

andypost’s picture

@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

andypost’s picture

to illustrate full flush from core

    if ($wildcard) {
      if ($cid == '*') {
        db_query("DELETE FROM {". $table ."}");
      }
      else {
        db_query("DELETE FROM {". $table ."} WHERE cid LIKE '%s%%'", $cid);
      }
    }

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

slantview’s picture

Status: Needs review » Postponed
andypost’s picture

Status: Postponed » Needs work

First we should fix $flush parameter before Release right now only Db-engine use this parameter by the wrong way

  function flush($time = NULL) {
    if (empty($time)) { // What about 0 value which means CACHE_PERMANENT ?????
      $time = time();
    }
    parent::flush($time);
    db_query("DELETE FROM {". $this->name ."} WHERE expire != %d AND expire < %d", CACHE_PERMANENT, $time);
  }

@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

andypost’s picture

Assigned: R.Muilwijk » andypost

Suppose before release all flush should look the same