diff --git a/CHANGELOG.md b/CHANGELOG.md index bf0047315..aba1583e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -286,6 +286,11 @@ not change anything for an upgrade from 7.x. document's tenant, whichever tenant is selected on the writing `Database`, and leaves every other tenant's in place. - A written document's own attribute named `options` no longer names a collection to invalidate. + - Writes no longer leave a key behind in Redis each. The document cache and the `find()` query cache registered + every invalidation under a key of its own, and a Redis purge keeps a purged key, holding its generation, with no + expiry. On the Redis adapters a collection's invalidations now register as fields of one `#owners` key; + adapters that store no fields keep a key per invalidation, which their purge deletes. On Redis, keys matching + `*#owner:*` left by earlier builds are no longer read and can be deleted. - **Permissions and tenancy:** - Under shared tables, `upsertDocuments()` and `upsertDocument()` store their permission rows under the tenant, on an adapter with no earlier write and through `Adapter\Pool` alike. diff --git a/src/Database/Cache/Owners.php b/src/Database/Cache/Owners.php new file mode 100644 index 000000000..b23af76a0 --- /dev/null +++ b/src/Database/Cache/Owners.php @@ -0,0 +1,57 @@ +getOwnersKey($key); + if ($this->cache->save($owners, $token, $token) === false) { + return false; + } + + if ($this->isField($owners, $token)) { + return true; + } + + return $this->cache->save($this->getOwnerKey($key, $token), $token) !== false; + } + + public function find(string $key, string $token): Registration + { + $owners = $this->getOwnersKey($key); + + return $this->isField($owners, $token) + ? new Registration($owners, $token) + : new Registration($this->getOwnerKey($key, $token)); + } + + private function isField(string $owners, string $token): bool + { + return \in_array($token, $this->cache->list($owners), true); + } + + private function getOwnersKey(string $key): string + { + return $key.'#owners'; + } + + private function getOwnerKey(string $key, string $token): string + { + return $key.'#owner:'.$token; + } +} diff --git a/src/Database/Cache/QueryCache.php b/src/Database/Cache/QueryCache.php index 2fd280868..a0a7f552a 100644 --- a/src/Database/Cache/QueryCache.php +++ b/src/Database/Cache/QueryCache.php @@ -169,7 +169,7 @@ public function invalidateCollection(Scope $scope, string $collection): void */ public function blockCollection(string $key, string $token): void { - if ($this->cache->save($this->getOwnerKey($key, $token), $token) === false) { + if (! (new Owners($this->cache))->register($key, $token)) { throw new RuntimeException("Failed to register query cache owner for '{$key}'"); } @@ -186,14 +186,14 @@ public function blockCollection(string $key, string $token): void */ public function activateCollection(string $key, string $token): void { - $ownerKey = $this->getOwnerKey($key, $token); - $owner = $this->cache->load($ownerKey, self::PERMANENT); + $registration = (new Owners($this->cache))->find($key, $token); + $owner = $this->cache->load($registration->key, self::PERMANENT, $registration->field); if ($owner !== false && $owner !== null && $owner !== $token) { throw new RuntimeException("Invalid query cache owner for '{$key}'"); } $owned = $owner === $token; - if ($owned && ! $this->cache->purge($ownerKey)) { - $owner = $this->cache->load($ownerKey, self::PERMANENT); + if ($owned && ! $this->cache->purge($registration->key, $registration->field)) { + $owner = $this->cache->load($registration->key, self::PERMANENT, $registration->field); if ($owner !== false && $owner !== null) { throw new RuntimeException("Failed to release query cache owner for '{$key}'"); } @@ -324,11 +324,6 @@ private function getFinishedKey(string $key): string return $key.'#finished'; } - private function getOwnerKey(string $key, string $token): string - { - return $key.'#owner:'.$token; - } - private function getStartedKey(string $key): string { return $key.'#started'; diff --git a/src/Database/Cache/Registration.php b/src/Database/Cache/Registration.php new file mode 100644 index 000000000..6c07723a0 --- /dev/null +++ b/src/Database/Cache/Registration.php @@ -0,0 +1,12 @@ +cache->save($ownerKey, $token) === false) { + if (! (new Owners($this->cache))->register($collectionKey, $token)) { $epoch = $this->cache->load($epochKey, self::TTL); if ($epoch === false || $epoch === null) { return false; } - throw new RuntimeException("Failed to register document cache owner '{$ownerKey}'"); + throw new RuntimeException("Failed to register document cache owner '{$token}' for '{$collectionKey}'"); } $startedKey = $collectionKey.'#started'; @@ -2633,16 +2633,16 @@ protected function activateDocumentInvalidation(array $tokens): void private function activateDocumentCacheEpoch(string $collectionKey, string $token): void { - $ownerKey = $collectionKey.'#owner:'.$token; - $owner = $this->cache->load($ownerKey, self::TTL); + $registration = (new Owners($this->cache))->find($collectionKey, $token); + $owner = $this->cache->load($registration->key, self::TTL, $registration->field); if ($owner !== false && $owner !== null && $owner !== $token) { - throw new RuntimeException("Invalid document cache owner '{$ownerKey}'"); + throw new RuntimeException("Invalid document cache owner '{$token}' for '{$collectionKey}'"); } $owned = $owner === $token; - if ($owned && ! $this->cache->purge($ownerKey)) { - $owner = $this->cache->load($ownerKey, self::TTL); + if ($owned && ! $this->cache->purge($registration->key, $registration->field)) { + $owner = $this->cache->load($registration->key, self::TTL, $registration->field); if ($owner !== false && $owner !== null) { - throw new RuntimeException("Failed to release document cache owner '{$ownerKey}'"); + throw new RuntimeException("Failed to release document cache owner '{$token}' for '{$collectionKey}'"); } $owned = false; } diff --git a/tests/e2e/Adapter/Scopes/GeneralTests.php b/tests/e2e/Adapter/Scopes/GeneralTests.php index 99091a805..ed4d5367e 100644 --- a/tests/e2e/Adapter/Scopes/GeneralTests.php +++ b/tests/e2e/Adapter/Scopes/GeneralTests.php @@ -847,6 +847,55 @@ public function testNestedTransactionKeepsDocumentCacheEpochBlockedUntilOuterCom } } + public function testCacheInvalidationDoesNotAddRedisKeysPerWrite(): void + { + $database = $this->getDatabase(); + if (! $database->getAdapter()->supports(Capability::Caching)) { + $this->markTestSkipped('Adapter does not use the document cache.'); + } + + $collection = 'ownerKeys'.ID::unique(); + $database->createCollection(new Collection(id: $collection, attributes: [ + Attribute::string(key: 'name', size: 64, required: true), + ], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + ])); + + $redis = new Redis(); + $redis->connect('redis', 6379); + $cache = new Cache(new RedisAdapter($redis)); + $original = $database->getCache(); + $queryCache = $database->getQueryCache(); + $destination = $database instanceof Mirror ? $database->getDestination() : null; + $destinationCache = $destination?->getCache(); + $database->setCache($cache)->setQueryCache(new QueryCache($cache)); + + try { + $database->createDocument($collection, new Document(['$id' => 'doc0', 'name' => 'doc 0'])); + $keys = $this->scanKeys($redis, $collection); + + for ($index = 1; $index <= 10; $index++) { + $database->createDocument($collection, new Document(['$id' => 'doc'.$index, 'name' => 'doc '.$index])); + } + $database->withTransaction(function () use ($database, $collection): void { + $database->updateDocument($collection, 'doc1', new Document(['name' => 'updated'])); + $database->updateDocument($collection, 'doc2', new Document(['name' => 'updated'])); + }); + + $this->assertNotEmpty($keys); + $this->assertSame($keys, $this->scanKeys($redis, $collection), 'Redis keeps a purged key with no expiry, so a write must not leave a key of its own'); + $this->assertCount(11, $database->find($collection, [Query::limit(100)])); + } finally { + $database->setQueryCache($queryCache)->setCache($original); + if ($destination !== null && $destinationCache !== null) { + $destination->setCache($destinationCache); + } + $database->deleteCollection($collection); + } + } + /** * Test that withTransaction correctly resets inTransaction state * when a known exception (DuplicateException) is thrown after successful rollback. @@ -1068,6 +1117,26 @@ public function testCacheReconnect(): void } } + /** + * @return array + */ + private function scanKeys(Redis $redis, string $collection): array + { + $keys = []; + $iterator = null; + do { + $batch = $redis->scan($iterator, '*'.\strtolower($collection).'*', 1000); + if (\is_array($batch)) { + \array_push($keys, ...$batch); + } + } while ($iterator > 0); + + $keys = \array_values(\array_unique($keys)); + \sort($keys); + + return $keys; + } + private function dropRedisConnection(Redis $redis): void { $id = $redis->rawCommand('CLIENT', 'ID'); diff --git a/tests/unit/Cache/QueryCacheTest.php b/tests/unit/Cache/QueryCacheTest.php index dd8524d78..46b040efa 100644 --- a/tests/unit/Cache/QueryCacheTest.php +++ b/tests/unit/Cache/QueryCacheTest.php @@ -564,6 +564,75 @@ public function testActivationPurgeFailureStillPropagates(): void $this->assertNull($queryCache->getEntry(new Scope(), 'users', [])); } + public function testInvalidationsDoNotAddKeysToACacheThatKeepsPurgedKeys(): void + { + $adapter = new RedisLeasableCache(); + $queryCache = new QueryCache(new Cache($adapter)); + $scope = new Scope(namespace: 'ns'); + $queryCache->invalidateCollection($scope, 'users'); + $keys = $adapter->keys(); + + for ($invalidation = 0; $invalidation < 20; $invalidation++) { + $queryCache->invalidateCollection($scope, 'users'); + } + + $this->assertSame($keys, $adapter->keys(), 'A purged key stays behind in Redis, so an invalidation must not leave a key of its own'); + $this->assertNotNull($queryCache->getEntry($scope, 'users', [])); + } + + public function testOverlappingInvalidationsSucceedOnACacheWithoutFields(): void + { + $queryCache = new QueryCache(new Cache(new Memory())); + $key = $queryCache->getCollectionKey(new Scope(), 'users'); + + $queryCache->blockCollection($key, 'first'); + $queryCache->blockCollection($key, 'second'); + $queryCache->activateCollection($key, 'first'); + $this->assertNull($queryCache->getEntry(new Scope(), 'users', []), 'The second writer is still in flight'); + $queryCache->activateCollection($key, 'second'); + + $this->assertNull( + $queryCache->getEntry(new Scope(), 'users', []), + 'A cache without generations cannot prove a new epoch fresh, so it stays blocked', + ); + } + + public function testActivationRejectsACorruptedOwnerRegistration(): void + { + $adapter = new RedisLeasableCache(); + $queryCache = new QueryCache(new Cache($adapter)); + $key = $queryCache->getCollectionKey(new Scope(), 'users'); + $adapter->corruptFieldWrites(); + $queryCache->blockCollection($key, 'owner'); + + try { + $queryCache->activateCollection($key, 'owner'); + $this->fail('A corrupted owner registration was accepted'); + } catch (\RuntimeException $error) { + $this->assertStringContainsString('Invalid query cache owner', $error->getMessage()); + } + + $this->assertNull($queryCache->getEntry(new Scope(), 'users', [])); + } + + public function testActivationPropagatesAnOwnerReleaseFailure(): void + { + $adapter = new RedisLeasableCache(); + $queryCache = new QueryCache(new Cache($adapter)); + $key = $queryCache->getCollectionKey(new Scope(), 'users'); + $queryCache->blockCollection($key, 'owner'); + $adapter->failFieldPurges(); + + try { + $queryCache->activateCollection($key, 'owner'); + $this->fail('An owner release failure was not propagated'); + } catch (\RuntimeException $error) { + $this->assertStringContainsString('Failed to release query cache owner', $error->getMessage()); + } + + $this->assertNull($queryCache->getEntry(new Scope(), 'users', [])); + } + public function testInvalidationPropagatesACacheWriteFailure(): void { $cache = new InvalidationCache(); diff --git a/tests/unit/Cache/RedisLeasableCache.php b/tests/unit/Cache/RedisLeasableCache.php new file mode 100644 index 000000000..a28fca95e --- /dev/null +++ b/tests/unit/Cache/RedisLeasableCache.php @@ -0,0 +1,160 @@ +|string}>> */ + private array $fields = []; + + /** @var array */ + private array $generations = []; + + private bool $failingFieldPurges = false; + + private bool $corruptingFieldWrites = false; + + public function load(string $key, int $ttl, string $hash = ''): mixed + { + $saved = $this->fields[$key][$this->field($key, $hash)] ?? null; + + return $saved !== null && $saved['time'] + $ttl > \time() ? $saved['data'] : false; + } + + public function save(string $key, array|string $data, string $hash = ''): bool|string|array + { + if ($key === '' || empty($data)) { + return false; + } + + if ($hash !== '' && $this->corruptingFieldWrites) { + $data = 'corrupted'; + } + + $this->fields[$key][$this->field($key, $hash)] = ['time' => \time(), 'data' => $data]; + + return $data; + } + + public function getGeneration(string $key): string + { + return (string) ($this->generations[$key] ?? 0); + } + + public function saveWithLease(string $key, array|string $data, string $hash, string $generation): bool|string|array + { + if ($this->getGeneration($key) !== $generation) { + return false; + } + + return $this->save($key, $data, $hash); + } + + public function touch(string $key, string $hash = ''): bool + { + $field = $this->field($key, $hash); + if (! isset($this->fields[$key][$field])) { + return false; + } + + $this->fields[$key][$field]['time'] = \time(); + + return true; + } + + /** @return array */ + public function list(string $key): array + { + return \array_map(\strval(...), \array_keys($this->fields[$key] ?? [])); + } + + public function purge(string $key, string $hash = ''): bool + { + if ($hash !== '' && $this->failingFieldPurges) { + return false; + } + + $this->generations[$key] = ($this->generations[$key] ?? 0) + 1; + + if ($hash === '') { + $removed = \count($this->fields[$key] ?? []); + unset($this->fields[$key]); + + return $removed > 0; + } + + $removed = isset($this->fields[$key][$hash]); + unset($this->fields[$key][$hash]); + if (($this->fields[$key] ?? null) === []) { + unset($this->fields[$key]); + } + + return $removed; + } + + public function flush(): bool + { + $this->fields = []; + $this->generations = []; + + return true; + } + + public function ping(): bool + { + return true; + } + + public function getSize(): int + { + return \count($this->keys()); + } + + public function getName(?string $key = null): string + { + return 'redis-leasable'; + } + + /** + * Every key the cache holds, including keys that only hold a generation. + * + * @return array + */ + public function keys(): array + { + $keys = \array_map(\strval(...), \array_keys($this->fields + $this->generations)); + \sort($keys); + + return $keys; + } + + /** + * Fail every purge of a single field, leaving the field in place. + */ + public function failFieldPurges(): void + { + $this->failingFieldPurges = true; + } + + /** + * Store a different value than the one given on every write to a single field. + */ + public function corruptFieldWrites(): void + { + $this->corruptingFieldWrites = true; + } + + private function field(string $key, string $hash): string + { + return $hash === '' ? $key : $hash; + } +} diff --git a/tests/unit/Documents/DocumentCacheEpochTest.php b/tests/unit/Documents/DocumentCacheEpochTest.php index 0699f1a43..3eabcef0e 100644 --- a/tests/unit/Documents/DocumentCacheEpochTest.php +++ b/tests/unit/Documents/DocumentCacheEpochTest.php @@ -4,12 +4,15 @@ use Closure; use PHPUnit\Framework\TestCase; +use Tests\Unit\Cache\RedisLeasableCache; +use Utopia\Cache\Adapter as CacheAdapter; use Utopia\Cache\Adapter\Memory as MemoryCache; use Utopia\Cache\Cache; use Utopia\Cache\Feature\Leasable; use Utopia\Database\Adapter\Memory as DatabaseMemory; use Utopia\Database\Adapter\SQLite; use Utopia\Database\Attribute; +use Utopia\Database\Cache\QueryCache; use Utopia\Database\Collection; use Utopia\Database\Database; use Utopia\Database\Document; @@ -246,6 +249,111 @@ public function testActivationPurgeFailureStillPropagates(): void $this->assertSame('updated', $database->getDocument('webhooks', 'hook')->getAttribute('name')); } + public function testWritesDoNotAddKeysToACacheThatKeepsPurgedKeys(): void + { + $cache = new RedisLeasableCache(); + $database = $this->createDatabaseWithCache($cache); + $database->setQueryCache(new QueryCache(new Cache($cache))); + $database->createDocument('webhooks', new Document([ + '$id' => 'hook0', + 'name' => 'hook 0', + ])); + $keys = $cache->keys(); + + for ($index = 1; $index <= 20; $index++) { + $database->createDocument('webhooks', new Document([ + '$id' => 'hook'.$index, + 'name' => 'hook '.$index, + ])); + } + $database->withTransaction(function () use ($database): void { + $database->updateDocument('webhooks', 'hook1', new Document(['name' => 'updated'])); + $database->updateDocument('webhooks', 'hook2', new Document(['name' => 'updated'])); + }); + + $this->assertSame($keys, $cache->keys(), 'A purged key stays behind in Redis, so a write must not leave a key of its own'); + $this->assertSame('updated', $database->getDocument('webhooks', 'hook1')->getAttribute('name')); + } + + public function testOverlappingWritesSucceedOnACacheWithoutFields(): void + { + $cache = new FailDocumentEpochMemory(); + $writer = $this->createDatabaseWithCache($cache); + $other = $this->createDatabaseWithCache($cache, $writer->getNamespace()); + foreach ([$writer, $other] as $database) { + $database->createDocument('webhooks', new Document([ + '$id' => 'hook', + 'name' => 'original', + ])); + } + + $writer->withTransaction(function () use ($writer, $other, $cache): void { + $writer->updateDocument('webhooks', 'hook', new Document(['name' => 'updated'])); + $cache->failBlocks(); + try { + $other->updateDocument('webhooks', 'hook', new Document(['name' => 'updated'])); + $this->fail('The other writer\'s block did not fail'); + } catch (\RuntimeException $error) { + $this->assertStringContainsString('block document cache epoch', $error->getMessage()); + } finally { + $cache->failBlocks(false); + } + }); + + $this->assertSame('updated', $writer->getDocument('webhooks', 'hook')->getAttribute('name')); + } + + public function testActivationRejectsACorruptedOwnerRegistration(): void + { + $cache = new RedisLeasableCache(); + $database = $this->createDatabaseWithCache($cache); + $database->createDocument('webhooks', new Document([ + '$id' => 'hook', + 'name' => 'original', + ])); + $cache->corruptFieldWrites(); + + try { + $database->updateDocument('webhooks', 'hook', new Document(['name' => 'updated'])); + $this->fail('A corrupted document cache owner registration was accepted'); + } catch (\RuntimeException $error) { + $this->assertStringContainsString('Invalid document cache owner', $error->getMessage()); + } + + [$collectionKey] = $database->getCacheKeys('webhooks', 'hook'); + $this->assertDocumentCacheEpochBlocked($database, $collectionKey); + $this->assertSame('updated', $database->getDocument('webhooks', 'hook')->getAttribute('name')); + } + + public function testActivationPropagatesAnOwnerReleaseFailure(): void + { + $cache = new RedisLeasableCache(); + $database = $this->createDatabaseWithCache($cache); + $database->createDocument('webhooks', new Document([ + '$id' => 'hook', + 'name' => 'original', + ])); + $cache->failFieldPurges(); + + try { + $database->updateDocument('webhooks', 'hook', new Document(['name' => 'updated'])); + $this->fail('A document cache owner release failure was not propagated'); + } catch (\RuntimeException $error) { + $this->assertStringContainsString('Failed to release document cache owner', $error->getMessage()); + } + + [$collectionKey] = $database->getCacheKeys('webhooks', 'hook'); + $this->assertDocumentCacheEpochBlocked($database, $collectionKey); + $this->assertSame('updated', $database->getDocument('webhooks', 'hook')->getAttribute('name')); + } + + private function assertDocumentCacheEpochBlocked(Database $database, string $collectionKey): void + { + $epoch = $database->getCache()->load($collectionKey.'#epoch', Database::TTL); + $this->assertIsString($epoch); + $this->assertStringStartsWith('blocked:', $epoch); + } + /** * @return array{Database, FailPurgeMemory} */ @@ -257,12 +365,12 @@ private function createDatabase(): array return [$database, $adapter]; } - private function createDatabaseWithCache(MemoryCache $cache): Database + private function createDatabaseWithCache(CacheAdapter $cache, ?string $namespace = null): Database { $database = new Database(new DatabaseMemory(), new Cache($cache)); $database ->setDatabase('utopiaTests') - ->setNamespace('epoch_'.\uniqid()); + ->setNamespace($namespace ?? 'epoch_'.\uniqid()); $database->create(); $database->createCollection(new Collection(id: 'webhooks', attributes: [ Attribute::string(key: 'name'), @@ -379,9 +487,9 @@ final class FailDocumentEpochMemory extends MemoryCache private ?string $activationFailure = null; - public function failBlocks(): void + public function failBlocks(bool $failing = true): void { - $this->failingBlocks = true; + $this->failingBlocks = $failing; } public function failActivations(?string $key = null): void