From dacf17962b2ad89ec94f4bf2c06255ceee4ea821 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 26 Aug 2018 12:18:56 +0200 Subject: [PATCH 1/7] [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 --- .../Adapter/PredisRedisClusterAdapterTest.php | 28 +++++++++++++ .../Component/Cache/Traits/RedisTrait.php | 40 +++++++++---------- 2 files changed, 46 insertions(+), 22 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 0000000000..6bf0348a1e --- /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), ['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 7f5d191692..395a9b8051 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; @@ -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) { From bd92349a3e0fef6a9f5c88072080ad30fe2b3b07 Mon Sep 17 00:00:00 2001 From: Teoh Han Hui Date: Tue, 12 Mar 2019 15:29:13 +0100 Subject: [PATCH 2/7] Fix return type of Request::getRequestFormat --- src/Symfony/Component/HttpFoundation/Request.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index f6ff0f5099..fe1a33a597 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -1462,7 +1462,7 @@ class Request * * @param string|null $default The default format * - * @return string The request format + * @return string|null The request format */ public function getRequestFormat($default = 'html') { From b86fa9312b08b050d4f2e61c091f6655c53de948 Mon Sep 17 00:00:00 2001 From: Stanislav Kocanda Date: Wed, 13 Mar 2019 15:17:50 +0100 Subject: [PATCH 3/7] Correct language code for ukrainian language in security translations. --- .../Resources/translations/{security.ua.xlf => security.uk.xlf} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/Symfony/Component/Security/Core/Resources/translations/{security.ua.xlf => security.uk.xlf} (100%) diff --git a/src/Symfony/Component/Security/Core/Resources/translations/security.ua.xlf b/src/Symfony/Component/Security/Core/Resources/translations/security.uk.xlf similarity index 100% rename from src/Symfony/Component/Security/Core/Resources/translations/security.ua.xlf rename to src/Symfony/Component/Security/Core/Resources/translations/security.uk.xlf From d15c76cd53efeb7cd54191e21f375c0b30ea0cc5 Mon Sep 17 00:00:00 2001 From: Simeon Kolev Date: Wed, 13 Mar 2019 17:31:25 +0200 Subject: [PATCH 4/7] Make translations consistent with other translations. --- .../Resources/translations/validators.bg.xlf | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/Symfony/Component/Validator/Resources/translations/validators.bg.xlf b/src/Symfony/Component/Validator/Resources/translations/validators.bg.xlf index bd09da5d0e..1db88086cf 100644 --- a/src/Symfony/Component/Validator/Resources/translations/validators.bg.xlf +++ b/src/Symfony/Component/Validator/Resources/translations/validators.bg.xlf @@ -36,23 +36,23 @@ This field was not expected. - Това поле не се е очаквало. + Полето не се е очаквало. This field is missing. - Това поле липсва. + Полето липсва. This value is not a valid date. - Стойността не е валидна дата (date). + Стойността не е валидна дата. This value is not a valid datetime. - Стойността не е валидна дата (datetime). + Стойността не е валидна дата и час. This value is not a valid email address. - Стойността не е валиден email адрес. + Стойността не е валиден имейл адрес. The file could not be found. @@ -68,7 +68,7 @@ The mime type of the file is invalid ({{ type }}). Allowed mime types are {{ types }}. - Майм типа на файла е невалиден ({{ type }}). Разрешени майм типове са {{ types }}. + Mime типа на файла е невалиден ({{ type }}). Разрешени mime типове са {{ types }}. This value should be {{ limit }} or less. @@ -104,7 +104,7 @@ This value is not a valid time. - Стойността не е валидно време (time). + Стойността не е валидно време. This value is not a valid URL. @@ -216,35 +216,35 @@ Invalid card number. - Невалиден номер на картата. + Невалиден номер на карта. Unsupported card type or invalid card number. - Неподдържан тип карта или невалиден номер на картата. + Неподдържан тип карта или невалиден номер на карта. This is not a valid International Bank Account Number (IBAN). - Невалиден Международен номер на банкова сметка (IBAN). + Това не е валиден Международен номер на банкова сметка (IBAN). This value is not a valid ISBN-10. - Невалиден ISBN-10. + Стойността не е валиден ISBN-10. This value is not a valid ISBN-13. - Невалиден ISBN-13. + Стойността не е валиден ISBN-13. This value is neither a valid ISBN-10 nor a valid ISBN-13. - Невалидна стойност както за ISBN-10, така и за ISBN-13 . + Стойността не е нито валиден ISBN-10, нито валиден ISBN-13. This value is not a valid ISSN. - Невалиден Международен стандартен сериен номер (ISSN). + Стойността не е валиден ISSN. This value is not a valid currency. - Невалидна валута. + Стойността не е валидна валута. This value should be equal to {{ compared_value }}. @@ -308,11 +308,11 @@ This value does not match the expected {{ charset }} charset. - Стойността не съвпада с {{ charset }}. + Стойността не съвпада с очакваната {{ charset }} кодировка. This is not a valid Business Identifier Code (BIC). - Невалиден бизнес идентификационен код (BIC). + Това не е валиден Бизнес идентификационен код (BIC). Error @@ -324,15 +324,15 @@ This value should be a multiple of {{ compared_value }}. - Тази стойност трябва да бъде кратно число на {{ compared_value }}. + Стойността трябва да бъде кратно число на {{ compared_value }}. This Business Identifier Code (BIC) is not associated with IBAN {{ iban }}. - Този бизнес идентификационен код (BIC) не е свързана с IBAN {{ iban }}. + Бизнес идентификационния код (BIC) не е свързан с IBAN {{ iban }}. This value should be valid JSON. - Тази стойност трябва да е валидна JSON. + Стойността трябва да е валиден JSON. From 5f49e6c4d56af8f0e7058ac8fe75b58ed564a6d8 Mon Sep 17 00:00:00 2001 From: Dennis Fridrich Date: Thu, 14 Mar 2019 11:33:34 +0100 Subject: [PATCH 5/7] Update validators.cs.xlf --- .../Resources/translations/validators.cs.xlf | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Symfony/Component/Validator/Resources/translations/validators.cs.xlf b/src/Symfony/Component/Validator/Resources/translations/validators.cs.xlf index 4b966698f7..e637d09aa9 100644 --- a/src/Symfony/Component/Validator/Resources/translations/validators.cs.xlf +++ b/src/Symfony/Component/Validator/Resources/translations/validators.cs.xlf @@ -318,6 +318,22 @@ Error Chyba + + This is not a valid UUID. + Tato hodnota není platné UUID. + + + This value should be a multiple of {{ compared_value }}. + Tato hodnota musí být násobek hodnoty {{ compared_value }}. + + + This Business Identifier Code (BIC) is not associated with IBAN {{ iban }}. + Bankovní identifikační kód (BIC) neodpovídá mezinárodnímu číslu účtu (IBAN) {{ iban }}. + + + This value should be valid JSON. + Tato hodnota musí být validní JSON. + From f49df4ab0559a11c45a952a7fe976438d15a3f3d Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 14 Mar 2019 22:54:25 +0100 Subject: [PATCH 6/7] [Cache] fix LockRegistry --- src/Symfony/Component/Cache/LockRegistry.php | 19 +++++++++++++++---- .../Component/Cache/Traits/ContractsTrait.php | 18 +++++++++++------- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/Symfony/Component/Cache/LockRegistry.php b/src/Symfony/Component/Cache/LockRegistry.php index e0318e900c..ef49805ea0 100644 --- a/src/Symfony/Component/Cache/LockRegistry.php +++ b/src/Symfony/Component/Cache/LockRegistry.php @@ -23,7 +23,7 @@ use Symfony\Contracts\Cache\ItemInterface; * * @author Nicolas Grekas */ -class LockRegistry +final class LockRegistry { private static $openedFiles = []; private static $lockedFiles = []; @@ -74,7 +74,7 @@ class LockRegistry return $previousFiles; } - public static function compute(callable $callback, ItemInterface $item, bool &$save, CacheInterface $pool) + public static function compute(callable $callback, ItemInterface $item, bool &$save, CacheInterface $pool, \Closure $setMetadata = null) { $key = self::$files ? crc32($item->getKey()) % \count(self::$files) : -1; @@ -88,7 +88,18 @@ class LockRegistry if (flock($lock, LOCK_EX | LOCK_NB)) { self::$lockedFiles[$key] = true; - return $callback($item, $save); + $value = $callback($item, $save); + + if ($save) { + if ($setMetadata) { + $setMetadata($item); + } + + $pool->save($item->set($value)); + $save = false; + } + + return $value; } // if we failed the race, retry locking in blocking mode to wait for the winner flock($lock, LOCK_SH); @@ -125,6 +136,6 @@ class LockRegistry restore_error_handler(); } - self::$openedFiles[$key] = $h ?: @fopen(self::$files[$key], 'r'); + return self::$openedFiles[$key] = $h ?: @fopen(self::$files[$key], 'r'); } } diff --git a/src/Symfony/Component/Cache/Traits/ContractsTrait.php b/src/Symfony/Component/Cache/Traits/ContractsTrait.php index 8b1eb5a244..bd7be08dd5 100644 --- a/src/Symfony/Component/Cache/Traits/ContractsTrait.php +++ b/src/Symfony/Component/Cache/Traits/ContractsTrait.php @@ -40,7 +40,7 @@ trait ContractsTrait public function setCallbackWrapper(?callable $callbackWrapper): callable { $previousWrapper = $this->callbackWrapper; - $this->callbackWrapper = $callbackWrapper ?? function (callable $callback, ItemInterface $item, bool &$save, CacheInterface $pool) { + $this->callbackWrapper = $callbackWrapper ?? function (callable $callback, ItemInterface $item, bool &$save, CacheInterface $pool, \Closure $setMetadata) { return $callback($item, $save); }; @@ -56,17 +56,19 @@ trait ContractsTrait static $setMetadata; $setMetadata = $setMetadata ?? \Closure::bind( - function (AdapterInterface $pool, ItemInterface $item, float $startTime) { + function (CacheItem $item, float $startTime, ?array &$metadata) { if ($item->expiry > $endTime = microtime(true)) { - $item->newMetadata[ItemInterface::METADATA_EXPIRY] = $item->expiry; - $item->newMetadata[ItemInterface::METADATA_CTIME] = 1000 * (int) ($endTime - $startTime); + $item->newMetadata[CacheItem::METADATA_EXPIRY] = $metadata[CacheItem::METADATA_EXPIRY] = $item->expiry; + $item->newMetadata[CacheItem::METADATA_CTIME] = $metadata[CacheItem::METADATA_CTIME] = 1000 * (int) ($endTime - $startTime); + } else { + unset($metadata[CacheItem::METADATA_EXPIRY], $metadata[CacheItem::METADATA_CTIME]); } }, null, CacheItem::class ); - return $this->contractsGet($pool, $key, function (CacheItem $item, bool &$save) use ($pool, $callback, $setMetadata) { + return $this->contractsGet($pool, $key, function (CacheItem $item, bool &$save) use ($pool, $callback, $setMetadata, &$metadata) { // don't wrap nor save recursive calls if (null === $callbackWrapper = $this->callbackWrapper) { $value = $callback($item, $save); @@ -78,8 +80,10 @@ trait ContractsTrait $startTime = microtime(true); try { - $value = $callbackWrapper($callback, $item, $save, $pool); - $setMetadata($pool, $item, $startTime); + $value = $callbackWrapper($callback, $item, $save, $pool, function (CacheItem $item) use ($setMetadata, $startTime, &$metadata) { + $setMetadata($item, $startTime, $metadata); + }); + $setMetadata($item, $startTime, $metadata); return $value; } finally { From 036e72210db356c4b16231660a94db04ae9de51c Mon Sep 17 00:00:00 2001 From: Emmanuel BORGES Date: Fri, 8 Mar 2019 11:29:43 +0100 Subject: [PATCH 7/7] Fix Cache error while using anonymous class --- .../Component/Validator/Mapping/Cache/Psr6Cache.php | 5 +++++ .../Validator/Tests/Mapping/Cache/Psr6CacheTest.php | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/src/Symfony/Component/Validator/Mapping/Cache/Psr6Cache.php b/src/Symfony/Component/Validator/Mapping/Cache/Psr6Cache.php index 73c9513c16..a99d3ae700 100644 --- a/src/Symfony/Component/Validator/Mapping/Cache/Psr6Cache.php +++ b/src/Symfony/Component/Validator/Mapping/Cache/Psr6Cache.php @@ -70,6 +70,11 @@ class Psr6Cache implements CacheInterface */ private function escapeClassName($class) { + if (false !== strpos($class, '@')) { + // anonymous class: replace all PSR6-reserved characters + return str_replace(["\0", '\\', '/', '@', ':', '{', '}', '(', ')'], '.', $class); + } + return str_replace('\\', '.', $class); } } diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Cache/Psr6CacheTest.php b/src/Symfony/Component/Validator/Tests/Mapping/Cache/Psr6CacheTest.php index c11dddbf6f..fcac5e232a 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/Cache/Psr6CacheTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/Cache/Psr6CacheTest.php @@ -23,4 +23,12 @@ class Psr6CacheTest extends AbstractCacheTest $this->cache->write($metadata); $this->assertFalse($this->cache->has('Foo_Bar')); } + + public function testNameWithInvalidChars() + { + $metadata = new ClassMetadata('class@anonymous/path/file'); + + $this->cache->write($metadata); + $this->assertTrue($this->cache->has('class@anonymous/path/file')); + } }