From 3d38c58b42a242d1eaadb7905ffedb6dc2e9889c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20R?= Date: Wed, 25 Sep 2019 18:03:38 +0200 Subject: [PATCH] [Cache] Improve RedisTagAwareAdapter invalidation logic & requirements --- .../Cache/Adapter/RedisTagAwareAdapter.php | 117 ++++++++++-------- src/Symfony/Component/Cache/CHANGELOG.md | 2 + .../PredisTagAwareRedisClusterAdapterTest.php | 35 ------ .../Component/Cache/Traits/RedisTrait.php | 35 ++++-- 4 files changed, 92 insertions(+), 97 deletions(-) delete mode 100644 src/Symfony/Component/Cache/Tests/Adapter/PredisTagAwareRedisClusterAdapterTest.php diff --git a/src/Symfony/Component/Cache/Adapter/RedisTagAwareAdapter.php b/src/Symfony/Component/Cache/Adapter/RedisTagAwareAdapter.php index 71230ad69b1f..7bba83429428 100644 --- a/src/Symfony/Component/Cache/Adapter/RedisTagAwareAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/RedisTagAwareAdapter.php @@ -11,33 +11,32 @@ namespace Symfony\Component\Cache\Adapter; -use Predis; use Predis\Connection\Aggregate\ClusterInterface; +use Predis\Connection\Aggregate\PredisCluster; use Predis\Response\Status; -use Symfony\Component\Cache\CacheItem; -use Symfony\Component\Cache\Exception\LogicException; +use Symfony\Component\Cache\Exception\InvalidArgumentException; use Symfony\Component\Cache\Marshaller\MarshallerInterface; use Symfony\Component\Cache\Traits\RedisTrait; /** - * Stores tag id <> cache id relationship as a Redis Set, lookup on invalidation using sPOP. + * Stores tag id <> cache id relationship as a Redis Set, lookup on invalidation using RENAME+SMEMBERS. * * Set (tag relation info) is stored without expiry (non-volatile), while cache always gets an expiry (volatile) even * if not set by caller. Thus if you configure redis with the right eviction policy you can be safe this tag <> cache * relationship survives eviction (cache cleanup when Redis runs out of memory). * * Requirements: - * - Server: Redis 3.2+ - * - Client: PHP Redis 3.1.3+ OR Predis - * - Redis Server(s) configured with any `volatile-*` eviction policy, OR `noeviction` if it will NEVER fill up memory + * - Client: PHP Redis or Predis + * Note: Due to lack of RENAME support it is NOT recommended to use Cluster on Predis, instead use phpredis. + * - Server: Redis 2.8+ + * Configured with any `volatile-*` eviction policy, OR `noeviction` if it will NEVER fill up memory * * Design limitations: - * - Max 2 billion cache keys per cache tag - * E.g. If you use a "all" items tag for expiry instead of clear(), that limits you to 2 billion cache items as well + * - Max 4 billion cache keys per cache tag as limited by Redis Set datatype. + * E.g. If you use a "all" items tag for expiry instead of clear(), that limits you to 4 billion cache items also. * * @see https://redis.io/topics/lru-cache#eviction-policies Documentation for Redis eviction policies. * @see https://redis.io/topics/data-types#sets Documentation for Redis Set datatype. - * @see https://redis.io/commands/spop Documentation for sPOP operation, capable of retriving AND emptying a Set at once. * * @author Nicolas Grekas * @author André Rømcke @@ -46,11 +45,6 @@ class RedisTagAwareAdapter extends AbstractTagAwareAdapter { use RedisTrait; - /** - * Redis "Set" can hold more than 4 billion members, here we limit ourselves to PHP's > 2 billion max int (32Bit). - */ - private const POP_MAX_LIMIT = 2147483647 - 1; - /** * Limits for how many keys are deleted in batch. */ @@ -62,26 +56,18 @@ class RedisTagAwareAdapter extends AbstractTagAwareAdapter */ private const DEFAULT_CACHE_TTL = 8640000; - /** - * @var bool|null - */ - private $redisServerSupportSPOP = null; - /** * @param \Redis|\RedisArray|\RedisCluster|\Predis\ClientInterface $redisClient The redis client * @param string $namespace The default namespace * @param int $defaultLifetime The default lifetime - * - * @throws \Symfony\Component\Cache\Exception\LogicException If phpredis with version lower than 3.1.3. */ public function __construct($redisClient, string $namespace = '', int $defaultLifetime = 0, MarshallerInterface $marshaller = null) { - $this->init($redisClient, $namespace, $defaultLifetime, $marshaller); - - // Make sure php-redis is 3.1.3 or higher configured for Redis classes - if (!$this->redis instanceof \Predis\ClientInterface && version_compare(phpversion('redis'), '3.1.3', '<')) { - throw new LogicException('RedisTagAwareAdapter requires php-redis 3.1.3 or higher, alternatively use predis/predis'); + if ($redisClient instanceof \Predis\ClientInterface && $redisClient->getConnection() instanceof ClusterInterface && !$redisClient->getConnection() instanceof PredisCluster) { + throw new InvalidArgumentException(sprintf('Unsupported Predis cluster connection: only "%s" is, "%s" given.', PredisCluster::class, \get_class($redisClient->getConnection()))); } + + $this->init($redisClient, $namespace, $defaultLifetime, $marshaller); } /** @@ -121,7 +107,7 @@ protected function doSave(array $values, ?int $lifetime, array $addTagData = [], continue; } // setEx results - if (true !== $result && (!$result instanceof Status || $result !== Status::get('OK'))) { + if (true !== $result && (!$result instanceof Status || Status::get('OK') !== $result)) { $failed[] = $id; } } @@ -138,9 +124,10 @@ protected function doDelete(array $ids, array $tagData = []): bool return true; } - $predisCluster = $this->redis instanceof \Predis\ClientInterface && $this->redis->getConnection() instanceof ClusterInterface; + $predisCluster = $this->redis instanceof \Predis\ClientInterface && $this->redis->getConnection() instanceof PredisCluster; $this->pipeline(static function () use ($ids, $tagData, $predisCluster) { if ($predisCluster) { + // Unlike phpredis, Predis does not handle bulk calls for us against cluster foreach ($ids as $id) { yield 'del' => [$id]; } @@ -161,46 +148,76 @@ protected function doDelete(array $ids, array $tagData = []): bool */ protected function doInvalidate(array $tagIds): bool { - if (!$this->redisServerSupportSPOP()) { + if (!$this->redis instanceof \Predis\ClientInterface || !$this->redis->getConnection() instanceof PredisCluster) { + $movedTagSetIds = $this->renameKeys($this->redis, $tagIds); + } else { + $clusterConnection = $this->redis->getConnection(); + $tagIdsByConnection = new \SplObjectStorage(); + $movedTagSetIds = []; + + foreach ($tagIds as $id) { + $connection = $clusterConnection->getConnectionByKey($id); + $slot = $tagIdsByConnection[$connection] ?? $tagIdsByConnection[$connection] = new \ArrayObject(); + $slot[] = $id; + } + + foreach ($tagIdsByConnection as $connection) { + $slot = $tagIdsByConnection[$connection]; + $movedTagSetIds = array_merge($movedTagSetIds, $this->renameKeys(new $this->redis($connection, $this->redis->getOptions()), $slot->getArrayCopy())); + } + } + + // No Sets found + if (!$movedTagSetIds) { return false; } - // Pop all tag info at once to avoid race conditions - $tagIdSets = $this->pipeline(static function () use ($tagIds) { - foreach ($tagIds as $tagId) { - // Client: Predis or PHP Redis 3.1.3+ (https://github.com/phpredis/phpredis/commit/d2e203a6) - // Server: Redis 3.2 or higher (https://redis.io/commands/spop) - yield 'sPop' => [$tagId, self::POP_MAX_LIMIT]; + // Now safely take the time to read the keys in each set and collect ids we need to delete + $tagIdSets = $this->pipeline(static function () use ($movedTagSetIds) { + foreach ($movedTagSetIds as $movedTagId) { + yield 'sMembers' => [$movedTagId]; } }); - // Flatten generator result from pipeline, ignore keys (tag ids) - $ids = array_unique(array_merge(...iterator_to_array($tagIdSets, false))); + // Return combination of the temporary Tag Set ids and their values (cache ids) + $ids = array_merge($movedTagSetIds, ...iterator_to_array($tagIdSets, false)); // Delete cache in chunks to avoid overloading the connection - foreach (array_chunk($ids, self::BULK_DELETE_LIMIT) as $chunkIds) { + foreach (array_chunk(array_unique($ids), self::BULK_DELETE_LIMIT) as $chunkIds) { $this->doDelete($chunkIds); } return true; } - private function redisServerSupportSPOP(): bool + /** + * Renames several keys in order to be able to operate on them without risk of race conditions. + * + * Filters out keys that do not exist before returning new keys. + * + * @see https://redis.io/commands/rename + * @see https://redis.io/topics/cluster-spec#keys-hash-tags + * + * @return array Filtered list of the valid moved keys (only those that existed) + */ + private function renameKeys($redis, array $ids): array { - if (null !== $this->redisServerSupportSPOP) { - return $this->redisServerSupportSPOP; - } + $newIds = []; + $uniqueToken = bin2hex(random_bytes(10)); - foreach ($this->getHosts() as $host) { - $info = $host->info('Server'); - $info = isset($info['Server']) ? $info['Server'] : $info; - if (version_compare($info['redis_version'], '3.2', '<')) { - CacheItem::log($this->logger, 'Redis server needs to be version 3.2 or higher, your Redis server was detected as '.$info['redis_version']); + $results = $this->pipeline(static function () use ($ids, $uniqueToken) { + foreach ($ids as $id) { + yield 'rename' => [$id, '{'.$id.'}'.$uniqueToken]; + } + }, $redis); - return $this->redisServerSupportSPOP = false; + foreach ($results as $id => $result) { + if (true === $result || ($result instanceof Status && Status::get('OK') === $result)) { + // Only take into account if ok (key existed), will be false on phpredis if it did not exist + $newIds[] = '{'.$id.'}'.$uniqueToken; } } - return $this->redisServerSupportSPOP = true; + return $newIds; } } diff --git a/src/Symfony/Component/Cache/CHANGELOG.md b/src/Symfony/Component/Cache/CHANGELOG.md index 136953fe27f9..bbe316830b56 100644 --- a/src/Symfony/Component/Cache/CHANGELOG.md +++ b/src/Symfony/Component/Cache/CHANGELOG.md @@ -6,6 +6,8 @@ CHANGELOG * added support for connecting to Redis Sentinel clusters * added argument `$prefix` to `AdapterInterface::clear()` + * improved `RedisTagAwareAdapter` to support Redis server >= 2.8 and up to 4B items per tag + * [BC BREAK] `RedisTagAwareAdapter` is not compatible with `RedisCluster` from `Predis` anymore, use `phpredis` instead 4.3.0 ----- diff --git a/src/Symfony/Component/Cache/Tests/Adapter/PredisTagAwareRedisClusterAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/PredisTagAwareRedisClusterAdapterTest.php deleted file mode 100644 index 8357fffe39ea..000000000000 --- a/src/Symfony/Component/Cache/Tests/Adapter/PredisTagAwareRedisClusterAdapterTest.php +++ /dev/null @@ -1,35 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Cache\Tests\Adapter; - -use Psr\Cache\CacheItemPoolInterface; -use Symfony\Component\Cache\Adapter\RedisTagAwareAdapter; -use Symfony\Component\Cache\Tests\Traits\TagAwareTestTrait; - -class PredisTagAwareRedisClusterAdapterTest extends PredisRedisClusterAdapterTest -{ - use TagAwareTestTrait; - - protected function setUp(): void - { - parent::setUp(); - $this->skippedTests['testTagItemExpiry'] = 'Testing expiration slows down the test suite'; - } - - public function createCachePool(int $defaultLifetime = 0): CacheItemPoolInterface - { - $this->assertInstanceOf(\Predis\Client::class, self::$redis); - $adapter = new RedisTagAwareAdapter(self::$redis, str_replace('\\', '.', __CLASS__), $defaultLifetime); - - return $adapter; - } -} diff --git a/src/Symfony/Component/Cache/Traits/RedisTrait.php b/src/Symfony/Component/Cache/Traits/RedisTrait.php index f3e7a072d402..fbc8f6c69fc8 100644 --- a/src/Symfony/Component/Cache/Traits/RedisTrait.php +++ b/src/Symfony/Component/Cache/Traits/RedisTrait.php @@ -55,9 +55,17 @@ private function init($redisClient, string $namespace, int $defaultLifetime, ?Ma if (preg_match('#[^-+_.A-Za-z0-9]#', $namespace, $match)) { throw new InvalidArgumentException(sprintf('RedisAdapter namespace contains "%s" but only characters in [-+_.A-Za-z0-9] are allowed.', $match[0])); } + if (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \RedisCluster && !$redisClient instanceof \Predis\ClientInterface && !$redisClient instanceof RedisProxy && !$redisClient instanceof RedisClusterProxy) { throw new InvalidArgumentException(sprintf('%s() expects parameter 1 to be Redis, RedisArray, RedisCluster or Predis\ClientInterface, %s given.', __METHOD__, \is_object($redisClient) ? \get_class($redisClient) : \gettype($redisClient))); } + + if ($redisClient instanceof \Predis\ClientInterface && $redisClient->getOptions()->exceptions) { + $options = clone $redisClient->getOptions(); + \Closure::bind(function () { $this->options['exceptions'] = false; }, $options, $options)(); + $redisClient = new $redisClient($redisClient->getConnection(), $options); + } + $this->redis = $redisClient; $this->marshaller = $marshaller ?? new DefaultMarshaller(); } @@ -277,6 +285,7 @@ public static function createConnection($dsn, array $options = []) $params['replication'] = true; $hosts[0] += ['alias' => 'master']; } + $params['exceptions'] = false; $redis = new $class($hosts, array_diff_key($params, self::$defaultConnectionOptions)); if (isset($params['redis_sentinel'])) { @@ -414,8 +423,9 @@ protected function doSave(array $values, $lifetime) } } }); + foreach ($results as $id => $result) { - if (true !== $result && (!$result instanceof Status || $result !== Status::get('OK'))) { + if (true !== $result && (!$result instanceof Status || Status::get('OK') !== $result)) { $failed[] = $id; } } @@ -423,31 +433,32 @@ protected function doSave(array $values, $lifetime) return $failed; } - private function pipeline(\Closure $generator): \Generator + private function pipeline(\Closure $generator, $redis = null): \Generator { $ids = []; + $redis = $redis ?? $this->redis; - if ($this->redis instanceof RedisClusterProxy || $this->redis instanceof \RedisCluster || ($this->redis instanceof \Predis\ClientInterface && $this->redis->getConnection() instanceof RedisCluster)) { + if ($redis instanceof RedisClusterProxy || $redis instanceof \RedisCluster || ($redis instanceof \Predis\ClientInterface && $redis->getConnection() instanceof RedisCluster)) { // phpredis & predis don't support pipelining with RedisCluster // see https://github.com/phpredis/phpredis/blob/develop/cluster.markdown#pipelining // see https://github.com/nrk/predis/issues/267#issuecomment-123781423 $results = []; foreach ($generator() as $command => $args) { - $results[] = $this->redis->{$command}(...$args); + $results[] = $redis->{$command}(...$args); $ids[] = $args[0]; } - } elseif ($this->redis instanceof \Predis\ClientInterface) { - $results = $this->redis->pipeline(function ($redis) use ($generator, &$ids) { + } elseif ($redis instanceof \Predis\ClientInterface) { + $results = $redis->pipeline(static function ($redis) use ($generator, &$ids) { foreach ($generator() as $command => $args) { $redis->{$command}(...$args); $ids[] = $args[0]; } }); - } elseif ($this->redis instanceof \RedisArray) { + } elseif ($redis instanceof \RedisArray) { $connections = $results = $ids = []; foreach ($generator() as $command => $args) { - if (!isset($connections[$h = $this->redis->_target($args[0])])) { - $connections[$h] = [$this->redis->_instance($h), -1]; + if (!isset($connections[$h = $redis->_target($args[0])])) { + $connections[$h] = [$redis->_instance($h), -1]; $connections[$h][0]->multi(\Redis::PIPELINE); } $connections[$h][0]->{$command}(...$args); @@ -461,12 +472,12 @@ private function pipeline(\Closure $generator): \Generator $results[$k] = $connections[$h][$c]; } } else { - $this->redis->multi(\Redis::PIPELINE); + $redis->multi(\Redis::PIPELINE); foreach ($generator() as $command => $args) { - $this->redis->{$command}(...$args); + $redis->{$command}(...$args); $ids[] = $args[0]; } - $results = $this->redis->exec(); + $results = $redis->exec(); } foreach ($ids as $k => $id) {