Repository: list flat folders without a type check per entry
Opening a repository lists index/, which holds one file per pack. StorageBackend::list() checked every entry with is_dir(), and on a remote storage each check is a round trip. Over SFTP to a Hetzner Storage Box (33 ms) with a 60 GiB repository, occ backup:run (30 s steps) spent nearly every step on these requests and uploaded 45 KiB/s; with this change it uploads 5.8 MB/s. The cost grows with the number of packs. index/, snapshots/, catalog/ and trash/info/ only ever hold objects: list them with the new IBackend::listFiles(), one listing without per-entry checks.
This commit is contained in:
@@ -0,0 +1,47 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace OCA\NgBackup\Tests\Unit\Backend;
|
||||
|
||||
use OCA\NgBackup\Backend\StorageBackend;
|
||||
use OCA\NgBackup\Tests\Unit\TempDirTrait;
|
||||
use OCP\Files\Storage\IStorage;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
/** On a remote storage every is_dir() is a round trip: listing index/ must not cost one per pack. */
|
||||
class StorageBackendListTest extends TestCase {
|
||||
use TempDirTrait;
|
||||
|
||||
public function testListFilesChecksOnlyTheFolderItself(): void {
|
||||
$dir = $this->tempDir();
|
||||
foreach (['b1', 'a2', 'c3.part-0123abcd'] as $name) {
|
||||
touch("$dir/$name");
|
||||
}
|
||||
$isDirCalls = 0;
|
||||
$storage = $this->createMock(IStorage::class);
|
||||
$storage->method('instanceOfStorage')->willReturn(false);
|
||||
$storage->method('is_dir')->willReturnCallback(function (string $path) use (&$isDirCalls): bool {
|
||||
$isDirCalls++;
|
||||
return $path === 'repo' || $path === 'repo/index';
|
||||
});
|
||||
$storage->method('opendir')->willReturnCallback(fn (string $path) => $path === 'repo/index' ? opendir($dir) : false);
|
||||
|
||||
$backend = new StorageBackend($storage, 'repo');
|
||||
$isDirCalls = 0; // the constructor checks the base folder
|
||||
$this->assertSame(['index/a2', 'index/b1'], $backend->listFiles('index'));
|
||||
$this->assertSame(1, $isDirCalls);
|
||||
|
||||
$isDirCalls = 0;
|
||||
$this->assertSame(['index/a2', 'index/b1'], $backend->list('index'));
|
||||
$this->assertSame(4, $isDirCalls, 'list() checks every entry: the reason listFiles() exists');
|
||||
}
|
||||
|
||||
public function testListFilesOfAMissingFolderIsEmpty(): void {
|
||||
$storage = $this->createMock(IStorage::class);
|
||||
$storage->method('instanceOfStorage')->willReturn(false);
|
||||
$storage->method('is_dir')->willReturnCallback(fn (string $path): bool => $path === 'repo');
|
||||
$storage->expects($this->never())->method('opendir');
|
||||
$this->assertSame([], (new StorageBackend($storage, 'repo'))->listFiles('snapshots'));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user