Merge pull request #62176 from nextcloud/backport/61559/stable33

[stable33] fix(cache): make clear() redis cluster compatible

Author: AndyScherzinger

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 */

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;
 	}
 
 	/**

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)));
+	}
 }