From 9fc985b6a3849a4ae10a6066f8148bc6b21cedec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 29 Jul 2021 12:59:25 +0200 Subject: [PATCH 1/4] Already pass size difference to the cache updater to avoid full calculation on s3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/dav/lib/Connector/Sabre/File.php | 3 ++- lib/private/Files/Cache/Updater.php | 12 +++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 3a871b06259d1..f1ba0d716f9be 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -236,6 +236,7 @@ public function put($data) { } } + $previousFileSize = $exists ? $partStorage->filesize($internalPath) : 0; if ($partStorage->instanceOfStorage(Storage\IWriteStreamStorage::class)) { $isEOF = false; $wrappedData = CallbackWrapper::wrap($data, null, null, null, null, function ($stream) use (&$isEOF) { @@ -365,7 +366,7 @@ public function put($data) { } // since we skipped the view we need to scan and emit the hooks ourselves - $storage->getUpdater()->update($internalPath); + $storage->getUpdater()->update($internalPath, null, ($count-$previousFileSize)); try { $this->changeLock(ILockingProvider::LOCK_SHARED); diff --git a/lib/private/Files/Cache/Updater.php b/lib/private/Files/Cache/Updater.php index f8c187996e69d..acccf37c3727c 100644 --- a/lib/private/Files/Cache/Updater.php +++ b/lib/private/Files/Cache/Updater.php @@ -113,8 +113,9 @@ public function propagate($path, $time = null) { * * @param string $path * @param int $time + * @param int $sizeDifference */ - public function update($path, $time = null) { + public function update($path, $time = null, $sizeDifference = null) { if (!$this->enabled or Scanner::isPartialFile($path)) { return; } @@ -129,14 +130,15 @@ public function update($path, $time = null) { ) { $sizeDifference = $data['size'] - $data['oldSize']; } else { - // scanner didn't provide size info, fallback to full size calculation - $sizeDifference = 0; - if ($this->cache instanceof Cache) { + // scanner didn't provide size info, fallback to full size calculation if the difference was not already passed + // otherwise we can update through the propagator + if ($this->cache instanceof Cache && $sizeDifference === null) { $this->cache->correctFolderSize($path, $data); + $sizeDifference = 0; } } $this->correctParentStorageMtime($path); - $this->propagator->propagateChange($path, $time, $sizeDifference); + $this->propagator->propagateChange($path, $time, $sizeDifference ?? 0); } /** From 59275e2c2fd8feb7c38d362e769321b51b0cd44c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 5 Aug 2021 09:32:19 +0200 Subject: [PATCH 2/4] Make written byte counter variable name more obvious MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/dav/lib/Connector/Sabre/File.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index f1ba0d716f9be..6b9bae329e788 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -244,9 +244,9 @@ public function put($data) { }); $result = true; - $count = -1; + $writtenByteCount = -1; try { - $count = $partStorage->writeStream($internalPartPath, $wrappedData); + $writtenByteCount = $partStorage->writeStream($internalPartPath, $wrappedData); } catch (GenericFileException $e) { $result = false; } catch (BadGateway $e) { @@ -267,7 +267,7 @@ public function put($data) { // because we have no clue about the cause we can only throw back a 500/Internal Server Error throw new Exception($this->l10n->t('Could not write file contents')); } - [$count, $result] = \OC_Helper::streamCopy($data, $target); + [$writtenByteCount, $result] = \OC_Helper::streamCopy($data, $target); fclose($target); } @@ -281,7 +281,7 @@ public function put($data) { $this->l10n->t( 'Error while copying file to target location (copied: %1$s, expected filesize: %2$s)', [ - $this->l10n->n('%n byte', '%n bytes', $count), + $this->l10n->n('%n byte', '%n bytes', $writtenByteCount), $this->l10n->n('%n byte', '%n bytes', $expected), ], ) @@ -294,13 +294,13 @@ public function put($data) { // compare expected and actual size if (isset($_SERVER['CONTENT_LENGTH']) && $_SERVER['REQUEST_METHOD'] === 'PUT') { $expected = (int)$_SERVER['CONTENT_LENGTH']; - if ($count !== $expected) { + if ($writtenByteCount !== $expected) { throw new BadRequest( $this->l10n->t( 'Expected filesize of %1$s but read (from Nextcloud client) and wrote (to Nextcloud storage) %2$s. Could either be a network problem on the sending side or a problem writing to the storage on the server side.', [ $this->l10n->n('%n byte', '%n bytes', $expected), - $this->l10n->n('%n byte', '%n bytes', $count), + $this->l10n->n('%n byte', '%n bytes', $writtenByteCount), ], ) ); @@ -366,7 +366,7 @@ public function put($data) { } // since we skipped the view we need to scan and emit the hooks ourselves - $storage->getUpdater()->update($internalPath, null, ($count-$previousFileSize)); + $storage->getUpdater()->update($internalPath, null, ($writtenByteCount - $previousFileSize)); try { $this->changeLock(ILockingProvider::LOCK_SHARED); From 834d0810b5c69697b56d2be3adb095b97ad3e117 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 2 Sep 2021 20:42:41 +0200 Subject: [PATCH 3/4] Ensure to only update size if available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/dav/lib/Connector/Sabre/File.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 6b9bae329e788..fb238c38ea1bd 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -366,7 +366,8 @@ public function put($data) { } // since we skipped the view we need to scan and emit the hooks ourselves - $storage->getUpdater()->update($internalPath, null, ($writtenByteCount - $previousFileSize)); + $sizeDifference = isset($previousFileSize, $writtenByteCount) ? ($writtenByteCount - $previousFileSize) : null; + $storage->getUpdater()->update($internalPath, null, $sizeDifference); try { $this->changeLock(ILockingProvider::LOCK_SHARED); From cc7c4679a4b4948825ac20f018629c3e51e07ea4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 2 Sep 2021 20:43:08 +0200 Subject: [PATCH 4/4] Pass size difference for mkdir/file_put_content operations to speed up correcting the folder size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/private/Files/View.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 16a9381768b3a..776a8034154b2 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -51,7 +51,6 @@ use OC\Files\Storage\Storage; use OC\User\LazyUser; use OC\Share\Share; -use OC\User\User; use OCA\Files_Sharing\SharedMount; use OCP\Constants; use OCP\Files\Cache\ICacheEntry; @@ -316,12 +315,12 @@ public function enableCacheUpdate() { $this->updaterEnabled = true; } - protected function writeUpdate(Storage $storage, $internalPath, $time = null) { + protected function writeUpdate(Storage $storage, $internalPath, $time = null, $size = null) { if ($this->updaterEnabled) { if (is_null($time)) { $time = time(); } - $storage->getUpdater()->update($internalPath, $time); + $storage->getUpdater()->update($internalPath, $time, $size); } } @@ -1175,6 +1174,7 @@ private function basicOperation($operation, $path, $hooks = [], $extraParam = nu $this->unlockFile($path, ILockingProvider::LOCK_SHARED); throw $e; } + $previousSize = $storage->filesize($internalPath); } try { if (!is_null($extraParam)) { @@ -1195,7 +1195,13 @@ private function basicOperation($operation, $path, $hooks = [], $extraParam = nu $this->removeUpdate($storage, $internalPath); } if ($result !== false && in_array('write', $hooks, true) && $operation !== 'fopen' && $operation !== 'touch') { - $this->writeUpdate($storage, $internalPath); + $size = null; + if ($operation === 'mkdir') { + $size = 0; + } elseif ($operation === 'file_put_contents' && isset($previousSize)) { + $size = $result - $previousSize; + } + $this->writeUpdate($storage, $internalPath, null, $size); } if ($result !== false && in_array('touch', $hooks)) { $this->writeUpdate($storage, $internalPath, $extraParam);