From 8ddfa20121dbab7a3b0d324cdac3eb0560bc7a1e Mon Sep 17 00:00:00 2001 From: sam marshall Date: Thu, 12 Aug 2021 14:23:40 +0100 Subject: [PATCH] MDL-72328 cachestore_redis: Add TTL support for Redis cache A list of times for each cache key in a TTL cache is kept in a Redis sorted list, which can be queried efficiently to delete expired cache items later in a scheduled task. This change makes set and delete 2x slower (only for caches which use TTL) but there is no impact on get performance. --- cache/stores/redis/classes/task/ttl.php | 105 ++++++++++++ cache/stores/redis/db/tasks.php | 38 +++++ .../stores/redis/lang/en/cachestore_redis.php | 3 + cache/stores/redis/lib.php | 161 +++++++++++++++++- cache/stores/redis/settings.php | 6 + cache/stores/redis/tests/ttl_test.php | 113 ++++++++++++ cache/stores/redis/version.php | 2 +- 7 files changed, 422 insertions(+), 6 deletions(-) create mode 100644 cache/stores/redis/classes/task/ttl.php create mode 100644 cache/stores/redis/db/tasks.php create mode 100644 cache/stores/redis/tests/ttl_test.php diff --git a/cache/stores/redis/classes/task/ttl.php b/cache/stores/redis/classes/task/ttl.php new file mode 100644 index 00000000000..47e9b1fa6c2 --- /dev/null +++ b/cache/stores/redis/classes/task/ttl.php @@ -0,0 +1,105 @@ +. + +namespace cachestore_redis\task; + +/** + * Task deletes old data from Redis caches with TTL set. + * + * @package cachestore_redis + * @copyright 2021 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class ttl extends \core\task\scheduled_task { + /** @var int Only display memory savings of at least 100 KB */ + const MIN_MEMORY_SIZE = 100 * 1024; + + /** + * Gets the name of this task. + * + * @return string Task name + */ + public function get_name(): string { + return get_string('task_ttl', 'cachestore_redis'); + } + + /** + * Executes the scheduled task. + */ + public function execute(): void { + // Find all Redis cache stores. + $factory = \cache_factory::instance(); + $config = $factory->create_config_instance(); + $stores = $config->get_all_stores(); + $doneanything = false; + foreach ($stores as $storename => $storeconfig) { + if ($storeconfig['plugin'] !== 'redis') { + continue; + } + + // For each definition in the cache store, do TTL expiry if needed. + $definitions = $config->get_definitions_by_store($storename); + foreach ($definitions as $definition) { + if (empty($definition['ttl'])) { + continue; + } + if (!empty($definition['requireidentifiers'])) { + // We can't make cache below if it requires identifiers. + continue; + } + $doneanything = true; + $definitionname = $definition['component'] . '/' . $definition['area']; + mtrace($definitionname, ': '); + \cache::make($definition['component'], $definition['area']); + $definition = $factory->create_definition($definition['component'], $definition['area']); + $stores = $factory->get_store_instances_in_use($definition); + foreach ($stores as $store) { + // These were all definitions using a Redis store but one definition may + // potentially have multiple stores, we need to process the Redis ones only. + if (!($store instanceof \cachestore_redis)) { + continue; + } + $info = $store->expire_ttl(); + $infotext = 'Deleted ' . $info['keys'] . ' key(s) in ' . + sprintf('%0.2f', $info['time']) . 's'; + // Only report memory information if available, positive, and reasonably large. + // Otherwise the real information is hard to see amongst random variation etc. + if (!empty($info['memory']) && $info['memory'] > self::MIN_MEMORY_SIZE) { + $infotext .= ' - reported saving ' . display_size($info['memory']); + } + mtrace($infotext); + } + } + } + if (!$doneanything) { + mtrace('No TTL caches assigned to a Redis store; nothing to do.'); + } + } + + /** + * Checks if this task is allowed to run - this makes it show the 'Run now' link (or not). + * + * @return bool True if task can run + */ + public function can_run(): bool { + // The default implementation of this function checks the plugin is enabled, which doesn't + // seem to work (probably because cachestore plugins can't be enabled). + // We could check if there is a Redis store configured, but it would have to do the exact + // same logic as already in the first part of 'execute', so it's probably OK to just return + // true. + return true; + } +} diff --git a/cache/stores/redis/db/tasks.php b/cache/stores/redis/db/tasks.php new file mode 100644 index 00000000000..235c10d30a1 --- /dev/null +++ b/cache/stores/redis/db/tasks.php @@ -0,0 +1,38 @@ +. + +/** + * Scheduled tasks. + * + * @copyright 2021 The Open University + * @package cachestore_redis + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$tasks = [ + [ + 'classname' => '\cachestore_redis\task\ttl', + 'blocking' => 0, + 'minute' => 'R', + 'hour' => '*', + 'day' => '*', + 'month' => '*', + 'dayofweek' => '*', + 'disabled' => 0 + ] +]; diff --git a/cache/stores/redis/lang/en/cachestore_redis.php b/cache/stores/redis/lang/en/cachestore_redis.php index a52b93f93fd..f323d312a5f 100644 --- a/cache/stores/redis/lang/en/cachestore_redis.php +++ b/cache/stores/redis/lang/en/cachestore_redis.php @@ -41,12 +41,15 @@ $string['server'] = 'Server'; $string['server_help'] = 'This sets the hostname or IP address of the Redis server to use.'; $string['password'] = 'Password'; $string['password_help'] = 'This sets the password of the Redis server.'; +$string['task_ttl'] = 'Free up memory used by expired entries in Redis caches'; $string['test_server'] = 'Test server'; $string['test_server_desc'] = 'Redis server to use for testing.'; $string['test_password'] = 'Test server password'; $string['test_password_desc'] = 'Redis test server password.'; $string['test_serializer'] = 'Serializer'; $string['test_serializer_desc'] = 'Serializer to use for testing.'; +$string['test_ttl'] = 'Testing TTL'; +$string['test_ttl_desc'] = 'Run the performance test using a cache that requires TTL (slower sets).'; $string['useserializer'] = 'Use serializer'; $string['useserializer_help'] = 'Specifies the serializer to use for serializing. The valid serializers are Redis::SERIALIZER_PHP or Redis::SERIALIZER_IGBINARY. diff --git a/cache/stores/redis/lib.php b/cache/stores/redis/lib.php index 9a6b0576651..796370c3168 100644 --- a/cache/stores/redis/lib.php +++ b/cache/stores/redis/lib.php @@ -53,6 +53,16 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_ */ const COMPRESSOR_PHP_ZSTD = 2; + /** + * @var string Suffix used on key name (for hash) to store the TTL sorted list + */ + const TTL_SUFFIX = '_ttl'; + + /** + * @var int Number of items to delete from cache in one batch when expiring old TTL data. + */ + const TTL_EXPIRE_BATCH = 10000; + /** * Name of this store. * @@ -128,6 +138,10 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_ * @return int */ public static function get_supported_features(array $configuration = array()) { + // Although this plugin now supports TTL I did not add SUPPORTS_NATIVE_TTL here, because + // doing so would cause Moodle to stop adding a 'TTL wrapper' to data items which enforces + // the precise specified TTL. Unless the scheduled task is set to run rather frequently, + // this could cause change in behaviour. Maybe later this should be reconsidered... return self::SUPPORTS_DATA_GUARANTEE + self::DEREFERENCES_OBJECTS + self::IS_SEARCHABLE; } @@ -311,7 +325,17 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_ $value = $this->compress($value); } - return ($this->redis->hSet($this->hash, $key, $value) !== false); + if ($this->redis->hSet($this->hash, $key, $value) === false) { + return false; + } + if ($this->definition->get_ttl()) { + // When TTL is enabled, we also store the key name in a list sorted by the current time. + $this->redis->zAdd($this->hash . self::TTL_SUFFIX, [], self::get_time(), $key); + // The return value to the zAdd function never indicates whether the operation succeeded + // (it returns zero when there was no error if the item is already in the list) so we + // ignore it. + } + return true; } /** @@ -323,6 +347,13 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_ */ public function set_many(array $keyvaluearray) { $pairs = []; + $usettl = false; + if ($this->definition->get_ttl()) { + $usettl = true; + $ttlparams = []; + $now = self::get_time(); + } + foreach ($keyvaluearray as $pair) { $key = $pair['key']; if ($this->compressor != self::COMPRESSOR_NONE) { @@ -330,6 +361,19 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_ } else { $pairs[$key] = $pair['value']; } + if ($usettl) { + // When TTL is enabled, we also store the key names in a list sorted by the current + // time. + $ttlparams[] = $now; + $ttlparams[] = $key; + } + } + if ($usettl) { + // Store all the key values with current time. + $this->redis->zAdd($this->hash . self::TTL_SUFFIX, [], ...$ttlparams); + // The return value to the zAdd function never indicates whether the operation succeeded + // (it returns zero when there was no error if the item is already in the list) so we + // ignore it. } if ($this->redis->hMSet($this->hash, $pairs)) { return count($pairs); @@ -344,7 +388,15 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_ * @return bool True if the delete operation succeeds, false otherwise. */ public function delete($key) { - return ($this->redis->hDel($this->hash, $key) > 0); + $ok = true; + if (!$this->redis->hDel($this->hash, $key)) { + $ok = false; + } + if ($this->definition->get_ttl()) { + // When TTL is enabled, also remove the key from the TTL list. + $this->redis->zRem($this->hash . self::TTL_SUFFIX, $key); + } + return $ok; } /** @@ -354,9 +406,12 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_ * @return int The number of keys successfully deleted. */ public function delete_many(array $keys) { - // Redis needs the hash as the first argument, so we have to put it at the start of the array. - array_unshift($keys, $this->hash); - return call_user_func_array(array($this->redis, 'hDel'), $keys); + $count = $this->redis->hDel($this->hash, ...$keys); + if ($this->definition->get_ttl()) { + // When TTL is enabled, also remove the keys from the TTL list. + $this->redis->zRem($this->hash . self::TTL_SUFFIX, ...$keys); + } + return $count; } /** @@ -365,6 +420,13 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_ * @return bool */ public function purge() { + if ($this->definition->get_ttl()) { + // Purge the TTL list as well. + $this->redis->del($this->hash . self::TTL_SUFFIX); + // According to documentation, there is no error return for the 'del' command (it + // only returns the number of keys deleted, which could be 0 or 1 in this case) so we + // do not need to check the return value. + } return ($this->redis->del($this->hash) !== false); } @@ -493,6 +555,88 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_ return false; } + /** + * Runs TTL expiry process for this cache. + * + * This is not part of the standard cache API and is intended for use by the scheduled task + * \cachestore_redis\ttl. + * + * @return array Various keys with information about how the expiry went + */ + public function expire_ttl(): array { + $ttl = $this->definition->get_ttl(); + if (!$ttl) { + throw new \coding_exception('Cache definition ' . $this->definition->get_id() . ' does not use TTL'); + } + $limit = self::get_time() - $ttl; + $count = 0; + $batches = 0; + $timebefore = microtime(true); + $memorybefore = $this->get_used_memory(); + do { + $keys = $this->redis->zRangeByScore($this->hash . self::TTL_SUFFIX, 0, $limit, + ['limit' => [0, self::TTL_EXPIRE_BATCH]]); + $this->delete_many($keys); + $count += count($keys); + $batches++; + } while (count($keys) === self::TTL_EXPIRE_BATCH); + $memoryafter = $this->get_used_memory(); + $timeafter = microtime(true); + + $result = ['keys' => $count, 'batches' => $batches, 'time' => $timeafter - $timebefore]; + if ($memorybefore !== null) { + $result['memory'] = $memorybefore - $memoryafter; + } + return $result; + } + + /** + * Gets the current time for TTL functionality. This wrapper makes it easier to unit-test + * the TTL behaviour. + * + * @return int Current time + */ + protected static function get_time(): int { + global $CFG; + if (PHPUNIT_TEST && !empty($CFG->phpunit_cachestore_redis_time)) { + return $CFG->phpunit_cachestore_redis_time; + } + return time(); + } + + /** + * Sets the current time (within unit test) for TTL functionality. + * + * This setting is stored in $CFG so will be automatically reset if you use resetAfterTest. + * + * @param int $time Current time (set 0 to start using real time). + */ + public static function set_phpunit_time(int $time = 0): void { + global $CFG; + if (!PHPUNIT_TEST) { + throw new \coding_exception('Function only available during unit test'); + } + if ($time) { + $CFG->phpunit_cachestore_redis_time = $time; + } else { + unset($CFG->phpunit_cachestore_redis_time); + } + } + + /** + * Gets Redis reported memory usage. + * + * @return int|null Memory used by Redis or null if we don't know + */ + protected function get_used_memory(): ?int { + $details = $this->redis->info('MEMORY'); + if (empty($details['used_memory'])) { + return null; + } else { + return (int)$details['used_memory']; + } + } + /** * Creates a configuration array from given 'add instance' form data. * @@ -553,6 +697,13 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_ if (!empty($config->test_password)) { $configuration['password'] = $config->test_password; } + // Make it possible to test TTL performance by hacking a copy of the cache definition. + if (!empty($config->test_ttl)) { + $definition = clone $definition; + $property = (new ReflectionClass($definition))->getProperty('ttl'); + $property->setAccessible(true); + $property->setValue($definition, 999); + } $cache = new cachestore_redis('Redis test', $configuration); $cache->initialise($definition); diff --git a/cache/stores/redis/settings.php b/cache/stores/redis/settings.php index 67f651cfa24..71781a82e47 100644 --- a/cache/stores/redis/settings.php +++ b/cache/stores/redis/settings.php @@ -60,3 +60,9 @@ if (class_exists('Redis')) { // Only if Redis is available. ) ); } + +$settings->add(new admin_setting_configcheckbox( + 'cachestore_redis/test_ttl', + get_string('test_ttl', 'cachestore_redis'), + get_string('test_ttl_desc', 'cachestore_redis'), + false)); diff --git a/cache/stores/redis/tests/ttl_test.php b/cache/stores/redis/tests/ttl_test.php new file mode 100644 index 00000000000..15c20b06f22 --- /dev/null +++ b/cache/stores/redis/tests/ttl_test.php @@ -0,0 +1,113 @@ +. + +namespace cachestore_redis; + +/** + * TTL support test for Redis cache. + * + * If you wish to use these unit tests all you need to do is add the following definition to + * your config.php file. + * + * define('TEST_CACHESTORE_REDIS_TESTSERVERS', '127.0.0.1'); + * + * @package cachestore_redis + * @copyright 2021 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \cachestore_redis + */ +class ttl_test extends \advanced_testcase { + /** @var \cachestore_redis|null Cache store */ + protected $store = null; + + public function setUp(): void { + // Make sure cachestore_redis is available. + require_once(__DIR__ . '/../lib.php'); + if (!\cachestore_redis::are_requirements_met() || !defined('TEST_CACHESTORE_REDIS_TESTSERVERS')) { + $this->markTestSkipped('Could not test cachestore_redis. Requirements are not met.'); + } + + // Set up a Redis store with a fake definition that has TTL set to 10 seconds. + $definition = \cache_definition::load('core/wibble', [ + 'mode' => 1, + 'simplekeys' => true, + 'simpledata' => true, + 'ttl' => 10, + 'component' => 'core', + 'area' => 'wibble', + 'selectedsharingoption' => 2, + 'userinputsharingkey' => '', + 'sharingoptions' => 15, + ]); + $this->store = new \cachestore_redis('Test', \cachestore_redis::unit_test_configuration()); + $this->store->initialise($definition); + + parent::setUp(); + } + + protected function tearDown(): void { + parent::tearDown(); + + if ($this->store instanceof \cachestore_redis) { + $this->store->purge(); + } + } + + /** + * Tests expiring data. + */ + public function test_expire_ttl(): void { + $this->resetAfterTest(); + + // Set some data at time 100. + \cachestore_redis::set_phpunit_time(100); + $this->store->set('a', 1); + $this->store->set('b', 2); + $this->store->set_many([['key' => 'c', 'value' => 3], ['key' => 'd', 'value' => 4], + ['key' => 'e', 'value' => 5], ['key' => 'f', 'value' => 6], + ['key' => 'g', 'value' => 7], ['key' => 'h', 'value' => 8]]); + + // Set some other data at time 110, including some of the existing values. Whether the + // value changes or not, its TTL should update. + \cachestore_redis::set_phpunit_time(110); + $this->store->set('b', 2); + $this->store->set_many([['key' => 'c', 'value' => 99], ['key' => 'd', 'value' => 4]]); + + // Check all the data is still set. + $this->assertEqualsCanonicalizing(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'], + $this->store->find_all()); + + // Delete some data (to check deletion doesn't confuse expiry). + $this->store->delete('f'); + $this->store->delete_many(['g', 'h']); + + // Set time to 115 and expire data. + \cachestore_redis::set_phpunit_time(115); + $info = $this->store->expire_ttl(); + + // We are expecting keys a and e to be deleted. + $this->assertEquals(2, $info['keys']); + $this->assertEquals(1, $info['batches']); + + // Check the keys are as expected. + $this->assertEqualsCanonicalizing(['b', 'c', 'd'], $this->store->find_all()); + + // Might as well check the values of the surviving keys. + $this->assertEquals(2, $this->store->get('b')); + $this->assertEquals(99, $this->store->get('c')); + $this->assertEquals(4, $this->store->get('d')); + } +} diff --git a/cache/stores/redis/version.php b/cache/stores/redis/version.php index 0c71a1a80e9..a291dd4a9e3 100644 --- a/cache/stores/redis/version.php +++ b/cache/stores/redis/version.php @@ -24,7 +24,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2021052500; +$plugin->version = 2021081200; $plugin->requires = 2021052500; // Requires this Moodle version. $plugin->maturity = MATURITY_STABLE; $plugin->component = 'cachestore_redis';