minor #28269 [Cache] improve perf when using RedisCluster by reducing…

… roundtrips to the servers (nicolas-grekas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Cache] improve perf when using RedisCluster by reducing roundtrips to the servers

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Improves perf when using RedisCluster by:
- disabling versioning and replacing by per-master clear/flush
- grouping multiple "get" in one "mget"
- enabling pipelining in PredisCluster mode

~Might need adjustment depending on the answer to predis/predis#520

Commits
-------

5155f48 [Cache] improve perf when using RedisCluster by reducing roundtrips to the servers

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

@@ -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), array('cluster' => 'redis'));
+    }
+
+    public static function tearDownAfterClass()
+    {
+        self::$redis = null;
+    }
+}

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,21 +226,23 @@ 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();
             foreach ($this->redis->_hosts() as $host) {
                 $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) {