diff --git a/CHANGELOG.md b/CHANGELOG.md index b63e95f..47868a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,7 +32,8 @@ Bugfix release for SFTP locations and large repositories. Tested on a production - Checking and opening a large repository over SFTP is much faster: the index files (one per pack, 27,547 for 1 TB) are downloaded 32 at a time instead of one after the other, 114 instead of 6 files per second to a Hetzner Storage Box (about 4 minutes instead of 77 for 1 TB). A check - also lists the pack folders (256 requests) instead of asking for every pack. + of a repository with more than 256 packs also lists the pack folders (256 requests) instead of + asking for every pack. - App store listing: the author's e-mail address is shown. - App store listing: the homepage link points to the GitHub repository instead of back to the app store page itself. diff --git a/lib/Repository/BlobIndex.php b/lib/Repository/BlobIndex.php index 632fa32..1fcfc3d 100644 --- a/lib/Repository/BlobIndex.php +++ b/lib/Repository/BlobIndex.php @@ -211,6 +211,15 @@ final class BlobIndex { } } + /** Number of packs that hold blobs of the index. */ + public function packCount(): int { + $packs = []; + foreach ($this->blobs as [$pack]) { + $packs[$pack] = true; + } + return count($packs); + } + /** Bytes of all blobs in the index (encrypted size, i.e. what is stored in packs). */ public function storedBytes(): int { $total = 0; diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index 5298596..188266c 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -37,6 +37,8 @@ final class Repository { private ?int $maxBytes = null; private Catalog $catalog; private PackReader $packReader; + /** verify() lists the pack folders from this many packs on (see presentPacks()); tests lower it. */ + public int $listPacksAbove = 256; private function __construct( private IBackend $backend, @@ -803,7 +805,9 @@ final class Repository { /** * Which packs are on the location, from one listing per pack folder instead of a request per * pack. A folder that cannot be listed, or comes back empty (which a storage also reports when - * it could not look), is left out: its packs are then checked one by one. + * it could not look), is left out: its packs are then checked one by one. With no more packs + * than folders nothing is listed: a request per pack is fewer requests then, and on FTP every + * request takes a data connection, of which a server may have only a few. * * @return array{0: array, 1: array} pack id => present, and the * folders that were listed @@ -811,6 +815,9 @@ final class Repository { private function presentPacks(callable $heartbeat): array { $present = []; $listed = []; + if ($this->index->packCount() <= $this->listPacksAbove) { + return [$present, $listed]; + } for ($i = 0; $i < 256; $i++) { $dir = sprintf('%02x', $i); try { diff --git a/tests/Unit/Repository/VerifyTest.php b/tests/Unit/Repository/VerifyTest.php index c210739..480cd95 100644 --- a/tests/Unit/Repository/VerifyTest.php +++ b/tests/Unit/Repository/VerifyTest.php @@ -83,7 +83,9 @@ class VerifyTest extends TestCase { $s = $repo->backupDirectory($this->src); $seen = []; - $this->open()->verify($s['snapshot'], heartbeat: function (array $p) use (&$seen): void { + $open = $this->open(); + $open->listPacksAbove = 0; + $open->verify($s['snapshot'], heartbeat: function (array $p) use (&$seen): void { $seen[] = $p; }); @@ -95,13 +97,32 @@ class VerifyTest extends TestCase { $this->assertNull($last['total'], 'this snapshot has no file count in its metadata (a backup run records one)'); } - public function testVerifyListsPackFoldersInsteadOfCheckingEveryPack(): void { + /** Lists the pack folders however few packs there are. */ + private static function listing(Repository $repo): Repository { + $repo->listPacksAbove = 0; + return $repo; + } + + public function testWithNoMorePacksThanFoldersEveryPackIsCheckedInstead(): void { + // fewer requests; on FTP each takes a data connection, of which a server may have few $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw')); $s = $repo->backupDirectory($this->src); $counting = new ListingBackend($this->backend); $result = Repository::openWithKey($counting, $this->keys, null, null)->verify($s['snapshot']); + $this->assertSame([], $result['missing']); + $this->assertSame(0, $counting->packListings); + $this->assertGreaterThan(0, $counting->packExists); + } + + public function testVerifyListsPackFoldersInsteadOfCheckingEveryPack(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw')); + $s = $repo->backupDirectory($this->src); + $counting = new ListingBackend($this->backend); + + $result = self::listing(Repository::openWithKey($counting, $this->keys, null, null))->verify($s['snapshot']); + $this->assertSame([], $result['missing']); $this->assertSame(0, $counting->packExists); } @@ -111,7 +132,7 @@ class VerifyTest extends TestCase { $s = $repo->backupDirectory($this->src); $counting = new ListingBackend($this->backend, unlistable: true); - $result = Repository::openWithKey($counting, $this->keys, null, null)->verify($s['snapshot']); + $result = self::listing(Repository::openWithKey($counting, $this->keys, null, null))->verify($s['snapshot']); $this->assertSame([], $result['missing']); $this->assertGreaterThan(0, $counting->packExists); @@ -123,7 +144,7 @@ class VerifyTest extends TestCase { $s = $repo->backupDirectory($this->src); $counting = new ListingBackend($this->backend, emptyListings: true); - $result = Repository::openWithKey($counting, $this->keys, null, null)->verify($s['snapshot']); + $result = self::listing(Repository::openWithKey($counting, $this->keys, null, null))->verify($s['snapshot']); $this->assertSame([], $result['missing']); $this->assertGreaterThan(0, $counting->packExists); @@ -140,7 +161,7 @@ class VerifyTest extends TestCase { } $counting = new ListingBackend($this->backend); - $result = Repository::openWithKey($counting, $this->keys, null, null)->verify($s['snapshot']); + $result = self::listing(Repository::openWithKey($counting, $this->keys, null, null))->verify($s['snapshot']); $this->assertNotSame([], $result['missing']); $this->assertSame(0, $counting->packExists); @@ -153,7 +174,7 @@ class VerifyTest extends TestCase { $this->backend->delete($pack); } - $result = Repository::openWithKey(new ListingBackend($this->backend), $this->keys, null, null)->verify($s['snapshot']); + $result = self::listing(Repository::openWithKey(new ListingBackend($this->backend), $this->keys, null, null))->verify($s['snapshot']); $this->assertNotSame([], $result['missing']); } @@ -269,6 +290,7 @@ final class ListingBackend implements IBackend { use \OCA\NgBackup\Backend\GetsOneByOne; public int $packExists = 0; + public int $packListings = 0; public function __construct( private IBackend $inner, @@ -305,6 +327,9 @@ final class ListingBackend implements IBackend { } public function listFiles(string $dir): array { + if (str_starts_with($dir, 'packs/')) { + $this->packListings++; + } if ($this->unlistable && str_starts_with($dir, 'packs/')) { throw new BackendException('Cannot list ' . $dir); }