Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 95e2e44

Browse filesBrowse files
nicolas-grekasandrerom
nicolas-grekas
authored and
andrerom
committed
[Cache] Fix perf when using RedisCluster by reducing roundtrips to the servers
This is slimmed down version of: symfony#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
1 parent 21d0197 commit 95e2e44
Copy full SHA for 95e2e44

File tree

2 files changed

+46
-22
lines changed
Filter options

2 files changed

+46
-22
lines changed

‎src/Symfony/Component/Cache/Tests/Adapter/PredisRedisClusterAdapterTest.php

Copy file name to clipboard
+28Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Cache\Tests\Adapter;
13+
14+
class PredisRedisClusterAdapterTest extends AbstractRedisAdapterTest
15+
{
16+
public static function setupBeforeClass()
17+
{
18+
if (!$hosts = getenv('REDIS_CLUSTER_HOSTS')) {
19+
self::markTestSkipped('REDIS_CLUSTER_HOSTS env var is not defined.');
20+
}
21+
self::$redis = new \Predis\Client(explode(' ', $hosts), array('cluster' => 'redis'));
22+
}
23+
24+
public static function tearDownAfterClass()
25+
{
26+
self::$redis = null;
27+
}
28+
}

‎src/Symfony/Component/Cache/Traits/RedisTrait.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Cache/Traits/RedisTrait.php
+18-22Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
namespace Symfony\Component\Cache\Traits;
1313

1414
use Predis\Connection\Aggregate\ClusterInterface;
15-
use Predis\Connection\Aggregate\PredisCluster;
1615
use Predis\Connection\Aggregate\RedisCluster;
1716
use Predis\Connection\Factory;
1817
use Predis\Response\Status;
@@ -48,9 +47,7 @@ private function init($redisClient, $namespace = '', $defaultLifetime = 0)
4847
if (preg_match('#[^-+_.A-Za-z0-9]#', $namespace, $match)) {
4948
throw new InvalidArgumentException(sprintf('RedisAdapter namespace contains "%s" but only characters in [-+_.A-Za-z0-9] are allowed.', $match[0]));
5049
}
51-
if ($redisClient instanceof \RedisCluster) {
52-
$this->enableVersioning();
53-
} elseif (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \Predis\Client && !$redisClient instanceof RedisProxy) {
50+
if (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \RedisCluster && !$redisClient instanceof \Predis\Client && !$redisClient instanceof RedisProxy) {
5451
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)));
5552
}
5653
$this->redis = $redisClient;
@@ -207,9 +204,6 @@ protected function doHave($id)
207204
*/
208205
protected function doClear($namespace)
209206
{
210-
// When using a native Redis cluster, clearing the cache is done by versioning in AbstractTrait::clear().
211-
// This means old keys are not really removed until they expire and may need garbage collection.
212-
213207
$cleared = true;
214208
$hosts = [$this->redis];
215209
$evalArgs = [[$namespace], 0];
@@ -218,21 +212,23 @@ protected function doClear($namespace)
218212
$evalArgs = [0, $namespace];
219213

220214
$connection = $this->redis->getConnection();
221-
if ($connection instanceof PredisCluster) {
215+
if ($connection instanceof ClusterInterface && $connection instanceof \Traversable) {
222216
$hosts = [];
223217
foreach ($connection as $c) {
224218
$hosts[] = new \Predis\Client($c);
225219
}
226-
} elseif ($connection instanceof RedisCluster) {
227-
return false;
228220
}
229221
} elseif ($this->redis instanceof \RedisArray) {
230222
$hosts = [];
231223
foreach ($this->redis->_hosts() as $host) {
232224
$hosts[] = $this->redis->_instance($host);
233225
}
234226
} elseif ($this->redis instanceof \RedisCluster) {
235-
return false;
227+
$hosts = array();
228+
foreach ($this->redis->_masters() as $host) {
229+
$hosts[] = $h = new \Redis();
230+
$h->connect($host[0], $host[1]);
231+
}
236232
}
237233
foreach ($hosts as $host) {
238234
if (!isset($namespace[0])) {
@@ -259,7 +255,7 @@ protected function doClear($namespace)
259255
$keys = $keys[1];
260256
}
261257
if ($keys) {
262-
$host->del($keys);
258+
$this->doDelete($keys);
263259
}
264260
} while ($cursor = (int) $cursor);
265261
}
@@ -331,7 +327,16 @@ private function pipeline(\Closure $generator)
331327
{
332328
$ids = [];
333329

334-
if ($this->redis instanceof \Predis\Client && !$this->redis->getConnection() instanceof ClusterInterface) {
330+
if ($this->redis instanceof \RedisCluster || ($this->redis instanceof \Predis\Client && $this->redis->getConnection() instanceof RedisCluster)) {
331+
// phpredis & predis don't support pipelining with RedisCluster
332+
// see https://github.com/phpredis/phpredis/blob/develop/cluster.markdown#pipelining
333+
// see https://github.com/nrk/predis/issues/267#issuecomment-123781423
334+
$results = array();
335+
foreach ($generator() as $command => $args) {
336+
$results[] = \call_user_func_array(array($this->redis, $command), $args);
337+
$ids[] = $args[0];
338+
}
339+
} elseif ($this->redis instanceof \Predis\Client) {
335340
$results = $this->redis->pipeline(function ($redis) use ($generator, &$ids) {
336341
foreach ($generator() as $command => $args) {
337342
\call_user_func_array([$redis, $command], $args);
@@ -355,15 +360,6 @@ private function pipeline(\Closure $generator)
355360
foreach ($results as $k => list($h, $c)) {
356361
$results[$k] = $connections[$h][$c];
357362
}
358-
} elseif ($this->redis instanceof \RedisCluster || ($this->redis instanceof \Predis\Client && $this->redis->getConnection() instanceof ClusterInterface)) {
359-
// phpredis & predis don't support pipelining with RedisCluster
360-
// see https://github.com/phpredis/phpredis/blob/develop/cluster.markdown#pipelining
361-
// see https://github.com/nrk/predis/issues/267#issuecomment-123781423
362-
$results = [];
363-
foreach ($generator() as $command => $args) {
364-
$results[] = \call_user_func_array([$this->redis, $command], $args);
365-
$ids[] = $args[0];
366-
}
367363
} else {
368364
$this->redis->multi(\Redis::PIPELINE);
369365
foreach ($generator() as $command => $args) {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.