Verify: list the pack folders only with more packs than folders
With few packs a request per pack is fewer requests than 256 listings. On
FTP every listing takes a data connection; the FTP server of the integration
tests has ten passive ports, and 256 listings in a row slowed it down until a
read failed ("Not found: snapshots/...").
This commit is contained in:
+2
-1
@@ -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.
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<string, bool>, 1: array<string, true>} 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 {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user