From 5155f48029990b7bc94adb46412ea4ac60fa383f Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 26 Aug 2018 12:18:56 +0200 Subject: [PATCH] [Cache] improve perf when using RedisCluster by reducing roundtrips to the servers --- .../Adapter/PredisRedisClusterAdapterTest.php | 28 +++++++ .../Component/Cache/Traits/RedisTrait.php | 74 ++++++++++++------- 2 files changed, 74 insertions(+), 28 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 000000000000..881c44f01a42 --- /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), array('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 59ddeff2cc6f..72050b6ef0f1 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; @@ -53,9 +52,7 @@ private function init($redisClient, $namespace, $defaultLifetime, ?MarshallerInt 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; @@ -182,18 +179,30 @@ public static function createConnection($dsn, array $options = array()) */ protected function doFetch(array $ids) { - if ($ids) { + if (!$ids) { + return array(); + } + + $i = -1; + $result = array(); + + if ($this->redis instanceof \Predis\Client) { $values = $this->pipeline(function () use ($ids) { foreach ($ids as $id) { yield 'get' => array($id); } }); - foreach ($values as $id => $v) { - if ($v) { - yield $id => $this->marshaller->unmarshall($v); - } + } else { + $values = array_combine($ids, $this->redis->mget($ids)); + } + + foreach ($values as $id => $v) { + if ($v) { + $result[$id] = $this->marshaller->unmarshall($v); } } + + return $result; } /** @@ -209,9 +218,6 @@ protected function doHave($id) */ 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 = array($this->redis); $evalArgs = array(array($namespace), 0); @@ -220,13 +226,11 @@ protected function doClear($namespace) $evalArgs = array(0, $namespace); $connection = $this->redis->getConnection(); - if ($connection instanceof PredisCluster) { + if ($connection instanceof ClusterInterface && $connection instanceof \Traversable) { $hosts = array(); foreach ($connection as $c) { $hosts[] = new \Predis\Client($c); } - } elseif ($connection instanceof RedisCluster) { - return false; } } elseif ($this->redis instanceof \RedisArray) { $hosts = array(); @@ -234,7 +238,11 @@ protected function doClear($namespace) $hosts[] = $this->redis->_instance($host); } } elseif ($this->redis instanceof \RedisCluster) { - return false; + $hosts = array(); + foreach ($this->redis->_masters() as $host) { + $hosts[] = $h = new \Redis(); + $h->connect($host[0], $host[1]); + } } foreach ($hosts as $host) { if (!isset($namespace[0])) { @@ -261,7 +269,7 @@ protected function doClear($namespace) $keys = $keys[1]; } if ($keys) { - $host->del($keys); + $this->doDelete($keys); } } while ($cursor = (int) $cursor); } @@ -274,7 +282,17 @@ protected function doClear($namespace) */ protected function doDelete(array $ids) { - if ($ids) { + if (!$ids) { + return true; + } + + if ($this->redis instanceof \Predis\Client) { + $this->pipeline(function () use ($ids) { + foreach ($ids as $id) { + yield 'del' => array($id); + } + })->rewind(); + } else { $this->redis->del($ids); } @@ -312,7 +330,16 @@ private function pipeline(\Closure $generator) { $ids = array(); - 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 = array(); + foreach ($generator() as $command => $args) { + $results[] = \call_user_func_array(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(array($redis, $command), $args); @@ -336,15 +363,6 @@ private function pipeline(\Closure $generator) 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 = array(); - foreach ($generator() as $command => $args) { - $results[] = \call_user_func_array(array($this->redis, $command), $args); - $ids[] = $args[0]; - } } else { $this->redis->multi(\Redis::PIPELINE); foreach ($generator() as $command => $args) {