Skip to content

Commit bff5b43

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #62699 from nextcloud/backport/62562/stable34
[stable34] Optimize propagator
2 parents 2775fe7 + c2c8162 commit bff5b43

3 files changed

Lines changed: 139 additions & 78 deletions

File tree

‎apps/files_trashbin/lib/Trashbin.php‎

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
use OCP\EventDispatcher\IEventDispatcher;
3131
use OCP\EventDispatcher\IEventListener;
3232
use OCP\Exceptions\AbortedEventException;
33+
use OCP\Files\Cache\IPropagator;
3334
use OCP\Files\Events\Node\BeforeNodeDeletedEvent;
3435
use OCP\Files\File;
3536
use OCP\Files\Folder;
@@ -939,21 +940,27 @@ protected static function deleteFiles(array $files, string $user, int|float $ava
939940
$size = 0;
940941

941942
if ($availableSpace <= 0) {
942-
foreach ($files as $file) {
943-
if ($availableSpace <= 0 && $expiration->isExpired($file['mtime'], true)) {
944-
$tmp = self::delete($file['name'], $user, $file['mtime']);
945-
Server::get(LoggerInterface::class)->info(
946-
'remove "' . $file['name'] . '" (' . $tmp . 'B) to meet the limit of trash bin size (50% of available quota) for user "{user}"',
947-
[
948-
'app' => 'files_trashbin',
949-
'user' => $user,
950-
]
951-
);
952-
$availableSpace += $tmp;
953-
$size += $tmp;
954-
} else {
955-
break;
943+
$propagator = self::getUserStoragePropagator($user);
944+
$propagator?->beginBatch();
945+
try {
946+
foreach ($files as $file) {
947+
if ($availableSpace <= 0 && $expiration->isExpired($file['mtime'], true)) {
948+
$tmp = self::delete($file['name'], $user, $file['mtime']);
949+
Server::get(LoggerInterface::class)->info(
950+
'remove "' . $file['name'] . '" (' . $tmp . 'B) to meet the limit of trash bin size (50% of available quota) for user "{user}"',
951+
[
952+
'app' => 'files_trashbin',
953+
'user' => $user,
954+
]
955+
);
956+
$availableSpace += $tmp;
957+
$size += $tmp;
958+
} else {
959+
break;
960+
}
956961
}
962+
} finally {
963+
$propagator?->commitBatch();
957964
}
958965
}
959966
return $size;
@@ -970,10 +977,17 @@ public static function deleteExpiredFiles($files, $user) {
970977
$expiration = Server::get(Expiration::class);
971978
$size = 0;
972979
$count = 0;
973-
foreach ($files as $file) {
974-
$timestamp = $file['mtime'];
975-
$filename = $file['name'];
976-
if ($expiration->isExpired($timestamp)) {
980+
981+
$propagator = self::getUserStoragePropagator($user);
982+
$propagator?->beginBatch();
983+
try {
984+
foreach ($files as $file) {
985+
$timestamp = $file['mtime'];
986+
$filename = $file['name'];
987+
if (!$expiration->isExpired($timestamp)) {
988+
break;
989+
}
990+
977991
try {
978992
$size += self::delete($filename, $user, $timestamp);
979993
$count++;
@@ -993,14 +1007,22 @@ public static function deleteExpiredFiles($files, $user) {
9931007
'user' => $user,
9941008
],
9951009
);
996-
} else {
997-
break;
9981010
}
1011+
} finally {
1012+
$propagator?->commitBatch();
9991013
}
10001014

10011015
return [$size, $count];
10021016
}
10031017

1018+
private static function getUserStoragePropagator(string $user): ?IPropagator {
1019+
try {
1020+
return Server::get(IRootFolder::class)->getUserFolder($user)->getStorage()->getPropagator();
1021+
} catch (\Exception) {
1022+
return null;
1023+
}
1024+
}
1025+
10041026
/**
10051027
* recursive copy to copy a whole directory
10061028
*

‎apps/files_versions/lib/Storage.php‎

Lines changed: 81 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use OCP\Command\IBus;
2727
use OCP\EventDispatcher\IEventDispatcher;
2828
use OCP\Files;
29+
use OCP\Files\Cache\IPropagator;
2930
use OCP\Files\FileInfo;
3031
use OCP\Files\Folder;
3132
use OCP\Files\IMimeTypeDetector;
@@ -229,6 +230,14 @@ public static function markDeletedFile($path) {
229230
'filename' => $filename];
230231
}
231232

233+
private static function getUserStoragePropagator(string $uid): ?IPropagator {
234+
try {
235+
return Server::get(IRootFolder::class)->getUserFolder($uid)->getStorage()->getPropagator();
236+
} catch (\Exception) {
237+
return null;
238+
}
239+
}
240+
232241
/**
233242
* delete the version from the storage and cache
234243
*
@@ -259,10 +268,16 @@ public static function delete($path) {
259268

260269
$versions = self::getVersions($uid, $filename);
261270
if (!empty($versions)) {
262-
foreach ($versions as $v) {
263-
\OC_Hook::emit('\OCP\Versions', 'preDelete', ['path' => $path . $v['version'], 'trigger' => self::DELETE_TRIGGER_MASTER_REMOVED]);
264-
self::deleteVersion($view, $filename . '.v' . $v['version']);
265-
\OC_Hook::emit('\OCP\Versions', 'delete', ['path' => $path . $v['version'], 'trigger' => self::DELETE_TRIGGER_MASTER_REMOVED]);
271+
$propagator = self::getUserStoragePropagator($uid);
272+
$propagator?->beginBatch();
273+
try {
274+
foreach ($versions as $v) {
275+
\OC_Hook::emit('\OCP\Versions', 'preDelete', ['path' => $path . $v['version'], 'trigger' => self::DELETE_TRIGGER_MASTER_REMOVED]);
276+
self::deleteVersion($view, $filename . '.v' . $v['version']);
277+
\OC_Hook::emit('\OCP\Versions', 'delete', ['path' => $path . $v['version'], 'trigger' => self::DELETE_TRIGGER_MASTER_REMOVED]);
278+
}
279+
} finally {
280+
$propagator?->commitBatch();
266281
}
267282
}
268283
}
@@ -614,21 +629,27 @@ public static function expireOlderThanMaxForUser($uid) {
614629
return $version < $threshold;
615630
});
616631

617-
foreach ($versions as $version) {
618-
$internalPath = $version->getInternalPath();
619-
\OC_Hook::emit('\OCP\Versions', 'preDelete', ['path' => $internalPath, 'trigger' => self::DELETE_TRIGGER_RETENTION_CONSTRAINT]);
632+
$propagator = self::getUserStoragePropagator($uid);
633+
$propagator?->beginBatch();
634+
try {
635+
foreach ($versions as $version) {
636+
$internalPath = $version->getInternalPath();
637+
\OC_Hook::emit('\OCP\Versions', 'preDelete', ['path' => $internalPath, 'trigger' => self::DELETE_TRIGGER_RETENTION_CONSTRAINT]);
620638

621-
$versionEntity = isset($versionEntities[$version->getId()]) ? $versionEntities[$version->getId()] : null;
622-
if (!is_null($versionEntity)) {
623-
$versionsMapper->delete($versionEntity);
624-
}
639+
$versionEntity = isset($versionEntities[$version->getId()]) ? $versionEntities[$version->getId()] : null;
640+
if (!is_null($versionEntity)) {
641+
$versionsMapper->delete($versionEntity);
642+
}
625643

626-
try {
627-
$version->delete();
628-
\OC_Hook::emit('\OCP\Versions', 'delete', ['path' => $internalPath, 'trigger' => self::DELETE_TRIGGER_RETENTION_CONSTRAINT]);
629-
} catch (NotPermittedException $e) {
630-
Server::get(LoggerInterface::class)->error("Missing permissions to delete version for user {$uid}: {$internalPath}", ['app' => 'files_versions', 'exception' => $e]);
644+
try {
645+
$version->delete();
646+
\OC_Hook::emit('\OCP\Versions', 'delete', ['path' => $internalPath, 'trigger' => self::DELETE_TRIGGER_RETENTION_CONSTRAINT]);
647+
} catch (NotPermittedException $e) {
648+
Server::get(LoggerInterface::class)->error("Missing permissions to delete version for user {$uid}: {$internalPath}", ['app' => 'files_versions', 'exception' => $e]);
649+
}
631650
}
651+
} finally {
652+
$propagator?->commitBatch();
632653
}
633654
}
634655

@@ -929,47 +950,53 @@ public static function expire($filename, $uid) {
929950
$versionsSize = $versionsSize - $sizeOfDeletedVersions;
930951
}
931952

932-
foreach ($toDelete as $key => $path) {
933-
// Make sure to cleanup version table relations as expire does not pass deleteVersion
934-
try {
935-
/** @var VersionsMapper $versionsMapper */
936-
$versionsMapper = Server::get(VersionsMapper::class);
937-
$file = Server::get(IRootFolder::class)->getUserFolder($uid)->get($filename);
938-
$pathparts = pathinfo($path);
939-
$timestamp = (int)substr($pathparts['extension'] ?? '', 1);
940-
$versionEntity = $versionsMapper->findVersionForFileId($file->getId(), $timestamp);
941-
if ($versionEntity->getMetadataValue('label') !== null && $versionEntity->getMetadataValue('label') !== '') {
942-
continue;
953+
$propagator = self::getUserStoragePropagator($uid);
954+
$propagator?->beginBatch();
955+
try {
956+
foreach ($toDelete as $key => $path) {
957+
// Make sure to cleanup version table relations as expire does not pass deleteVersion
958+
try {
959+
/** @var VersionsMapper $versionsMapper */
960+
$versionsMapper = Server::get(VersionsMapper::class);
961+
$file = Server::get(IRootFolder::class)->getUserFolder($uid)->get($filename);
962+
$pathparts = pathinfo($path);
963+
$timestamp = (int)substr($pathparts['extension'] ?? '', 1);
964+
$versionEntity = $versionsMapper->findVersionForFileId($file->getId(), $timestamp);
965+
if ($versionEntity->getMetadataValue('label') !== null && $versionEntity->getMetadataValue('label') !== '') {
966+
continue;
967+
}
968+
$versionsMapper->delete($versionEntity);
969+
} catch (DoesNotExistException $e) {
943970
}
944-
$versionsMapper->delete($versionEntity);
945-
} catch (DoesNotExistException $e) {
946-
}
947971

948-
\OC_Hook::emit('\OCP\Versions', 'preDelete', ['path' => $path, 'trigger' => self::DELETE_TRIGGER_QUOTA_EXCEEDED]);
949-
self::deleteVersion($versionsFileview, $path);
950-
\OC_Hook::emit('\OCP\Versions', 'delete', ['path' => $path, 'trigger' => self::DELETE_TRIGGER_QUOTA_EXCEEDED]);
951-
unset($allVersions[$key]); // update array with the versions we keep
952-
$logger->info('Expire: ' . $path, ['app' => 'files_versions']);
953-
}
972+
\OC_Hook::emit('\OCP\Versions', 'preDelete', ['path' => $path, 'trigger' => self::DELETE_TRIGGER_QUOTA_EXCEEDED]);
973+
self::deleteVersion($versionsFileview, $path);
974+
\OC_Hook::emit('\OCP\Versions', 'delete', ['path' => $path, 'trigger' => self::DELETE_TRIGGER_QUOTA_EXCEEDED]);
975+
unset($allVersions[$key]); // update array with the versions we keep
976+
$logger->info('Expire: ' . $path, ['app' => 'files_versions']);
977+
}
954978

955-
// Check if enough space is available after versions are rearranged.
956-
// If not we delete the oldest versions until we meet the size limit for versions,
957-
// but always keep the two latest versions
958-
$numOfVersions = count($allVersions) - 2 ;
959-
$i = 0;
960-
// sort oldest first and make sure that we start at the first element
961-
ksort($allVersions);
962-
reset($allVersions);
963-
while ($availableSpace < 0 && $i < $numOfVersions) {
964-
$version = current($allVersions);
965-
\OC_Hook::emit('\OCP\Versions', 'preDelete', ['path' => $version['path'] . '.v' . $version['version'], 'trigger' => self::DELETE_TRIGGER_QUOTA_EXCEEDED]);
966-
self::deleteVersion($versionsFileview, $version['path'] . '.v' . $version['version']);
967-
\OC_Hook::emit('\OCP\Versions', 'delete', ['path' => $version['path'] . '.v' . $version['version'], 'trigger' => self::DELETE_TRIGGER_QUOTA_EXCEEDED]);
968-
$logger->info('running out of space! Delete oldest version: ' . $version['path'] . '.v' . $version['version'], ['app' => 'files_versions']);
969-
$versionsSize -= $version['size'];
970-
$availableSpace += $version['size'];
971-
next($allVersions);
972-
$i++;
979+
// Check if enough space is available after versions are rearranged.
980+
// If not we delete the oldest versions until we meet the size limit for versions,
981+
// but always keep the two latest versions
982+
$numOfVersions = count($allVersions) - 2 ;
983+
$i = 0;
984+
// sort oldest first and make sure that we start at the first element
985+
ksort($allVersions);
986+
reset($allVersions);
987+
while ($availableSpace < 0 && $i < $numOfVersions) {
988+
$version = current($allVersions);
989+
\OC_Hook::emit('\OCP\Versions', 'preDelete', ['path' => $version['path'] . '.v' . $version['version'], 'trigger' => self::DELETE_TRIGGER_QUOTA_EXCEEDED]);
990+
self::deleteVersion($versionsFileview, $version['path'] . '.v' . $version['version']);
991+
\OC_Hook::emit('\OCP\Versions', 'delete', ['path' => $version['path'] . '.v' . $version['version'], 'trigger' => self::DELETE_TRIGGER_QUOTA_EXCEEDED]);
992+
$logger->info('running out of space! Delete oldest version: ' . $version['path'] . '.v' . $version['version'], ['app' => 'files_versions']);
993+
$versionsSize -= $version['size'];
994+
$availableSpace += $version['size'];
995+
next($allVersions);
996+
$i++;
997+
}
998+
} finally {
999+
$propagator?->commitBatch();
9731000
}
9741001

9751002
return $versionsSize; // finally return the new size of the version history

‎lib/private/Files/Cache/Propagator.php‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,10 @@ public function commitBatch(): void {
194194
// Ensure rows are always locked in the same order
195195
uasort($this->batch, static fn (array $a, array $b) => $a['hash'] <=> $b['hash']);
196196

197+
// storages with reliable etags maintain their own etag, so don't churn one
198+
// here on every batched row (matches the check in propagateChange())
199+
$reliableEtag = $this->storage->instanceOfStorage(IReliableEtagStorage::class);
200+
197201
try {
198202
$this->connection->beginTransaction();
199203

@@ -218,17 +222,21 @@ public function commitBatch(): void {
218222
$query = $this->connection->getQueryBuilder();
219223
$query->update('filecache')
220224
->set('mtime', $query->func()->greatest('mtime', $query->createParameter('time')))
221-
->set('etag', $query->expr()->literal(uniqid()))
222225
->where($query->expr()->eq('storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
223226
->andWhere($query->expr()->eq('fileid', $query->createParameter('fileid')));
227+
if (!$reliableEtag) {
228+
$query->set('etag', $query->expr()->literal(uniqid()));
229+
}
224230

225231
$queryWithSize = $this->connection->getQueryBuilder();
226232
$queryWithSize->update('filecache')
227233
->set('mtime', $queryWithSize->func()->greatest('mtime', $queryWithSize->createParameter('time')))
228-
->set('etag', $queryWithSize->expr()->literal(uniqid()))
229234
->set('size', $queryWithSize->func()->add('size', $queryWithSize->createParameter('size')))
230235
->where($queryWithSize->expr()->eq('storage', $queryWithSize->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
231236
->andWhere($queryWithSize->expr()->eq('fileid', $queryWithSize->createParameter('fileid')));
237+
if (!$reliableEtag) {
238+
$queryWithSize->set('etag', $queryWithSize->expr()->literal(uniqid()));
239+
}
232240

233241
while ($row = $result->fetchAssociative()) {
234242
$item = $this->batch[$row['path']];
@@ -249,17 +257,21 @@ public function commitBatch(): void {
249257
$query = $this->connection->getQueryBuilder();
250258
$query->update('filecache')
251259
->set('mtime', $query->func()->greatest('mtime', $query->createParameter('time')))
252-
->set('etag', $query->expr()->literal(uniqid()))
253260
->where($query->expr()->eq('storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
254261
->andWhere($query->expr()->eq('path_hash', $query->createParameter('hash')));
262+
if (!$reliableEtag) {
263+
$query->set('etag', $query->expr()->literal(uniqid()));
264+
}
255265

256266
$queryWithSize = $this->connection->getQueryBuilder();
257267
$queryWithSize->update('filecache')
258268
->set('mtime', $queryWithSize->func()->greatest('mtime', $queryWithSize->createParameter('time')))
259-
->set('etag', $queryWithSize->expr()->literal(uniqid()))
260269
->set('size', $queryWithSize->func()->add('size', $queryWithSize->createParameter('size')))
261270
->where($queryWithSize->expr()->eq('storage', $queryWithSize->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
262271
->andWhere($queryWithSize->expr()->eq('path_hash', $queryWithSize->createParameter('hash')));
272+
if (!$reliableEtag) {
273+
$queryWithSize->set('etag', $queryWithSize->expr()->literal(uniqid()));
274+
}
263275

264276
foreach ($this->batch as $item) {
265277
if ($item['size']) {

0 commit comments

Comments
 (0)