From 62766e1e71f32b1b85e5a5263e1e6611b3f8608b Mon Sep 17 00:00:00 2001 From: Dillard Blom Date: Thu, 8 Oct 2026 13:03:52 +0200 Subject: [PATCH] Backup: do not write the index cache inside the read-only dump transaction A database dump that fills a whole pack (32 MiB) uploads that pack while the dump transaction is still open, and the pack's index cache row was inserted on the same connection. PostgreSQL refuses that (25006, read-only transaction) and the run failed; on other databases the row was rolled back with the dump transaction. The index now holds those cache writes back until the transaction is over. --- lib/Db/DbDumper.php | 4 +++ lib/Repository/BlobIndex.php | 46 ++++++++++++++++++++++-- lib/Repository/Repository.php | 9 +++++ tests/Unit/Repository/IndexCacheTest.php | 37 +++++++++++++++++++ 4 files changed, 94 insertions(+), 2 deletions(-) diff --git a/lib/Db/DbDumper.php b/lib/Db/DbDumper.php index 2202a4c..841f9d0 100644 --- a/lib/Db/DbDumper.php +++ b/lib/Db/DbDumper.php @@ -72,6 +72,9 @@ final class DbDumper { public function dumpStep(Repository $repo, array $state, float $deadline, array &$stats): array { $state['steps']++; $sm = $this->db->createSchemaManager(); + // A pack that fills up during the dump is uploaded right away; its index cache row must + // wait until the read-only transaction is over. + $repo->holdIndexCache(); $this->db->setTransactionIsolation(TransactionIsolationLevel::REPEATABLE_READ); $this->db->beginTransaction(); try { @@ -116,6 +119,7 @@ final class DbDumper { } } finally { $this->db->rollBack(); + $repo->releaseIndexCache(); } $repo->flushPacks(); return $state; diff --git a/lib/Repository/BlobIndex.php b/lib/Repository/BlobIndex.php index cae45cf..8831e49 100644 --- a/lib/Repository/BlobIndex.php +++ b/lib/Repository/BlobIndex.php @@ -25,6 +25,8 @@ final class BlobIndex { private array $pending = []; private ?IIndexCache $cache = null; + /** @var list<\Closure():void>|null cache writes held back by holdCache(), or null when writing directly */ + private ?array $heldCacheWrites = null; private string $repositoryId = ''; /** @var int index files downloaded by the last load() (for tests and diagnostics) */ public int $downloaded = 0; @@ -40,6 +42,46 @@ final class BlobIndex { $this->repositoryId = $repositoryId; } + /** + * Hold back cache writes until releaseCache(). The database dump reads inside a read-only + * transaction on the same connection that the cache writes to: a write there fails on + * PostgreSQL and would be rolled back with the dump transaction elsewhere. + */ + public function holdCache(): void { + $this->heldCacheWrites ??= []; + } + + /** Write the cache changes held back since holdCache(). */ + public function releaseCache(): void { + $writes = $this->heldCacheWrites ?? []; + $this->heldCacheWrites = null; + foreach ($writes as $write) { + $write(); + } + } + + /** @param list $entries */ + private function cachePut(string $packId, array $entries): void { + if ($this->heldCacheWrites !== null) { + $this->heldCacheWrites[] = function () use ($packId, $entries): void { + $this->cache?->put($this->repositoryId, $packId, $entries); + }; + return; + } + $this->cache?->put($this->repositoryId, $packId, $entries); + } + + /** @param list $packIds */ + private function cacheRemove(array $packIds): void { + if ($this->heldCacheWrites !== null) { + $this->heldCacheWrites[] = function () use ($packIds): void { + $this->cache?->remove($this->repositoryId, $packIds); + }; + return; + } + $this->cache?->remove($this->repositoryId, $packIds); + } + /** * Load the index: one listing of index/, entries from the local cache where available, * and only the index files that are not cached yet are downloaded. Cached packs that no @@ -112,7 +154,7 @@ final class BlobIndex { fwrite($stream, $this->cipher->encryptString(json_encode($entries, JSON_THROW_ON_ERROR), 'index:' . $packId)); rewind($stream); $this->backend->put('index/' . $packId, $stream); - $this->cache?->put($this->repositoryId, $packId, $entries); + $this->cachePut($packId, $entries); foreach ($entries as $e) { $this->blobs[$e['id']] = [$packId, $e['offset'], $e['length'], $e['raw'], $e['flags']]; unset($this->pending[$e['id']]); @@ -131,7 +173,7 @@ final class BlobIndex { /** Forget a pack: delete its index file and cache row. Blobs that now live elsewhere are kept. */ public function removePack(string $packId): void { $this->backend->delete('index/' . $packId); - $this->cache?->remove($this->repositoryId, [$packId]); + $this->cacheRemove([$packId]); foreach ($this->blobs as $id => $loc) { if ($loc[0] === $packId) { unset($this->blobs[$id]); diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index 9a383ae..8c98ce3 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -262,6 +262,15 @@ final class Repository { }); } + /** See BlobIndex::holdCache(): no database writes until releaseIndexCache(). */ + public function holdIndexCache(): void { + $this->index->holdCache(); + } + + public function releaseIndexCache(): void { + $this->index->releaseCache(); + } + public function flushPacks(): void { $this->runPacks?->flush(); } diff --git a/tests/Unit/Repository/IndexCacheTest.php b/tests/Unit/Repository/IndexCacheTest.php index c2b0c5b..73ecdd3 100644 --- a/tests/Unit/Repository/IndexCacheTest.php +++ b/tests/Unit/Repository/IndexCacheTest.php @@ -51,4 +51,41 @@ class IndexCacheTest extends TestCase { $fourth->restore($r1['snapshot'], $target); $this->assertCount(12, self::hashTree($target)); } + + /** The database dump reads in a read-only transaction: a pack that fills up then must not write the cache yet. */ + public function testHeldCacheWritesWaitForRelease(): void { + $cache = new class implements IIndexCache { + public array $data = []; + public bool $readOnly = false; + public function all(string $r): array { return $this->data[$r] ?? []; } + public function put(string $r, string $p, array $e): void { + if ($this->readOnly) { + throw new \RuntimeException('cannot execute INSERT in a read-only transaction'); + } + $this->data[$r][$p] = $e; + } + public function remove(string $r, array $ps): void { foreach ($ps as $p) { unset($this->data[$r][$p]); } } + }; + $backend = new LocalBackend($this->tempDir()); + $keys = KeyRing::generate(); + Repository::initWithKey($backend, $keys, $keys->wrap('pw')); + $repo = Repository::openWithKey($backend, $keys, $cache); + + $stats = []; + $repo->holdIndexCache(); + $cache->readOnly = true; + $writer = $repo->blobWriter($stats); + for ($i = 0; $i < 40; $i++) { + $writer->write(random_bytes(1048576)); // more than one 32 MiB pack + } + $writer->finish(); + $this->assertGreaterThan(0, count($backend->list('index')), 'a full pack was uploaded during the hold'); + $this->assertSame([], $cache->data); + + $cache->readOnly = false; + $repo->releaseIndexCache(); + $repo->flushPacks(); + $this->assertCount(count($backend->list('index')), array_merge(...array_values($cache->data))); + $this->assertSame(0, Repository::openWithKey($backend, $keys, $cache)->indexDownloads()); + } }