cache_clear_all() not properly handled in PhpRedis.php

Patch in #1 has fixed a bunch of problems for me too. Many thanks

I was previously having stale cache problems, and this patch sure wiped out my cache :)

With my install that's about ~250k keys and takes around two minutes to clear now.

My poor cache is empty after pre-caching my ~60k nodes in redis after installing yesterday.

Going to have to write some more fine grained `drush cc` commands to prevent things like entity_cache for me from getting purged on stuff like this, but that's my problem and this patch seems to fix a bug I was experiencing.

Wow! The patch from #1 fixed this giant issue/thread as well! Thank you!

@mja That is the most awaited patch ever for this module! We fought literally days with omega8cc to find the cause of Redis connections stalling! Thank you so much, both of you deserve credits on the module homepage :)

Just a question, don't you think it should be directly after the while in the if () statement instead?

It's just one line, I can do it myself :) All the credit will go to you! thank you very much once again finding this one.

I won't do before a few days I have too much work and a few other patches to review and push for the module.

Ahah, I'm very happy to see this module being useful to many people. When I wrote it it was just a proof of concept because I wanted to test Redis, it wasn't supposed to become this, I'm glad I shared it.

On a side note while everyone is here, why does `drush cc all` take so long on a machine with many keys with this module. As mentioned, it took my install a couple minutes to clear all the keys, while PHP is at 100%.

I assume something is getting done, like pulling in all the keys from redis to PHP, then going through the keys in PHP and deleting them in redis.

Would there be a way to avoid this, and simply gather keys in redis, then expire them in redis, while avoiding PHP.

Keys are not "loaded" during the clear call, they are just listed by prefix. The explicit delete is the only place where we have a non scalable algorithm, because the Drupal signature askes us to do that. Tell me if I'm wrong, but Redis cannot do operations on multiples keys at once without listing them on the client side firt (i.e. for example cannot "DEL someprefix*") so we are stick to that method. Another probably faster way to introspect would be use a LUA piece of code to do that on the Redis side only.

Did you try with the patch in this issue?

There is some ways to improve slightly your cron performance, see http://drupalcode.org/project/redis.git/blob/refs/heads/7.x-2.x:/README....

The methodology would be:

1. Find the bin(s) that take too much time;
2. Change the flush mode 0 ("Never flush temporary: leave Redis handling the TTL;") for those bins

But mind the fact that it might cause stalling cache entries in the database. It might improve things to do this on the field, entity and page caches (which will expand the most I guess), but you MUST set a default cache lifetime for them ("cache_lifetime" variable, and for page another non documented variable is being used, see the bootstrap.inc file).

Remember that this CANNOT improve the manual cache clear, but remember such cache clear will never happen on a production site.

@pounard, Thanks for your reply. Yes, I did apply this patch, and now redis clears my cache as expected.

@mja, I haven't done an XHProf on a cache clear yet, but I can tell you from experience with this site I'm working on, cache clears used to take maybe 10 seconds, now they take 2 minutes with 100% cpu usage.

While I understand, that redis module is only loading the keys and not values, but with a lot of keys (250k+ for me), I do believe from looking at the code, that this is the problem.

--

I'm not a redis expert, but I have used it on a non-drupal related project (one of the reasons I switched over my Drupal install to use redis) and I do believe it should be possible to iterate over the keys in redis and delete them with out having to touch PHP. At least I'm hoping it can be done.

Although I'm not sure how currently. My guess is either Lua or something like loading the keys into a list, passing that list over to DEL.

A quick Google does come up with what I believe is a Lua script which kind of does what I'm looking for:
http://stackoverflow.com/questions/17224817/delete-redis-hash-values-in-...

EVAL Script (looks like it's for hashes though)

local keys = redis.call('KEYS',KEYS[1])
for i,k in ipairs(keys) do
    local res = redis.call('HKEYS',k)
    for j,v in ipairs(res) do
        if string.find(v,ARGV[1]) then
            redis.call('HDEL',k,v)
        end
    end
end

@mja, I have another thread in the issue queue you might have seen about igbinary support. Currently that's the least of my concerns for now, but something I will look at in the future. Let's keep this on topic.

As for Lua. I think I have a script which works, but needs some fixes.

Create clear_keys.lua in your current directory.

local thekeys = redis.call("KEYS", ARGV[1])
redis.call("DEL", unpack(thekeys))
return thekeys

For testing do this:

$ redis-cli SET foo:one bar
OK


$ redis-cli SET foo:two baz
OK


$ redis-cli KEYS foo:*
1) "foo:two"
2) "foo:one"


$ redis-cli EVAL "$(cat clear_keys.lua)" 0 "foo:*"
1) "foo:one"
2) "foo:two"


$ redis-cli KEYS foo:*
(empty list or set)

The script has problems after the second run, if nothing is returned when pulling the keys, but I assume this can be fixed with an if.

PS. "$(cat clear_keys.lua)" just loads the script into redis for the script as EVAL code. That's bash, has nothing to do with redis.

Agreed, we should probably stay away from Lua, but I think for this case, clearing keys in bulk and avoiding PHP, it's pretty valid.

I assume it'll be faster, but we need to benchmark that. I'm leaving for the day right now, but I'll test it sometime shortly in the future.

For the original issue, see http://drupalcode.org/project/redis.git/commit/810cce5

I'm not closing the issue even thought it's solved since the discussion that follows is quite advanced and very interesting.

I want to try the LUA script. If done correctly it will probably give a great performance boot for keys manual expiration.

Release 7.x-2.6 is 'en route' and coming soon containing the original bugfix.

Please give me feedback when you have tested it. If you need tuning or slight changes, I'll be pleased to listen and adapt if necessary.

There's a bug with my script if you're using it to remove a tonne of keys.

$ redis-cli EVAL "$(cat clear_redis.lua)" 0 "yt:cache_metatag:output:global:*"

(error) ERR Error running script (call to f_68761746df352b24cfdabd73ab97612378629cdb): user_script:2: too many results to unpack

Seems like there's a hardcoded limit of ~8000.

http://stackoverflow.com/questions/19202367/how-to-avoid-redis-calls-in-...

I'll look into improving it to avoid this limit. Any help appreciated.

Updated Script, which chunks through the keys ~8000 at a time.

local tempI = 0 
local tempKeysToDelete = {} 
local keysToDelete = redis.call("KEYS", ARGV[1])
for _, key in pairs(keysToDelete) do 
  tempKeysToDelete[tempI] = key 
  tempI = tempI + 1
  if (tempI >= 7995) then 
    redis.call("DEL", unpack(tempKeysToDelete)) 
    tempI = 0 
    tempKeysToDelete = {} 
  end
end
  
if (tempI > 0) then 
  redis.call("DEL", unpack(tempKeysToDelete))
end 

To run

$ redis-cli EVAL "$(cat clear_redis.lua)" 0 "yt:cache_metatag:output:global:*"

Same problem here. I use entity_cache for nodes and if i try delete node i get loop through more than 85k keys. Then I always get " Maximum execution time of 30 seconds exceeded".

havran, are you using the latest stable version?

If that's still happening, the LUA script may end necesserary in the end. One good way to implement would be to make it optional and require a settings.php parameter for the site admin to set for activating the LUA variant of the cache clear algorithm. I'd like to try this but sadly I don't really have much spare time nowadays.

Yes i use lastest stable (redis-2.6) version. For now i set max_execution_time to 60 seconds and i delete node successfuly but we have about 500k nodes on site. Then we can reach large number of entity_cache_node keys.

I've been using my latest lua script posted above to manually prune redis on mass. Seems to work well for my use case.

@havran, do you use the metatag module in your install? Else why do you have so many cache entries? Just curious.

Hi, i'm maybe doing something wrong (i'm not very experienced with this) but i use redis for caching almost all items on our pages (local newspaper online). Here is configuration:

/**
 * Redis section
 */
if ($enable_redis) {
  $conf['cache'] = 1;
  $conf['cache_lifetime'] = 120;
  $conf['page_cache_maximum_age'] = 120;

  $conf['redis_client_interface']      = 'PhpRedis';
  $conf['cache_backends'][]            = 'sites/all/modules/contrib/redis/redis.autoload.inc';
  $conf['cache_prefix']['default']     = 'hn';

  //$conf['redis_client_host'] = '10.1.1.46';
  $conf['redis_client_host'] = '127.0.0.1';
  $conf['redis_client_port'] = 6379;

  $conf['lock_inc'] = 'sites/all/modules/contrib/redis/redis.lock.inc';

  // cache bins
  $conf['cache_class_cache']                             = 'Redis_Cache';
  $conf['cache_class_cache_bootstrap']                   = 'Redis_Cache';
  $conf['cache_class_cache_menu']                        = 'Redis_Cache';
  $conf['cache_class_cache_path']                        = 'Redis_Cache';
  //$conf['cache_class_cache_form']                        = 'Redis_Cache';
  $conf['cache_class_cache_field']                       = 'Redis_Cache';
  $conf['cache_class_cache_filter']                      = 'Redis_Cache';
  $conf['cache_class_cache_page']                        = 'Redis_Cache';

  $conf['cache_class_cache_entity_file']                 = 'Redis_Cache';
  $conf['cache_class_cache_entity_node']                 = 'Redis_Cache';
  $conf['cache_class_cache_entity_comment']              = 'Redis_Cache';
  $conf['cache_class_cache_entity_taxonomy_term']        = 'Redis_Cache';
  $conf['cache_class_cache_entity_taxonomy_vocabulary']  = 'Redis_Cache';

  $conf['cache_class_cache_views']                       = 'Redis_Cache';
  $conf['cache_class_cache_views_data']                  = 'Redis_Cache';

  $conf['cache_class_cache_metatag']                     = 'Redis_Cache';

  $conf['cache_class_cache_online_prenos_item']          = 'Redis_Cache';
}

If i try delete single node through Drupal administration, Drupal try delete all cached keys for cache_entity_node bin. In this bin i have sometimes ~100k keys - then deleting all this keys are too slow. I test localy this approach (i modify PhpRedis.php script):

# This patch file was generated by NetBeans IDE
# It uses platform neutral UTF-8 encoding and \n newlines.
--- HEAD
+++ Modified In Working Tree
@@ -124,7 +124,11 @@
   }
 
   function clear($cid = NULL, $wildcard = FALSE) {
+    //dd($this->bin, 'bin');
+    //dd($cid, 'cid');
+    //dd($wildcard, 'wildcard');
 
+    $command = FALSE;
     $keys   = array();
     $skey   = $this->getKey(Redis_Cache_Base::TEMP_SET);
     $client = Redis_Client::getClient();
@@ -158,26 +162,36 @@
 
     if ('*' !== $cid && $wildcard) {
       // Prefix flush.
-      $remoteKeys = $client->keys($this->getKey($cid . '*'));
-      // PhpRedis seems to suffer of some bugs.
-      if (!empty($remoteKeys) && is_array($remoteKeys)) {
-        $keys = array_merge($keys, $remoteKeys);
+//      $remoteKeys = $client->keys($this->getKey($cid . '*'));
+//      // PhpRedis seems to suffer of some bugs.
+//      if (!empty($remoteKeys) && is_array($remoteKeys)) {
+//        $keys = array_merge($keys, $remoteKeys);
+//      }
+      
+      $remoteKey = $this->getKey($cid . '*');
+      $command = "for _,k in ipairs(redis.call('keys','{$remoteKey}')) do redis.call('del',k) end";
       }
-    }
     else if ('*' === $cid) {
       // Full bin flush.
-      $remoteKeys = $client->keys($this->getKey('*'));
-      // PhpRedis seems to suffer of some bugs.
-      if (!empty($remoteKeys) && is_array($remoteKeys)) {
-        $keys = array_merge($keys, $remoteKeys);
+//      $remoteKeys = $client->keys($this->getKey('*'));
+//      // PhpRedis seems to suffer of some bugs.
+//      if (!empty($remoteKeys) && is_array($remoteKeys)) {
+//        $keys = array_merge($keys, $remoteKeys);
+//      }
+      
+      $remoteKey = $this->getKey('*');
+      $command = "for _,k in ipairs(redis.call('keys','{$remoteKey}')) do redis.call('del',k)";
       }
-    }
     else if (empty($keys) && !empty($cid)) {
       // Single key drop.
       $keys[] = $key = $this->getKey($cid);
       $client->srem($skey, $key);
     }
 
+    if ($command) {
+      $client->eval($command);
+    }
+    
     if (!empty($keys)) {
       if (count($keys) < Redis_Cache_Base::KEY_THRESHOLD) {
         $client->del($keys);

I try eval this lua script on my live server and deleting many keys (about 100k) seems fast in this way.

It's an interesting solution it worthes looking at. As I said, I do need to make some time for looking into this, which I seriously can't do right now; Nevertheless please keep me posted in the issue about the various solutions that work, I'll need it!

LUA script is get from this link http://stackoverflow.com/questions/4006324/how-to-atomically-delete-keys... (How to atomically delete keys matching a pattern using Redis).

I try use this on our server and drush cc all command seems work much faster than before (about 300k keys id redis database). I make patch for this.

Indeed this should be a lot faster since it drastically reduces the consumed bandwith and remove all keys in a single call. I'd advice people that experience the clear problem to (review first of course) and use this patch until I commit it or a variant of it.

From your config. This is the problem.

 $conf['cache_class_cache_metatag']                     = 'Redis_Cache';

I've personally disabled metatag from getting stored in redis, as it stores way too much data. Here's a screenshot of what metatag module does to redis cache. The spikes are only because of metatag module. Each entry is about 8k on my install, and I get hundreds of thousands of entries after a couple days. THey've improved it, but it's still causes too many entries, so I've removed it.

I've recommended to pounard, that metatag get excluded from redis by default. It should also be removed from memcache, if you're using that as it will eat up your memory, and cause more important entries to get pruned.

https://drupal.org/comment/8522841#comment-8522841

Yep, I'll do that when I can :)

Why is this not fixed ? Humpf.

Both patches not work. Cache not cleared.

Here is a patch when you using predis instead of PhpRedis. Only tested on development environment yet.

I use PhpRedis. Nothing happend after cache clear. Redis server has all keys as before.

Please note that I just release the first 3.x version of this module, which relies on the EVAL command a few LUA scripts to proceed to cache flush. For all people still experiencing problems with cache flush, you can try this new version.

Be aware this has not been production tested yet and require Redis server 2.6 (and works fine with 2.8).

See https://www.drupal.org/node/2431349 which will be published in a few minutes.

For all other people still interested in seeing EVAL scripts going to 2.x branch, please follow this issue: #2415705: Timeout when clearing large databases

Since KEYS command is not recommended in production, you might be interested in
#2851625: New feature: option to use SCAN command instead of KEYS on cache wildcard deletions