[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
This commit is contained in:
Nicolas Grekas 2018-08-26 12:18:56 +02:00 committed by André R
parent 21d01973f7
commit dacf17962b
2 changed files with 46 additions and 22 deletions

View File

@ -0,0 +1,28 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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;
}
}

View File

@ -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) {