From 5ace4f76c6cf2c9cb61ae2f6eed25204c455f1bc Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Tue, 23 Jun 2026 23:31:28 +0200 Subject: [PATCH] fix(cache): make clear() redis cluster compatible Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- lib/private/Memcache/LoggerWrapperCache.php | 2 +- lib/private/Memcache/Redis.php | 48 ++++++++++++++++++--- tests/lib/Memcache/RedisTest.php | 33 ++++++++++++++ 3 files changed, 76 insertions(+), 7 deletions(-) diff --git a/lib/private/Memcache/LoggerWrapperCache.php b/lib/private/Memcache/LoggerWrapperCache.php index c2a06731910a8..5f1dcd983d5e2 100644 --- a/lib/private/Memcache/LoggerWrapperCache.php +++ b/lib/private/Memcache/LoggerWrapperCache.php @@ -157,7 +157,7 @@ public function ncad(string $key, mixed $old): bool { FILE_APPEND ); - return $this->wrappedCache->cad($key, $old); + return $this->wrappedCache->ncad($key, $old); } /** @inheritDoc */ diff --git a/lib/private/Memcache/Redis.php b/lib/private/Memcache/Redis.php index f8c51570c4fc6..545fb17ae90ad 100644 --- a/lib/private/Memcache/Redis.php +++ b/lib/private/Memcache/Redis.php @@ -36,8 +36,11 @@ class Redis extends Cache implements IMemcacheTTL { private const MAX_TTL = 30 * 24 * 60 * 60; // 1 month + /** Number of keys to request per SCAN iteration in {@see self::clear()} (only a hint to Redis) */ + private const SCAN_COUNT = 1000; + /** - * @var \Redis|\RedisCluster $cache + * @var \Redis|\RedisCluster|null $cache */ private static $cache = null; @@ -88,12 +91,45 @@ public function remove($key) { } public function clear($prefix = '') { - // TODO: this is slow and would fail with Redis cluster - $prefix = $this->getPrefix() . $prefix . '*'; - $keys = $this->getCache()->keys($prefix); - $deleted = $this->getCache()->del($keys); + $pattern = $this->getPrefix() . $prefix . '*'; + $cache = $this->getCache(); + + // Iterate with SCAN and remove with UNLINK rather than KEYS + DEL: + // KEYS walks the whole keyspace and blocks the server, while a + // multi-key DEL/UNLINK is not cluster-safe (keys spanning hash slots + // raise a CROSSSLOT error). SCAN is non-blocking and UNLINK reclaims + // memory in the background. + if ($cache instanceof \RedisCluster) { + // On a cluster SCAN must be run against each master node, and keys + // are unlinked one at a time so each command stays within a slot. + foreach ($cache->_masters() as $master) { + $iterator = null; + do { + /** @psalm-suppress NullArgument, PossiblyNullArgument the SCAN cursor must start as null (the phpredis stub types it as int) */ + $keys = $cache->scan($iterator, $master, $pattern, self::SCAN_COUNT); + if ($keys === false) { + break; + } + foreach ($keys as $key) { + $cache->unlink($key); + } + } while ($iterator > 0); + } + } else { + $iterator = null; + do { + /** @psalm-suppress NullArgument, PossiblyNullArgument the SCAN cursor must start as null (the phpredis stub types it as int) */ + $keys = $cache->scan($iterator, $pattern, self::SCAN_COUNT); + if ($keys === false) { + break; + } + if ($keys !== []) { + $cache->unlink($keys); + } + } while ($iterator > 0); + } - return (is_array($keys) && (count($keys) === $deleted)); + return true; } /** diff --git a/tests/lib/Memcache/RedisTest.php b/tests/lib/Memcache/RedisTest.php index 18c08b87c9884..86d1d119a674b 100644 --- a/tests/lib/Memcache/RedisTest.php +++ b/tests/lib/Memcache/RedisTest.php @@ -82,4 +82,37 @@ public function testCasTtlChanged(): void { // allow for 1s of inaccuracy due to time moving forward $this->assertLessThan(1, 50 - $this->instance->getTTL('foo')); } + + public function testClearWithPrefixOnlyRemovesMatchingKeys(): void { + $this->instance->set('foo1', 'a'); + $this->instance->set('foo2', 'b'); + $this->instance->set('bar1', 'c'); + + $this->assertTrue($this->instance->clear('foo')); + + $this->assertFalse($this->instance->hasKey('foo1')); + $this->assertFalse($this->instance->hasKey('foo2')); + $this->assertTrue($this->instance->hasKey('bar1')); + } + + public function testClearWithoutMatchesReturnsTrue(): void { + // Nothing is stored under this prefix; clearing must not error out + // (regression guard for calling UNLINK/DEL with an empty key list). + $this->assertTrue($this->instance->clear('no-such-prefix')); + } + + public function testClearRemovesEntriesAcrossMultipleScanBatches(): void { + // More keys than a single SCAN batch (self::SCAN_COUNT) to exercise the + // cursor loop and make sure nothing is left behind. + $count = 1500; + for ($i = 0; $i < $count; $i++) { + $this->instance->set('bulk-' . $i, $i); + } + + $this->assertTrue($this->instance->clear('bulk-')); + + $this->assertFalse($this->instance->hasKey('bulk-0')); + $this->assertFalse($this->instance->hasKey('bulk-' . ($count - 1))); + $this->assertFalse($this->instance->hasKey('bulk-' . intdiv($count, 2))); + } }