From 9ff5129de92488c0cf16bb1807ca6a6718eafc48 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 17 Aug 2026 18:51:21 +1200 Subject: [PATCH 1/9] fix: do not drop a peer's collection when losing a create race Two processes reconciling the same schema can both read a collection as missing: the metadata read is served by a negative cache entry that the peer only purges once its insert commits. The loser then took the DuplicateException from the adapter as proof of an orphaned table, so it dropped the peer's live table and recreated it, and when its own metadata insert hit the unique key it rolled that table back out again. The collection was left with metadata but no table, and the caller saw a generic "Failed to create collection metadata" that took down a booting server. Re-read metadata past the cache before treating a table as an orphan, and on a duplicate metadata insert report DuplicateException without rolling back a table the peer owns. Co-Authored-By: Claude Opus 5 --- src/Database/Database.php | 25 ++++++-- tests/e2e/Adapter/Scopes/CollectionTests.php | 65 ++++++++++++++++++++ 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 760c1aeafe..019d00668f 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1908,11 +1908,21 @@ public function createCollection(string $id, array $attributes = [], array $inde // tenants. A DuplicateException simply means the table already // exists for another tenant — not an orphan. } else { - // Metadata check (above) already verified collection is absent - // from metadata. A DuplicateException from the adapter means - // the collection exists only in physical schema — an orphan - // from a prior partial failure. Drop and recreate to ensure - // schema matches. + // The metadata check above can be served by a negative cache + // entry that a concurrent creator has not purged yet, because + // the purge only happens once its insert commits. Re-read past + // the cache before concluding the table is an orphan: if the + // metadata is there, the table belongs to that creator and + // dropping it would destroy a live collection. + $committed = $this->silent(fn () => $this->getDocument(self::METADATA, $id, forUpdate: true)); + + if (!$committed->isEmpty()) { + throw new DuplicateException('Collection ' . $id . ' already exists'); + } + + // The collection is absent from metadata but present in the + // physical schema — an orphan from a prior partial failure. + // Drop and recreate to ensure schema matches. try { $this->adapter->deleteCollection($id); } catch (NotFoundException) { @@ -1929,6 +1939,11 @@ public function createCollection(string $id, array $attributes = [], array $inde try { $createdCollection = $this->silent(fn () => $this->createDocument(self::METADATA, $collection)); + } catch (DuplicateException $e) { + // A concurrent creator committed the metadata for this id first, so + // the physical table is the one its metadata describes. Rolling back + // here would drop a live collection out from under it. + throw new DuplicateException('Collection ' . $id . ' already exists', previous: $e); } catch (\Throwable $e) { if ($createdPhysicalTable) { try { diff --git a/tests/e2e/Adapter/Scopes/CollectionTests.php b/tests/e2e/Adapter/Scopes/CollectionTests.php index 1cbebd1dba..b8d2e0af81 100644 --- a/tests/e2e/Adapter/Scopes/CollectionTests.php +++ b/tests/e2e/Adapter/Scopes/CollectionTests.php @@ -3,6 +3,8 @@ namespace Tests\E2E\Adapter\Scopes; use Exception; +use Utopia\Cache\Adapter\None as NoneCache; +use Utopia\Cache\Cache; use Utopia\Database\Adapter\SQL; use Utopia\Database\Database; use Utopia\Database\Document; @@ -1842,4 +1844,67 @@ public function testCreateCollectionWithLongId(): void $this->assertTrue($database->deleteCollection($collection)); } + + /** + * Two processes reconciling the same schema race: one reads the collection + * as missing, a peer creates it and commits, and only then does the first + * process try to create it. The loser must not mistake the peer's table for + * an orphan and drop it. + */ + public function testCreateCollectionConcurrentlyKeepsPeerData(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $collection = 'concurrentCreate'; + + // A peer process: same database, its own cache, so its writes do not + // purge the negative cache entry this process is about to record. + $peer = (new Database($database->getAdapter(), new Cache(new NoneCache()))) + ->setAuthorization(self::$authorization); + + $this->assertTrue($database->getCollection($collection)->isEmpty()); + + $peer->createCollection($collection, [ + new Document([ + '$id' => ID::custom('name'), + 'type' => Database::VAR_STRING, + 'size' => 128, + 'required' => false, + ]), + ], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + ]); + + $peer->createDocument($collection, new Document([ + '$id' => ID::custom('written'), + '$permissions' => [Permission::read(Role::any())], + 'name' => 'peer', + ])); + + try { + $database->createCollection($collection, [ + new Document([ + '$id' => ID::custom('name'), + 'type' => Database::VAR_STRING, + 'size' => 128, + 'required' => false, + ]), + ], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + ]); + $this->fail('Expected DuplicateException for a collection a peer already created'); + } catch (DuplicateException) { + } + + $survivor = $peer->getDocument($collection, 'written'); + $this->assertSame('peer', $survivor->getAttribute('name'), 'Peer document was destroyed by the losing creator'); + + $metadata = $peer->getCollection($collection); + $this->assertFalse($metadata->isEmpty(), 'Peer collection metadata was destroyed by the losing creator'); + + $this->assertTrue($peer->deleteCollection($collection)); + } } From 7dfc42d0fbc094e7710bb5e5922d7ad176e0d8b5 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 17 Aug 2026 19:06:55 +1200 Subject: [PATCH 2/9] fix: clear the loser's stale collection cache after a create race The read that lost the race left a negative cache entry recording the collection as missing, and the winner's purge only reaches its own cache. Without clearing it the loser cannot see the collection at all until the entry expires, which is how the losing process went on to fail a delete of a collection it had just been told already exists. Co-Authored-By: Claude Opus 5 --- src/Database/Database.php | 22 ++++++++++++++++++++ tests/e2e/Adapter/Scopes/CollectionTests.php | 9 +++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 019d00668f..dd9b9377f8 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1917,6 +1917,7 @@ public function createCollection(string $id, array $attributes = [], array $inde $committed = $this->silent(fn () => $this->getDocument(self::METADATA, $id, forUpdate: true)); if (!$committed->isEmpty()) { + $this->purgeStaleCollectionCache($id); throw new DuplicateException('Collection ' . $id . ' already exists'); } @@ -1943,6 +1944,7 @@ public function createCollection(string $id, array $attributes = [], array $inde // A concurrent creator committed the metadata for this id first, so // the physical table is the one its metadata describes. Rolling back // here would drop a live collection out from under it. + $this->purgeStaleCollectionCache($id); throw new DuplicateException('Collection ' . $id . ' already exists', previous: $e); } catch (\Throwable $e) { if ($createdPhysicalTable) { @@ -3627,6 +3629,26 @@ private function cleanupAttributes( return $errors; } + /** + * Drop this instance's metadata cache for a collection a peer created. + * + * The entry records the collection as missing, from a read taken before the + * peer's insert committed. The peer's purge cannot reach it, so without this + * the collection stays invisible to this instance for the rest of the TTL. + * A failure to purge must not replace the caller's DuplicateException. + * + * @param string $collectionId The collection ID + * @return void + */ + private function purgeStaleCollectionCache(string $collectionId): void + { + try { + $this->purgeCachedDocument(self::METADATA, $collectionId); + } catch (\Throwable $e) { + Console::warning("Failed to purge stale cache for collection '{$collectionId}': " . $e->getMessage()); + } + } + /** * Cleanup (delete) a collection with retry logic * diff --git a/tests/e2e/Adapter/Scopes/CollectionTests.php b/tests/e2e/Adapter/Scopes/CollectionTests.php index b8d2e0af81..44b801752b 100644 --- a/tests/e2e/Adapter/Scopes/CollectionTests.php +++ b/tests/e2e/Adapter/Scopes/CollectionTests.php @@ -1905,6 +1905,13 @@ public function testCreateCollectionConcurrentlyKeepsPeerData(): void $metadata = $peer->getCollection($collection); $this->assertFalse($metadata->isEmpty(), 'Peer collection metadata was destroyed by the losing creator'); - $this->assertTrue($peer->deleteCollection($collection)); + // The loser's cache still held the collection as missing from the read + // it took before the peer committed, and the peer's purge cannot reach + // this instance. Losing the race has to clear it, or the collection + // stays invisible here until the entry expires. + $this->assertFalse($database->getCollection($collection)->isEmpty(), 'Losing creator kept a stale empty collection cached'); + $this->assertSame('peer', $database->getDocument($collection, 'written')->getAttribute('name')); + + $this->assertTrue($database->deleteCollection($collection)); } } From af8aa8a9700379f5512f0d89aacfad534886970f Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 18 Aug 2026 20:42:59 +1200 Subject: [PATCH 3/9] fix: do not drop a physical collection when metadata is still uncommitted Empty metadata after a Duplicate table create is also the in-progress peer state, so delete+recreate was still able to destroy a live collection during concurrent boot. Adopt the existing table and let the metadata unique key decide the winner instead. --- src/Database/Database.php | 17 ++--- tests/e2e/Adapter/Scopes/CollectionTests.php | 56 ++++++++++++++++ tests/unit/CreateCollectionRaceTest.php | 70 ++++++++++++++++++++ 3 files changed, 133 insertions(+), 10 deletions(-) create mode 100644 tests/unit/CreateCollectionRaceTest.php diff --git a/src/Database/Database.php b/src/Database/Database.php index dd9b9377f8..d9a9962621 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1921,16 +1921,13 @@ public function createCollection(string $id, array $attributes = [], array $inde throw new DuplicateException('Collection ' . $id . ' already exists'); } - // The collection is absent from metadata but present in the - // physical schema — an orphan from a prior partial failure. - // Drop and recreate to ensure schema matches. - try { - $this->adapter->deleteCollection($id); - } catch (NotFoundException) { - // Already removed by a concurrent reconciler. - } - $this->adapter->createCollection($id, $attributes, $indexes); - $createdPhysicalTable = true; + // Empty metadata is also the in-progress peer state: the table + // exists and the metadata insert has not committed. Dropping + // here is what destroyed live collections during concurrent + // boot. Adopt the existing table and let the metadata unique + // key decide the winner. Same-schema orphans are claimed. + // Closing the remaining window by inserting metadata first + // is #939. } } diff --git a/tests/e2e/Adapter/Scopes/CollectionTests.php b/tests/e2e/Adapter/Scopes/CollectionTests.php index 44b801752b..0382794820 100644 --- a/tests/e2e/Adapter/Scopes/CollectionTests.php +++ b/tests/e2e/Adapter/Scopes/CollectionTests.php @@ -1914,4 +1914,60 @@ public function testCreateCollectionConcurrentlyKeepsPeerData(): void $this->assertTrue($database->deleteCollection($collection)); } + + /** + * A physical collection with no metadata is indistinguishable from a peer + * that has created the table and not yet committed its metadata row. + * createCollection must adopt that table, not drop it. + */ + public function testCreateCollectionAdoptsPhysicalTableWithoutDroppingIt(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $collection = 'preCommitCreate'; + $name = new Document([ + '$id' => ID::custom('name'), + 'type' => Database::VAR_STRING, + 'size' => 128, + 'required' => false, + ]); + + $database->getAdapter()->createCollection($collection, [$name], []); + + $schema = new Document([ + '$id' => $collection, + '$collection' => Database::METADATA, + 'name' => $collection, + 'attributes' => [$name], + 'indexes' => [], + 'documentSecurity' => true, + '$permissions' => [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + ]); + + $database->getAdapter()->createDocument($schema, new Document([ + '$id' => ID::custom('written'), + '$permissions' => [Permission::read(Role::any())], + 'name' => 'peer', + ])); + + $created = $database->createCollection($collection, [$name], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + ]); + + $this->assertSame($collection, $created->getId()); + $this->assertSame( + 'peer', + $database->getDocument($collection, 'written')->getAttribute('name'), + 'Physical collection was dropped while metadata was still uncommitted' + ); + + $this->assertTrue($database->deleteCollection($collection)); + } } diff --git a/tests/unit/CreateCollectionRaceTest.php b/tests/unit/CreateCollectionRaceTest.php new file mode 100644 index 0000000000..b8147483d8 --- /dev/null +++ b/tests/unit/CreateCollectionRaceTest.php @@ -0,0 +1,70 @@ +setDatabase('utopiaTests') + ->setNamespace('create_race_' . uniqid()); + $database->getAuthorization()->addRole(Role::any()->toString()); + $database->create(); + + $collection = 'preCommitCreate'; + $name = new Document([ + '$id' => ID::custom('name'), + 'type' => Database::VAR_STRING, + 'size' => 128, + 'required' => false, + ]); + + $adapter->createCollection($collection, [$name], []); + + $schema = new Document([ + '$id' => $collection, + '$collection' => Database::METADATA, + 'name' => $collection, + 'attributes' => [$name], + 'indexes' => [], + 'documentSecurity' => true, + '$permissions' => [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + ]); + + $adapter->createDocument($schema, new Document([ + '$id' => ID::custom('written'), + '$permissions' => [Permission::read(Role::any())], + 'name' => 'peer', + ])); + + $created = $database->createCollection($collection, [$name], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + ]); + + $this->assertSame($collection, $created->getId()); + $this->assertSame( + 'peer', + $database->getDocument($collection, 'written')->getAttribute('name'), + 'Physical collection was dropped while metadata was still uncommitted' + ); + } +} From 1fd114491e940ad1b00797124878ec788c2289cc Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 18 Aug 2026 20:49:58 +1200 Subject: [PATCH 4/9] test: skip pre-commit adopt coverage on shared tables The adapter-level marker write has no tenant, so the later tenant-scoped read is empty even when the table was kept. Dedicated adapters already prove the drop is gone; the Memory unit test is the red/green regression. --- tests/e2e/Adapter/Scopes/CollectionTests.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/e2e/Adapter/Scopes/CollectionTests.php b/tests/e2e/Adapter/Scopes/CollectionTests.php index 0382794820..4ad5b86ee3 100644 --- a/tests/e2e/Adapter/Scopes/CollectionTests.php +++ b/tests/e2e/Adapter/Scopes/CollectionTests.php @@ -1925,6 +1925,12 @@ public function testCreateCollectionAdoptsPhysicalTableWithoutDroppingIt(): void /** @var Database $database */ $database = $this->getDatabase(); + if ($database->getAdapter()->getSharedTables()) { + $this->expectNotToPerformAssertions(); + + return; + } + $collection = 'preCommitCreate'; $name = new Document([ '$id' => ID::custom('name'), From b1dd33dd9ce36a5d46f1eb8418fa597a3d2fbc38 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 18 Aug 2026 20:57:55 +1200 Subject: [PATCH 5/9] fix: refuse to adopt or drop a physical collection we did not create Empty metadata after Duplicate is either a peer mid-create or an orphan. Dropping destroyed live collections; attaching this caller's metadata to an unknown schema can invent columns that are not there. Leave the table and report Duplicate. Metadata-first claiming is #939. --- src/Database/Database.php | 29 ++++++-------------- tests/e2e/Adapter/Scopes/CollectionTests.php | 21 ++++++++------ tests/unit/CreateCollectionRaceTest.php | 16 +++++++---- 3 files changed, 31 insertions(+), 35 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index d9a9962621..956c9ca9e2 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1908,26 +1908,15 @@ public function createCollection(string $id, array $attributes = [], array $inde // tenants. A DuplicateException simply means the table already // exists for another tenant — not an orphan. } else { - // The metadata check above can be served by a negative cache - // entry that a concurrent creator has not purged yet, because - // the purge only happens once its insert commits. Re-read past - // the cache before concluding the table is an orphan: if the - // metadata is there, the table belongs to that creator and - // dropping it would destroy a live collection. - $committed = $this->silent(fn () => $this->getDocument(self::METADATA, $id, forUpdate: true)); - - if (!$committed->isEmpty()) { - $this->purgeStaleCollectionCache($id); - throw new DuplicateException('Collection ' . $id . ' already exists'); - } - - // Empty metadata is also the in-progress peer state: the table - // exists and the metadata insert has not committed. Dropping - // here is what destroyed live collections during concurrent - // boot. Adopt the existing table and let the metadata unique - // key decide the winner. Same-schema orphans are claimed. - // Closing the remaining window by inserting metadata first - // is #939. + // The table exists and this process did not create it. It may + // belong to a peer that has not committed metadata yet, or it + // may be an orphan. Dropping it destroyed live collections + // during concurrent boot; attaching this caller's metadata to + // an unknown physical schema can invent columns that are not + // there. Leave the table and report Duplicate. Claiming the + // metadata row first is #939. + $this->purgeStaleCollectionCache($id); + throw new DuplicateException('Collection ' . $id . ' already exists'); } } diff --git a/tests/e2e/Adapter/Scopes/CollectionTests.php b/tests/e2e/Adapter/Scopes/CollectionTests.php index 4ad5b86ee3..77f463ca3d 100644 --- a/tests/e2e/Adapter/Scopes/CollectionTests.php +++ b/tests/e2e/Adapter/Scopes/CollectionTests.php @@ -1918,9 +1918,9 @@ public function testCreateCollectionConcurrentlyKeepsPeerData(): void /** * A physical collection with no metadata is indistinguishable from a peer * that has created the table and not yet committed its metadata row. - * createCollection must adopt that table, not drop it. + * createCollection must leave that table alone. */ - public function testCreateCollectionAdoptsPhysicalTableWithoutDroppingIt(): void + public function testCreateCollectionDoesNotDropUncommittedPeerTable(): void { /** @var Database $database */ $database = $this->getDatabase(); @@ -1962,18 +1962,21 @@ public function testCreateCollectionAdoptsPhysicalTableWithoutDroppingIt(): void 'name' => 'peer', ])); - $created = $database->createCollection($collection, [$name], permissions: [ - Permission::read(Role::any()), - Permission::create(Role::any()), - ]); + try { + $database->createCollection($collection, [$name], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + ]); + $this->fail('Expected DuplicateException for an existing physical collection'); + } catch (DuplicateException) { + } - $this->assertSame($collection, $created->getId()); $this->assertSame( 'peer', - $database->getDocument($collection, 'written')->getAttribute('name'), + $database->getAdapter()->getDocument($schema, 'written')->getAttribute('name'), 'Physical collection was dropped while metadata was still uncommitted' ); - $this->assertTrue($database->deleteCollection($collection)); + $database->getAdapter()->deleteCollection($collection); } } diff --git a/tests/unit/CreateCollectionRaceTest.php b/tests/unit/CreateCollectionRaceTest.php index b8147483d8..50afdeda02 100644 --- a/tests/unit/CreateCollectionRaceTest.php +++ b/tests/unit/CreateCollectionRaceTest.php @@ -8,6 +8,7 @@ use Utopia\Database\Adapter\Memory as DatabaseMemory; use Utopia\Database\Database; use Utopia\Database\Document; +use Utopia\Database\Exception\Duplicate as DuplicateException; use Utopia\Database\Helpers\ID; use Utopia\Database\Helpers\Permission; use Utopia\Database\Helpers\Role; @@ -55,15 +56,18 @@ public function testCreateCollectionDoesNotDropUncommittedPeerTable(): void 'name' => 'peer', ])); - $created = $database->createCollection($collection, [$name], permissions: [ - Permission::read(Role::any()), - Permission::create(Role::any()), - ]); + try { + $database->createCollection($collection, [$name], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + ]); + $this->fail('Expected DuplicateException for an existing physical collection'); + } catch (DuplicateException) { + } - $this->assertSame($collection, $created->getId()); $this->assertSame( 'peer', - $database->getDocument($collection, 'written')->getAttribute('name'), + $adapter->getDocument($schema, 'written')->getAttribute('name'), 'Physical collection was dropped while metadata was still uncommitted' ); } From 459232ba2987d25d2da0f9075fc67227984dd465 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 18 Aug 2026 21:04:16 +1200 Subject: [PATCH 6/9] test: allow idempotent adapter creates in the pre-commit race Mongo createCollection does not throw Duplicate for an existing collection, so the process claims metadata instead. The invariant under test is that the physical collection is not dropped. --- tests/e2e/Adapter/Scopes/CollectionTests.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/e2e/Adapter/Scopes/CollectionTests.php b/tests/e2e/Adapter/Scopes/CollectionTests.php index 77f463ca3d..bcbfbe91af 100644 --- a/tests/e2e/Adapter/Scopes/CollectionTests.php +++ b/tests/e2e/Adapter/Scopes/CollectionTests.php @@ -1967,8 +1967,10 @@ public function testCreateCollectionDoesNotDropUncommittedPeerTable(): void Permission::read(Role::any()), Permission::create(Role::any()), ]); - $this->fail('Expected DuplicateException for an existing physical collection'); } catch (DuplicateException) { + // SQL adapters report the existing table as Duplicate. Mongo's + // createCollection is idempotent, so this process continues and + // claims metadata. Either way the physical collection must stay. } $this->assertSame( @@ -1977,6 +1979,10 @@ public function testCreateCollectionDoesNotDropUncommittedPeerTable(): void 'Physical collection was dropped while metadata was still uncommitted' ); - $database->getAdapter()->deleteCollection($collection); + try { + $database->deleteCollection($collection); + } catch (\Throwable) { + $database->getAdapter()->deleteCollection($collection); + } } } From 7d8516925f85f97eebc8a8a232426e81eb0b7e31 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 18 Aug 2026 21:14:02 +1200 Subject: [PATCH 7/9] fix: report Mongo collection-exists as Duplicate on dedicated tables processException already maps the conflict to DuplicateException, but createCollection swallowed it and returned true. Database then inserted metadata over an unknown physical collection. Shared tables and metadata still treat the existing collection as a no-op. --- src/Database/Adapter/Mongo.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index e26e14a55d..c529159468 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -479,7 +479,10 @@ public function createCollection(string $name, array $attributes = [], array $in } catch (MongoException $e) { $e = $this->processException($e); if ($e instanceof DuplicateException) { - return true; + if ($this->getSharedTables() || $name === Database::METADATA) { + return true; + } + throw $e; } // Client throws code-0 "Collection Exists" when its pre-check // finds the collection. In shared-tables/metadata context this From d95c6641b421f722692e488178da968641e3c20e Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 18 Aug 2026 21:43:21 +1200 Subject: [PATCH 8/9] refactor: purge the metadata cache inline on a create race The wrapper only called purgeCachedDocument. Two call sites can do that themselves. --- src/Database/Database.php | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 956c9ca9e2..973eeb32b8 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1915,7 +1915,7 @@ public function createCollection(string $id, array $attributes = [], array $inde // an unknown physical schema can invent columns that are not // there. Leave the table and report Duplicate. Claiming the // metadata row first is #939. - $this->purgeStaleCollectionCache($id); + $this->purgeCachedDocument(self::METADATA, $id); throw new DuplicateException('Collection ' . $id . ' already exists'); } } @@ -1930,7 +1930,7 @@ public function createCollection(string $id, array $attributes = [], array $inde // A concurrent creator committed the metadata for this id first, so // the physical table is the one its metadata describes. Rolling back // here would drop a live collection out from under it. - $this->purgeStaleCollectionCache($id); + $this->purgeCachedDocument(self::METADATA, $id); throw new DuplicateException('Collection ' . $id . ' already exists', previous: $e); } catch (\Throwable $e) { if ($createdPhysicalTable) { @@ -3615,26 +3615,6 @@ private function cleanupAttributes( return $errors; } - /** - * Drop this instance's metadata cache for a collection a peer created. - * - * The entry records the collection as missing, from a read taken before the - * peer's insert committed. The peer's purge cannot reach it, so without this - * the collection stays invisible to this instance for the rest of the TTL. - * A failure to purge must not replace the caller's DuplicateException. - * - * @param string $collectionId The collection ID - * @return void - */ - private function purgeStaleCollectionCache(string $collectionId): void - { - try { - $this->purgeCachedDocument(self::METADATA, $collectionId); - } catch (\Throwable $e) { - Console::warning("Failed to purge stale cache for collection '{$collectionId}': " . $e->getMessage()); - } - } - /** * Cleanup (delete) a collection with retry logic * From bd7ac3e2b0e2ae4776fe348f2f432228438f6200 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 18 Aug 2026 21:58:54 +1200 Subject: [PATCH 9/9] fix: keep DuplicateException when cache purge fails An unguarded purgeCachedDocument on the create-race exits could replace DuplicateException with a cache backend error, so callers that catch Duplicate never see the contract. Swallow the purge failure, log it, and still throw Duplicate. --- src/Database/Database.php | 14 ++++++-- tests/unit/CreateCollectionRaceTest.php | 47 +++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 973eeb32b8..9695708f52 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1915,8 +1915,12 @@ public function createCollection(string $id, array $attributes = [], array $inde // an unknown physical schema can invent columns that are not // there. Leave the table and report Duplicate. Claiming the // metadata row first is #939. - $this->purgeCachedDocument(self::METADATA, $id); - throw new DuplicateException('Collection ' . $id . ' already exists'); + try { + $this->purgeCachedDocument(self::METADATA, $id); + } catch (\Throwable $cacheError) { + Console::warning('Warning: Failed to purge stale collection cache: ' . $cacheError->getMessage()); + } + throw new DuplicateException('Collection ' . $id . ' already exists', previous: $e); } } @@ -1930,7 +1934,11 @@ public function createCollection(string $id, array $attributes = [], array $inde // A concurrent creator committed the metadata for this id first, so // the physical table is the one its metadata describes. Rolling back // here would drop a live collection out from under it. - $this->purgeCachedDocument(self::METADATA, $id); + try { + $this->purgeCachedDocument(self::METADATA, $id); + } catch (\Throwable $cacheError) { + Console::warning('Warning: Failed to purge stale collection cache: ' . $cacheError->getMessage()); + } throw new DuplicateException('Collection ' . $id . ' already exists', previous: $e); } catch (\Throwable $e) { if ($createdPhysicalTable) { diff --git a/tests/unit/CreateCollectionRaceTest.php b/tests/unit/CreateCollectionRaceTest.php index 50afdeda02..6a4393802b 100644 --- a/tests/unit/CreateCollectionRaceTest.php +++ b/tests/unit/CreateCollectionRaceTest.php @@ -71,4 +71,51 @@ public function testCreateCollectionDoesNotDropUncommittedPeerTable(): void 'Physical collection was dropped while metadata was still uncommitted' ); } + + public function testCreateCollectionStillReportsDuplicateWhenCachePurgeFails(): void + { + $cacheAdapter = new class () extends CacheMemory { + public bool $failPurge = false; + + public function purge(string $key, string $hash = ''): bool + { + if ($this->failPurge) { + throw new \RuntimeException('cache backend unavailable'); + } + + return parent::purge($key, $hash); + } + }; + + $adapter = new DatabaseMemory(); + $database = new Database($adapter, new Cache($cacheAdapter)); + $database + ->setDatabase('utopiaTests') + ->setNamespace('create_race_purge_' . uniqid()); + $database->getAuthorization()->addRole(Role::any()->toString()); + $database->create(); + + $collection = 'preCommitCreatePurgeFail'; + $name = new Document([ + '$id' => ID::custom('name'), + 'type' => Database::VAR_STRING, + 'size' => 128, + 'required' => false, + ]); + + $adapter->createCollection($collection, [$name], []); + + $cacheAdapter->failPurge = true; + + try { + $database->createCollection($collection, [$name], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + ]); + $this->fail('Expected DuplicateException even when cache purge fails'); + } catch (DuplicateException $exception) { + $this->assertSame('Collection ' . $collection . ' already exists', $exception->getMessage()); + $this->assertInstanceOf(DuplicateException::class, $exception->getPrevious()); + } + } }