upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-02-19 02:38:40 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 11

perf(Propagator): Always commit in batches

Browse source 
Signed-off-by: provokateurin <kate@provokateurin.de>
This commit is contained in:
provokateurin 2026-02-18 16:30:15 +01:00
parent a6d1fa1fa6
commit 15d2b5bd7a
No known key found for this signature in database
4 changed files with 17 additions and 110 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

103
lib/private/Files/Cache/Propagator.php
View file

@ -9,12 +9,8 @@ declare(strict_types=1);
*/
namespace OC\Files\Cache;
use OC\DB\Exceptions\DbalException;
use OC\Files\Storage\Wrapper\Encryption;
use OCP\DB\QueryBuilder\ILiteral;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Files\Cache\IPropagator;
use OCP\Files\Storage\IReliableEtagStorage;
use OCP\Files\Storage\IStorage;
use OCP\IDBConnection;
use OCP\Server;
@ -23,9 +19,6 @@ use Psr\Clock\ClockInterface;
use Psr\Log\LoggerInterface;
class Propagator implements IPropagator {
public const MAX_RETRIES = 3;
private bool $inBatch = false;
private array $batch = [];
private ClockInterface $clock;
@ -37,6 +30,10 @@ class Propagator implements IPropagator {
$this->clock = Server::get(ClockInterface::class);
}
public function __destruct() {
$this->commitBatch();
}
#[Override]
public function propagateChange(string $internalPath, int $time, int $sizeDifference = 0): void {
// Do not propagate changes in ignored paths
@ -48,91 +45,10 @@ class Propagator implements IPropagator {
$time = min($time, $this->clock->now()->getTimestamp());
$storageId = $this->storage->getCache()->getNumericStorageId();
$parents = $this->getParents($internalPath);
if ($this->inBatch) {
foreach ($parents as $parent) {
$this->addToBatch($parent, $time, $sizeDifference);
}
return;
}
$parentHashes = array_map('md5', $parents);
sort($parentHashes); // Ensure rows are always locked in the same order
$etag = uniqid(); // since we give all folders the same etag we don't ask the storage for the etag
$builder = $this->connection->getQueryBuilder();
$hashParams = array_map(static fn (string $hash): ILiteral => $builder->expr()->literal($hash), $parentHashes);
$builder->update('filecache')
->set('mtime', $builder->func()->greatest('mtime', $builder->createNamedParameter($time, IQueryBuilder::PARAM_INT)))
->where($builder->expr()->eq('storage', $builder->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
->andWhere($builder->expr()->in('path_hash', $hashParams));
if (!$this->storage->instanceOfStorage(IReliableEtagStorage::class)) {
$builder->set('etag', $builder->createNamedParameter($etag, IQueryBuilder::PARAM_STR));
}
if ($sizeDifference !== 0) {
$hasCalculatedSize = $builder->expr()->gt('size', $builder->expr()->literal(-1, IQUeryBuilder::PARAM_INT));
$sizeColumn = $builder->getColumnName('size');
$newSize = $builder->func()->greatest(
$builder->func()->add('size', $builder->createNamedParameter($sizeDifference)),
$builder->createNamedParameter(-1, IQueryBuilder::PARAM_INT)
);
// Only update if row had a previously calculated size
$builder->set('size', $builder->createFunction("CASE WHEN $hasCalculatedSize THEN $newSize ELSE $sizeColumn END"));
if ($this->storage->instanceOfStorage(Encryption::class)) {
// in case of encryption being enabled after some files are already uploaded, some entries will have an unencrypted_size of 0 and a non-zero size
$hasUnencryptedSize = $builder->expr()->neq('unencrypted_size', $builder->expr()->literal(0, IQueryBuilder::PARAM_INT));
$sizeColumn = $builder->getColumnName('size');
$unencryptedSizeColumn = $builder->getColumnName('unencrypted_size');
$newUnencryptedSize = $builder->func()->greatest(
$builder->func()->add(
$builder->createFunction("CASE WHEN $hasUnencryptedSize THEN $unencryptedSizeColumn ELSE $sizeColumn END"),
$builder->createNamedParameter($sizeDifference)
),
$builder->createNamedParameter(-1, IQueryBuilder::PARAM_INT)
);
// Only update if row had a previously calculated size
$builder->set('unencrypted_size', $builder->createFunction("CASE WHEN $hasCalculatedSize THEN $newUnencryptedSize ELSE $unencryptedSizeColumn END"));
}
}
for ($i = 0; $i < self::MAX_RETRIES; $i++) {
try {
if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_SQLITE) {
$this->connection->beginTransaction();
// Lock all the rows first with a SELECT FOR UPDATE ordered by path_hash
$forUpdate = $this->connection->getQueryBuilder();
$forUpdate->select('fileid')
->from('filecache')
->where($forUpdate->expr()->eq('storage', $forUpdate->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
->andWhere($forUpdate->expr()->in('path_hash', $hashParams))
->orderBy('path_hash')
->forUpdate()
->executeQuery();
$builder->executeStatement();
$this->connection->commit();
} else {
$builder->executeStatement();
}
break;
} catch (DbalException $e) {
if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_SQLITE) {
$this->connection->rollBack();
}
if (!$e->isRetryable()) {
throw $e;
}
$loggerInterface = Server::get(LoggerInterface::class);
$loggerInterface->warning('Retrying propagation query after retryable exception.', [ 'exception' => $e ]);
}
foreach ($parents as $parent) {
$this->addToBatch($parent, $time, $sizeDifference);
}
}
@ -152,7 +68,7 @@ class Propagator implements IPropagator {
#[Override]
public function beginBatch(): void {
$this->inBatch = true;
Server::get(LoggerInterface::class)->debug('Propagated changes are committed in batches automatically, so it is no longer necessary to begin a batch manually.');
}
private function addToBatch(string $internalPath, int $time, int $sizeDifference): void {
@ -172,11 +88,6 @@ class Propagator implements IPropagator {
#[Override]
public function commitBatch(): void {
if (!$this->inBatch) {
throw new \BadMethodCallException('Not in batch');
}
$this->inBatch = false;
// Ensure rows are always locked in the same order
uasort($this->batch, static fn (array $a, array $b) => $a['hash'] <=> $b['hash']);

16
lib/private/Files/Utils/Scanner.php
View file

@ -147,7 +147,6 @@ class Scanner extends PublicEmitter {
});
$propagator = $storage->getPropagator();
$propagator->beginBatch();
$scanner->backgroundScan();
$propagator->commitBatch();
} catch (\Exception $e) {
@ -243,7 +242,6 @@ class Scanner extends PublicEmitter {
}
try {
$propagator = $storage->getPropagator();
$propagator->beginBatch();
try {
$scanner->scan($relativePath, $recursive, \OC\Files\Cache\Scanner::REUSE_ETAG | \OC\Files\Cache\Scanner::REUSE_SIZE);
} catch (LockedException $e) {
@ -275,16 +273,10 @@ class Scanner extends PublicEmitter {
private function postProcessEntry(IStorage $storage, $internalPath) {
$this->triggerPropagator($storage, $internalPath);
if ($this->useTransaction) {
$this->entriesToCommit++;
if ($this->entriesToCommit >= self::MAX_ENTRIES_TO_COMMIT) {
$propagator = $storage->getPropagator();
$this->entriesToCommit = 0;
$this->db->commit();
$propagator->commitBatch();
$this->db->beginTransaction();
$propagator->beginBatch();
}
$this->entriesToCommit++;
if ($this->entriesToCommit >= self::MAX_ENTRIES_TO_COMMIT) {
$this->entriesToCommit = 0;
$storage->getPropagator()->commitBatch();
}
}
}

1
lib/public/Files/Cache/IPropagator.php
View file

@ -28,6 +28,7 @@ interface IPropagator {
* before the batch is committed.
*
* @since 9.1.0
* @depecated 34.0.0 Propagated changes are committed in batches automatically, so it is no longer necessary to begin a batch manually.
*/
public function beginBatch(): void;

7
tests/lib/Files/Cache/PropagatorTest.php
View file

@ -40,6 +40,7 @@ class PropagatorTest extends TestCase {
$paths = ['', 'foo', 'foo/bar'];
$oldInfos = $this->getFileInfos($paths);
$this->storage->getPropagator()->propagateChange('foo/bar/file.txt', time());
$this->storage->getPropagator()->commitBatch();
$newInfos = $this->getFileInfos($paths);
foreach ($oldInfos as $i => $oldInfo) {
@ -58,6 +59,7 @@ class PropagatorTest extends TestCase {
$cache->put('foo/bar', ['mtime' => $oldTime]);
$cache->put('foo/bar/file.txt', ['mtime' => $oldTime]);
$this->storage->getPropagator()->propagateChange('foo/bar/file.txt', $targetTime);
$this->storage->getPropagator()->commitBatch();
$newInfos = $this->getFileInfos($paths);
$this->assertEquals($targetTime, $newInfos['foo/bar']->getMTime());
@ -71,6 +73,7 @@ class PropagatorTest extends TestCase {
$paths = ['', 'foo', 'foo/bar'];
$oldInfos = $this->getFileInfos($paths);
$this->storage->getPropagator()->propagateChange('foo/bar/file.txt', time(), 10);
$this->storage->getPropagator()->commitBatch();
$newInfos = $this->getFileInfos($paths);
foreach ($oldInfos as $i => $oldInfo) {
@ -82,10 +85,11 @@ class PropagatorTest extends TestCase {
$paths = ['', 'foo', 'foo/bar'];
$oldInfos = $this->getFileInfos($paths);
$this->storage->getPropagator()->propagateChange('foo/bar/file.txt', time(), -100);
$this->storage->getPropagator()->commitBatch();
$newInfos = $this->getFileInfos($paths);
foreach ($oldInfos as $i => $oldInfo) {
$this->assertEquals(-1, $newInfos[$i]->getSize());
$this->assertEquals(-97, $newInfos[$i]->getSize());
}
}
@ -101,7 +105,6 @@ class PropagatorTest extends TestCase {
$oldInfos = $this->getFileInfos($paths);
$propagator = $this->storage->getPropagator();
$propagator->beginBatch();
$propagator->propagateChange('asd/file.txt', time(), 10);
$propagator->propagateChange('foo/bar/file.txt', time(), 2);