From 3c9da56116eda1f7064a694fa77f1ce96831fe95 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Tue, 10 Jun 2025 17:41:57 +0200 Subject: [PATCH 1/3] feat: Implement custom Updater for object store storage Add a custom wrapper around the Updater for ObjectStoreStorage. This wrapper will skip updating the cache in some scenario. This is because a lot of cache management is already done in ObjectStoreStorage, so this will prevent heavy operations. Signed-off-by: Louis Chemineau --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Files/Cache/Updater.php | 2 +- .../Files/ObjectStore/ObjectStoreStorage.php | 19 +++++++++++++++++ .../Files/ObjectStore/ObjectStoreUpdater.php | 21 +++++++++++++++++++ lib/public/Files/Cache/IUpdater.php | 4 ++++ 6 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 lib/private/Files/ObjectStore/ObjectStoreUpdater.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 3bf63efce0ffb..b4b2b014cf604 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1649,6 +1649,7 @@ 'OC\\Files\\ObjectStore\\Mapper' => $baseDir . '/lib/private/Files/ObjectStore/Mapper.php', 'OC\\Files\\ObjectStore\\ObjectStoreScanner' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreScanner.php', 'OC\\Files\\ObjectStore\\ObjectStoreStorage' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreStorage.php', + 'OC\\Files\\ObjectStore\\ObjectStoreUpdater' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreUpdater.php', 'OC\\Files\\ObjectStore\\PrimaryObjectStoreConfig' => $baseDir . '/lib/private/Files/ObjectStore/PrimaryObjectStoreConfig.php', 'OC\\Files\\ObjectStore\\S3' => $baseDir . '/lib/private/Files/ObjectStore/S3.php', 'OC\\Files\\ObjectStore\\S3ConfigTrait' => $baseDir . '/lib/private/Files/ObjectStore/S3ConfigTrait.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index f076aaca78302..49b7949f49516 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1690,6 +1690,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Files\\ObjectStore\\Mapper' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/Mapper.php', 'OC\\Files\\ObjectStore\\ObjectStoreScanner' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreScanner.php', 'OC\\Files\\ObjectStore\\ObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreStorage.php', + 'OC\\Files\\ObjectStore\\ObjectStoreUpdater' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreUpdater.php', 'OC\\Files\\ObjectStore\\PrimaryObjectStoreConfig' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/PrimaryObjectStoreConfig.php', 'OC\\Files\\ObjectStore\\S3' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3.php', 'OC\\Files\\ObjectStore\\S3ConfigTrait' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3ConfigTrait.php', diff --git a/lib/private/Files/Cache/Updater.php b/lib/private/Files/Cache/Updater.php index 03681036aa211..17d4062bd691c 100644 --- a/lib/private/Files/Cache/Updater.php +++ b/lib/private/Files/Cache/Updater.php @@ -272,7 +272,7 @@ private function updateStorageMTimeOnly($internalPath) { * * @param string $internalPath */ - private function correctParentStorageMtime($internalPath) { + public function correctParentStorageMtime($internalPath) { $parentId = $this->cache->getParentId($internalPath); $parent = dirname($internalPath); if ($parentId != -1) { diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index 752c3cf4fb79e..9100eb92ebd4f 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -18,6 +18,7 @@ use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\Cache\IScanner; +use OCP\Files\Cache\IUpdater; use OCP\Files\FileInfo; use OCP\Files\GenericFileException; use OCP\Files\NotFoundException; @@ -464,6 +465,7 @@ public function writeStream(string $path, $stream, ?int $size = null): int { } // update stat with new data $mTime = time(); + $oldSize = $stat['size'] ?? 0; $stat['size'] = (int)$size; $stat['mtime'] = $mTime; $stat['storage_mtime'] = $mTime; @@ -561,6 +563,9 @@ public function writeStream(string $path, $stream, ?int $size = null): int { } } + $this->getUpdater()->correctParentStorageMtime($path); + $this->propagator->propagateChange($path, $mTime, $stat['size'] - $oldSize); + return $size; } @@ -814,4 +819,18 @@ public function cancelChunkedWrite(string $targetPath, string $writeToken): void public function setPreserveCacheOnDelete(bool $preserve) { $this->preserveCacheItemsOnDelete = $preserve; } + + public function getUpdater(?IStorage $storage = null): IUpdater { + if (!$storage) { + $storage = $this; + } + if (!$storage->instanceOfStorage(self::class)) { + throw new \InvalidArgumentException('Storage is not of the correct class'); + } + /** @var self $storage */ + if (!isset($storage->updater)) { + $storage->updater = new ObjectStoreUpdater($storage); + } + return $storage->updater; + } } diff --git a/lib/private/Files/ObjectStore/ObjectStoreUpdater.php b/lib/private/Files/ObjectStore/ObjectStoreUpdater.php new file mode 100644 index 0000000000000..de78ad8f0697c --- /dev/null +++ b/lib/private/Files/ObjectStore/ObjectStoreUpdater.php @@ -0,0 +1,21 @@ + Date: Thu, 12 Jun 2025 18:16:57 +0200 Subject: [PATCH 2/3] fix: fix incorrect filesize for object store touch Signed-off-by: Robin Appelman --- lib/private/Files/ObjectStore/ObjectStoreStorage.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index 9100eb92ebd4f..2bfd736e11f59 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -14,11 +14,11 @@ use Icewind\Streams\IteratorDirectory; use OC\Files\Cache\Cache; use OC\Files\Cache\CacheEntry; +use OC\Files\Cache\Updater; use OC\Files\Storage\PolyFill\CopyDirectory; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\Cache\IScanner; -use OCP\Files\Cache\IUpdater; use OCP\Files\FileInfo; use OCP\Files\GenericFileException; use OCP\Files\NotFoundException; @@ -820,7 +820,7 @@ public function setPreserveCacheOnDelete(bool $preserve) { $this->preserveCacheItemsOnDelete = $preserve; } - public function getUpdater(?IStorage $storage = null): IUpdater { + public function getUpdater(?IStorage $storage = null): Updater { if (!$storage) { $storage = $this; } From b6766b5a7719ad6b1186e5e9670aa400784c98c5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 24 Jun 2025 18:55:27 +0200 Subject: [PATCH 3/3] perf: also do updater optimization for deleting for object store Signed-off-by: Robin Appelman --- .../Files/ObjectStore/ObjectStoreStorage.php | 30 ++++++++++++++++--- .../Files/ObjectStore/ObjectStoreUpdater.php | 4 +++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index 2bfd736e11f59..0f5d9ec6aa144 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -153,7 +153,23 @@ public function rmdir(string $path): bool { return false; } - return $this->rmObjects($entry); + $result = $this->rmObjects($entry); + if ($result) { + $this->removeFromCache($entry); + } + return $result; + } + + /** + * Remove an item from cache and propagate the change to the parent folders. + * + * Similar logic to the `Updater` but done in-storage because we have the correct info to avoid expensive + * folder size calculations. + */ + private function removeFromCache(ICacheEntry $entry): void { + $this->cache->remove($entry->getId()); + $this->getUpdater()->correctParentStorageMtime($entry->getPath()); + $this->propagator->propagateChange($entry->getPath(), time(), -$entry->getSize()); } private function rmObjects(ICacheEntry $entry): bool { @@ -180,15 +196,21 @@ private function rmObjects(ICacheEntry $entry): bool { public function unlink(string $path): bool { $path = $this->normalizePath($path); $entry = $this->getCache()->get($path); + $result = false; if ($entry instanceof ICacheEntry) { if ($entry->getMimeType() === ICacheEntry::DIRECTORY_MIMETYPE) { - return $this->rmObjects($entry); + $result = $this->rmObjects($entry); } else { - return $this->rmObject($entry); + $result = $this->rmObject($entry); } } - return false; + + if ($result) { + $this->removeFromCache($entry); + } + + return $result; } public function rmObject(ICacheEntry $entry): bool { diff --git a/lib/private/Files/ObjectStore/ObjectStoreUpdater.php b/lib/private/Files/ObjectStore/ObjectStoreUpdater.php index de78ad8f0697c..b35d883ae707b 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreUpdater.php +++ b/lib/private/Files/ObjectStore/ObjectStoreUpdater.php @@ -18,4 +18,8 @@ class ObjectStoreUpdater extends Updater { public function update($path, $time = null, ?int $sizeDifference = null) { // Noop } + + public function remove($path) { + // Noop + } }