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