Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cache] Fix perf when using RedisCluster by reducing roundtrips to the servers #30518

Merged
merged 1 commit into from Mar 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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;
}
}
40 changes: 18 additions & 22 deletions src/Symfony/Component/Cache/Traits/RedisTrait.php
Expand Up @@ -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;
Expand Down Expand Up @@ -48,9 +47,7 @@ private function init($redisClient, $namespace = '', $defaultLifetime = 0)
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;
Expand Down Expand Up @@ -207,9 +204,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 = [$this->redis];
$evalArgs = [[$namespace], 0];
Expand All @@ -218,21 +212,23 @@ protected function doClear($namespace)
$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 = [];
foreach ($this->redis->_hosts() as $host) {
$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])) {
Expand All @@ -259,7 +255,7 @@ protected function doClear($namespace)
$keys = $keys[1];
}
if ($keys) {
$host->del($keys);
$this->doDelete($keys);
}
} while ($cursor = (int) $cursor);
}
Expand Down Expand Up @@ -331,7 +327,16 @@ private function pipeline(\Closure $generator)
{
$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);
Expand All @@ -355,15 +360,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 = [];
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) {
Expand Down