From dacf17962b2ad89ec94f4bf2c06255ceee4ea821 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 26 Aug 2018 12:18:56 +0200 Subject: [PATCH] [Cache] Fix perf when using RedisCluster by reducing roundtrips to the servers This is slimmed down version of: https://github.com/symfony/symfony/pull/28269 _(many of the fixes here are already part of 3.4)_ Adds: - Test coverage for Predis with RedisCluster - Removes usage of key versioning when on RedisCluster, besides performance aspect of that simplify / aligning clear() handling across cases --- .../Adapter/PredisRedisClusterAdapterTest.php | 28 +++++++++++++ .../Component/Cache/Traits/RedisTrait.php | 40 +++++++++---------- 2 files changed, 46 insertions(+), 22 deletions(-) create mode 100644 src/Symfony/Component/Cache/Tests/Adapter/PredisRedisClusterAdapterTest.php diff --git a/src/Symfony/Component/Cache/Tests/Adapter/PredisRedisClusterAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/PredisRedisClusterAdapterTest.php new file mode 100644 index 0000000000..6bf0348a1e --- /dev/null +++ b/src/Symfony/Component/Cache/Tests/Adapter/PredisRedisClusterAdapterTest.php @@ -0,0 +1,28 @@ + + * + * 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; + +class PredisRedisClusterAdapterTest extends AbstractRedisAdapterTest +{ + public static function setupBeforeClass() + { + if (!$hosts = getenv('REDIS_CLUSTER_HOSTS')) { + self::markTestSkipped('REDIS_CLUSTER_HOSTS env var is not defined.'); + } + self::$redis = new \Predis\Client(explode(' ', $hosts), ['cluster' => 'redis']); + } + + public static function tearDownAfterClass() + { + self::$redis = null; + } +} diff --git a/src/Symfony/Component/Cache/Traits/RedisTrait.php b/src/Symfony/Component/Cache/Traits/RedisTrait.php index 7f5d191692..395a9b8051 100644 --- a/src/Symfony/Component/Cache/Traits/RedisTrait.php +++ b/src/Symfony/Component/Cache/Traits/RedisTrait.php @@ -12,7 +12,6 @@ namespace Symfony\Component\Cache\Traits; use Predis\Connection\Aggregate\ClusterInterface; -use Predis\Connection\Aggregate\PredisCluster; use Predis\Connection\Aggregate\RedisCluster; use Predis\Connection\Factory; use Predis\Response\Status; @@ -48,9 +47,7 @@ trait RedisTrait 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 \RedisCluster) { - $this->enableVersioning(); - } elseif (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \Predis\Client && !$redisClient instanceof RedisProxy) { + if (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \RedisCluster && !$redisClient instanceof \Predis\Client && !$redisClient instanceof RedisProxy) { throw new InvalidArgumentException(sprintf('%s() expects parameter 1 to be Redis, RedisArray, RedisCluster or Predis\Client, %s given', __METHOD__, \is_object($redisClient) ? \get_class($redisClient) : \gettype($redisClient))); } $this->redis = $redisClient; @@ -207,9 +204,6 @@ trait RedisTrait */ protected function doClear($namespace) { - // When using a native Redis cluster, clearing the cache is done by versioning in AbstractTrait::clear(). - // This means old keys are not really removed until they expire and may need garbage collection. - $cleared = true; $hosts = [$this->redis]; $evalArgs = [[$namespace], 0]; @@ -218,13 +212,11 @@ trait RedisTrait $evalArgs = [0, $namespace]; $connection = $this->redis->getConnection(); - if ($connection instanceof PredisCluster) { + if ($connection instanceof ClusterInterface && $connection instanceof \Traversable) { $hosts = []; foreach ($connection as $c) { $hosts[] = new \Predis\Client($c); } - } elseif ($connection instanceof RedisCluster) { - return false; } } elseif ($this->redis instanceof \RedisArray) { $hosts = []; @@ -232,7 +224,11 @@ trait RedisTrait $hosts[] = $this->redis->_instance($host); } } elseif ($this->redis instanceof \RedisCluster) { - return false; + $hosts = []; + foreach ($this->redis->_masters() as $host) { + $hosts[] = $h = new \Redis(); + $h->connect($host[0], $host[1]); + } } foreach ($hosts as $host) { if (!isset($namespace[0])) { @@ -259,7 +255,7 @@ trait RedisTrait $keys = $keys[1]; } if ($keys) { - $host->del($keys); + $this->doDelete($keys); } } while ($cursor = (int) $cursor); } @@ -331,7 +327,16 @@ trait RedisTrait { $ids = []; - if ($this->redis instanceof \Predis\Client && !$this->redis->getConnection() instanceof ClusterInterface) { + if ($this->redis instanceof \RedisCluster || ($this->redis instanceof \Predis\Client && $this->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[] = \call_user_func_array([$this->redis, $command], $args); + $ids[] = $args[0]; + } + } elseif ($this->redis instanceof \Predis\Client) { $results = $this->redis->pipeline(function ($redis) use ($generator, &$ids) { foreach ($generator() as $command => $args) { \call_user_func_array([$redis, $command], $args); @@ -355,15 +360,6 @@ trait RedisTrait foreach ($results as $k => list($h, $c)) { $results[$k] = $connections[$h][$c]; } - } elseif ($this->redis instanceof \RedisCluster || ($this->redis instanceof \Predis\Client && $this->redis->getConnection() instanceof ClusterInterface)) { - // 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[] = \call_user_func_array([$this->redis, $command], $args); - $ids[] = $args[0]; - } } else { $this->redis->multi(\Redis::PIPELINE); foreach ($generator() as $command => $args) {