From f349abb1aefcfc7370776d8d2bb762446a9685c3 Mon Sep 17 00:00:00 2001 From: Bentley O'Kane-Chase Date: Wed, 18 Dec 2024 07:28:36 +1000 Subject: [PATCH] Avoid writing multiple keys when using redis in cluster mode (#53940) * Avoid writing multiple keys when using redis in cluster mode * formatting --------- Co-authored-by: Taylor Otwell --- src/Illuminate/Cache/RedisStore.php | 20 +++++++++++++++++--- tests/Integration/Cache/RedisStoreTest.php | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Cache/RedisStore.php b/src/Illuminate/Cache/RedisStore.php index 566913389dcb..c61e925f3687 100755 --- a/src/Illuminate/Cache/RedisStore.php +++ b/src/Illuminate/Cache/RedisStore.php @@ -4,13 +4,19 @@ use Illuminate\Contracts\Cache\LockProvider; use Illuminate\Contracts\Redis\Factory as Redis; +use Illuminate\Redis\Connections\PhpRedisClusterConnection; use Illuminate\Redis\Connections\PhpRedisConnection; +use Illuminate\Redis\Connections\PredisClusterConnection; use Illuminate\Redis\Connections\PredisConnection; use Illuminate\Support\LazyCollection; use Illuminate\Support\Str; class RedisStore extends TaggableStore implements LockProvider { + use RetrievesMultipleKeys { + putMany as private putManyAlias; + } + /** * The Redis factory implementation. * @@ -118,25 +124,33 @@ public function put($key, $value, $seconds) */ public function putMany(array $values, $seconds) { + $connection = $this->connection(); + + // Cluster connections do not support writing multiple values if the keys hash differently... + if ($connection instanceof PhpRedisClusterConnection || + $connection instanceof PredisClusterConnection) { + return $this->putManyAlias($values, $seconds); + } + $serializedValues = []; foreach ($values as $key => $value) { $serializedValues[$this->prefix.$key] = $this->serialize($value); } - $this->connection()->multi(); + $connection->multi(); $manyResult = null; foreach ($serializedValues as $key => $value) { - $result = (bool) $this->connection()->setex( + $result = (bool) $connection->setex( $key, (int) max(1, $seconds), $value ); $manyResult = is_null($manyResult) ? $result : $result && $manyResult; } - $this->connection()->exec(); + $connection->exec(); return $manyResult ?: false; } diff --git a/tests/Integration/Cache/RedisStoreTest.php b/tests/Integration/Cache/RedisStoreTest.php index 1d748cf24525..8df8b4ee3f9c 100644 --- a/tests/Integration/Cache/RedisStoreTest.php +++ b/tests/Integration/Cache/RedisStoreTest.php @@ -3,10 +3,13 @@ namespace Illuminate\Tests\Integration\Cache; use DateTime; +use Illuminate\Cache\RedisStore; use Illuminate\Foundation\Testing\Concerns\InteractsWithRedis; +use Illuminate\Redis\Connections\PhpRedisClusterConnection; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Redis; use Illuminate\Support\Sleep; +use Mockery as m; use Orchestra\Testbench\TestCase; class RedisStoreTest extends TestCase @@ -25,6 +28,7 @@ protected function tearDown(): void parent::tearDown(); $this->tearDownRedis(); + m::close(); } public function testCacheTtl(): void @@ -231,4 +235,18 @@ public function testMultipleItemsCanBeSetAndRetrieved() $this->assertEquals([], $store->many([])); } + + public function testPutManyCallsPutWhenClustered() + { + $store = m::mock(RedisStore::class)->makePartial(); + $store->expects('connection')->andReturn(m::mock(PhpRedisClusterConnection::class)); + $store->expects('put') + ->twice() + ->andReturn(true); + + $store->putMany([ + 'foo' => 'bar', + 'fizz' => 'buz', + ], 10); + } }