[WIP] Improved performance of journal cleaning

src/Kdyby/Redis/RedisJournal.php

@@ -31,6 +31,9 @@ class RedisJournal extends Nette\Object implements Nette\Caching\Storages\IJourn
 		TAGS = 'tags',
 		KEYS = 'keys';
 
+	/** @internal batch delete size */
+	const BATCH_SIZE = 8000;
+
 	/**
 	 * @var RedisClient
 	 */
@@ -108,34 +111,48 @@ private function cleanEntry($keys)
 	 * Cleans entries from journal.
 	 *
 	 * @param  array  $conds
+	 * @param \Nette\Caching\IStorage $storage
 	 *
 	 * @return array of removed items or NULL when performing a full cleanup
 	 */
-	public function clean(array $conds)
+	public function clean(array $conds, Nette\Caching\IStorage $storage = NULL)
 	{
 		if (!empty($conds[Cache::ALL])) {
 			$all = $this->client->keys(self::NS_NETTE . ':*');
+			if ($storage instanceof RedisStorage) {
+				$all = array_merge($all, $this->client->keys(RedisStorage::NS_NETTE . ':*'));
+			}
 
-			$this->client->multi();
 			call_user_func_array(array($this->client, 'del'), $all);
-			$this->client->exec();
 			return NULL;
 		}
 
 		$entries = array();
 		if (!empty($conds[Cache::TAGS])) {
+			$removingTagKeys = array();
 			foreach ((array)$conds[Cache::TAGS] as $tag) {
-				$this->cleanEntry($found = $this->tagEntries($tag));
+				$found = $this->tagEntries($tag);
+				$removingTagKeys[] = $this->formatKey($tag, self::KEYS);
 				$entries = array_merge($entries, $found);
 			}
+			if ($removingTagKeys) {
+				call_user_func_array(array($this->client, 'del'), $removingTagKeys);
+			}
 		}
 
 		if (isset($conds[Cache::PRIORITY])) {
-			$this->cleanEntry($found = $this->priorityEntries($conds[Cache::PRIORITY]));
+			$found = $this->priorityEntries($conds[Cache::PRIORITY]);
+			call_user_func_array(array($this->client, 'zRemRangeByScore'), array($this->formatKey(self::PRIORITY), 0, (int)$conds[Cache::PRIORITY]));
 			$entries = array_merge($entries, $found);
 		}
 
-		return array_unique($entries);
+		$entries = array_unique($entries);
+
+		if ($storage instanceof RedisStorage && $entries) {
+			call_user_func_array(array($this->client, 'del'), $entries);
+		}
+
+		return $storage instanceof RedisStorage ? array() : $entries;
 	}
 
 

src/Kdyby/Redis/scripts/common.lua

@@ -29,6 +29,23 @@ local tagEntries = function (tag)
     return redis.call('sMembers', formatKey(tag, "keys"))
 end
 
+local range = function (from, to, step)
+	step = step or 1
+	local f =
+		step > 0 and
+			function(_, lastvalue)
+				local nextvalue = lastvalue + step
+				if nextvalue <= to then return nextvalue end
+			end or
+		step < 0 and
+			function(_, lastvalue)
+				local nextvalue = lastvalue + step
+				if nextvalue >= to then return nextvalue end
+			end or
+			function(_, lastvalue) return lastvalue end
+	return f, nil, from - step
+end
+
 local cleanEntry = function (keys)
     for i, key in pairs(keys) do
         local tags = entryTags(key)
@@ -45,3 +62,34 @@ local cleanEntry = function (keys)
         -- redis.call('exec')
     end
 end
+
+local mergeTables = function (first, second)
+    for i, key in pairs(second) do
+        first[#first + 1] = key
+    end
+    return first
+end
+
+local batch = function (keys, callback)
+    if #keys > 0 then
+        -- redis.call('multi')
+        -- the magic number 7998 becomes from Lua limitations, see http://stackoverflow.com/questions/19202367/how-to-avoid-redis-calls-in-lua-script-limitations
+        local tmp = {}
+        for i,key in pairs(keys) do
+            tmp[#tmp + 1] = key
+            if #tmp >= 7998 then
+                callback(tmp)
+                tmp = {}
+            end
+        end
+        callback(tmp)
+        -- redis.call('exec')
+    end
+end
+
+local batchDelete = function(keys)
+    local delete = function (tmp)
+        redis.call('del', unpack(tmp))
+    end
+    batch(keys, delete)
+end

src/Kdyby/Redis/scripts/journal.clean.lua

@@ -2,38 +2,18 @@
 local conds = cjson.decode(ARGV[1])
 
 if conds["all"] ~= nil then
-    -- redis.call('multi')
-    for i, value in pairs(redis.call('keys', "Nette.Journal:*")) do
-        redis.call('del', value)
+    batchDelete(redis.call('keys', "Nette.Journal:*"))
+    if conds["delete-entries"] ~= nil then
+        batchDelete(redis.call('keys', "Nette.Storage:*"))
     end
-    -- redis.call('exec')
 
     return redis.status_reply("Ok")
 end
 
 local entries = {}
-if conds["tags"] ~= nil then
-    for i, tag in pairs(conds["tags"]) do
-        local found = tagEntries(tag)
-        if #found > 0 then
-            cleanEntry(found)
-
-            for i, key in pairs(found) do
-                if conds["delete-entries"] ~= nil then
-                    redis.call("del", key)
-                else
-                    entries[#entries + 1] = key
-                end
-            end
-        end
-    end
-end
 
-if conds["priority"] ~= nil then
-    local found = priorityEntries(conds["priority"])
+local processFoundKeys = function (found)
     if #found > 0 then
-        cleanEntry(found)
-
         for i, key in pairs(found) do
             if conds["delete-entries"] ~= nil then
                 redis.call("del", key)
@@ -44,4 +24,20 @@ if conds["priority"] ~= nil then
     end
 end
 
+if conds["tags"] ~= nil then
+    local formattedTagKeys = {}
+    for i, tag in pairs(conds["tags"]) do
+        processFoundKeys(tagEntries(tag))
+        formattedTagKeys[#formattedTagKeys + 1] = formatKey(tag, 'keys')
+    end
+    if #formattedTagKeys > 0 then
+        redis.call("del", unpack(formattedTagKeys))
+    end
+end
+
+if conds["priority"] ~= nil then
+    processFoundKeys(priorityEntries(conds["priority"]))
+    redis.call('zRemRangeByScore', formatKey('priority'), 0, conds["priority"])
+end
+
 return entries

tests/KdybyTests/Redis/RedisJournal.phpt

@@ -173,15 +173,15 @@ class RedisJournalTest extends AbstractRedisTestCase
 		Assert::same('ok_test6_7', $result[0], "clean tag homepage/7");
 
 		$result = $this->journal->clean(array(Cache::TAGS => array('test:homepage/4')));
-		Assert::same(0, count($result), "clean non exists tag");
+		Assert::same(1, count($result), "clean non exists tag");
 
 		$result = $this->journal->clean(array(Cache::PRIORITY => 4));
 		Assert::same(0, count($result), "clean non exists priority");
 
 		$result = $this->journal->clean(array(Cache::TAGS => array('test:homepage')));
-		Assert::same(4, count($result), "clean other");
+		Assert::same(10, count($result), "clean other");
 		sort($result);
-		Assert::same(array('ok_test6_10', 'ok_test6_6', 'ok_test6_8', 'ok_test6_9'), $result, "clean other");
+		Assert::same(array('ok_test6_1', 'ok_test6_10', 'ok_test6_2', 'ok_test6_3', 'ok_test6_4', 'ok_test6_5', 'ok_test6_6', 'ok_test6_7', 'ok_test6_8', 'ok_test6_9'), $result, "clean other");
 	}
 
 
@@ -284,7 +284,7 @@ class RedisJournalTest extends AbstractRedisTestCase
 		Assert::null($result);
 
 		$result2 = $this->journal->clean(array(Cache::TAGS => 'test:all'));
-		Assert::true(empty($result2));
+		Assert::equal(array(), $result2);
 	}
 
 
@@ -313,7 +313,7 @@ LUA;
 		$this->assertKeysInDatabase(5100);
 
 		$this->journal->clean(array(Cache::TAGS => 'test.4356'));
-		$this->assertKeysInDatabase(0);
+		$this->assertKeysInDatabase(5099);
 	}
 
 
@@ -340,7 +340,7 @@ LUA;
 		$this->assertKeysInDatabase(200001);
 
 		$this->journal->clean(array(Cache::TAGS => 'kdyby'));
-		$this->assertKeysInDatabase(0);
+		$this->assertKeysInDatabase(200000);
 	}
 
 
@@ -362,26 +362,7 @@ LUA;
 	private function cacheGeneratorScripts()
 	{
 		$script = file_get_contents(__DIR__ . '/../../../src/Kdyby/Redis/scripts/common.lua');
-		$script .= <<<LUA
-local range = function (from, to, step)
-	step = step or 1
-	local f =
-		step > 0 and
-			function(_, lastvalue)
-				local nextvalue = lastvalue + step
-				if nextvalue <= to then return nextvalue end
-			end or
-		step < 0 and
-			function(_, lastvalue)
-				local nextvalue = lastvalue + step
-				if nextvalue >= to then return nextvalue end
-			end or
-			function(_, lastvalue) return lastvalue end
-	return f, nil, from - step
-end
-
 
-LUA;
 		return $script;
 	}