From a8f72a9870c803ddab0393ba0b339a486e344e70 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Fri, 1 Apr 2022 19:48:31 +0200 Subject: [PATCH] Cache locally the filecache information This on one side might improve the performance by doing less requests to the database but more importantly this decrease the likeliness of read after write errors in a cluster setup since we don't requests the data from the database that might be outdated. Currently this only cache get/put/insert/update and for copy/move/propagator this clear the cache. Signed-off-by: Carl Schwan --- lib/private/Files/Cache/Cache.php | 162 +++++++++++------- lib/private/Files/Cache/Propagator.php | 2 + .../Files/Cache/Wrapper/CacheWrapper.php | 2 + tests/lib/Files/ViewTest.php | 2 +- 4 files changed, 108 insertions(+), 60 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 949079dfa224b..1735bfa3ecf9a 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -59,6 +59,7 @@ use OCP\Files\Storage\IStorage; use OCP\IDBConnection; use Psr\Log\LoggerInterface; +use OC\Cache\CappedMemoryCache; /** * Metadata cache for a storage @@ -76,41 +77,21 @@ class Cache implements ICache { } /** - * @var array partial data for the cache + * Internal cache that allows to store temporarely data without having to + * fetch them from the database */ - protected $partial = []; + protected CappedMemoryCache $internalCache; + + /** @var array partial data for the cache */ + protected array $partial = []; + protected string $storageId; + private IStorage $storage; + protected Storage $storageCache; + protected IMimeTypeLoader $mimetypeLoader; + protected IDBConnection $connection; + protected IEventDispatcher $eventDispatcher; + protected QuerySearchHelper $querySearchHelper; - /** - * @var string - */ - protected $storageId; - - private $storage; - - /** - * @var Storage $storageCache - */ - protected $storageCache; - - /** @var IMimeTypeLoader */ - protected $mimetypeLoader; - - /** - * @var IDBConnection - */ - protected $connection; - - /** - * @var IEventDispatcher - */ - protected $eventDispatcher; - - /** @var QuerySearchHelper */ - protected $querySearchHelper; - - /** - * @param IStorage $storage - */ public function __construct(IStorage $storage) { $this->storageId = $storage->getId(); $this->storage = $storage; @@ -123,9 +104,10 @@ public function __construct(IStorage $storage) { $this->connection = \OC::$server->getDatabaseConnection(); $this->eventDispatcher = \OC::$server->get(IEventDispatcher::class); $this->querySearchHelper = \OC::$server->query(QuerySearchHelper::class); + $this->internalCache = new CappedMemoryCache(); } - protected function getQueryBuilder() { + protected function getQueryBuilder(): CacheQueryBuilder { return new CacheQueryBuilder( $this->connection, \OC::$server->getSystemConfig(), @@ -143,12 +125,18 @@ public function getNumericStorageId() { } /** - * get the stored metadata of a file or folder + * Get the stored metadata of a file or folder * - * @param string | int $file either the path of a file or folder or the file id for a file or folder + * @param string|int $file either the path of a file or folder or the file id for a file or folder * @return ICacheEntry|false the cache entry as array of false if the file is not found in the cache */ public function get($file) { + // Try fetching from memory cache first + if ($this->internalCache->hasKey($file)) { + return $this->internalCache->get($file); + } + + // Fetch cache entry from database $query = $this->getQueryBuilder(); $query->selectFileCache(); @@ -166,25 +154,22 @@ public function get($file) { $data = $result->fetch(); $result->closeCursor(); - //merge partial data + // If no cache entry is found, return the partially saved in-memory data if (!$data && is_string($file) && isset($this->partial[$file])) { return $this->partial[$file]; - } elseif (!$data) { - return $data; - } else { - return self::cacheEntryFromData($data, $this->mimetypeLoader); } + + if (!$data) { + return false; + } + return self::cacheEntryFromData($data, $this->mimetypeLoader); } /** * Create a CacheEntry from database row - * - * @param array $data - * @param IMimeTypeLoader $mimetypeLoader - * @return CacheEntry */ - public static function cacheEntryFromData($data, IMimeTypeLoader $mimetypeLoader) { - //fix types + public static function cacheEntryFromData(array $data, IMimeTypeLoader $mimetypeLoader): CacheEntry { + // fix types $data['fileid'] = (int)$data['fileid']; $data['parent'] = (int)$data['parent']; $data['size'] = 0 + $data['size']; @@ -221,7 +206,7 @@ public function getFolderContents($folder) { } /** - * get the metadata of all files stored in $folder + * Get the metadata of all files stored in $folder * * @param int $fileId the file id of the folder * @return ICacheEntry[] @@ -237,15 +222,18 @@ public function getFolderContentsById($fileId) { $files = $result->fetchAll(); $result->closeCursor(); - return array_map(function (array $data) { - return self::cacheEntryFromData($data, $this->mimetypeLoader); + return array_map(function (array $data): ICacheEntry { + $cacheEntry = self::cacheEntryFromData($data, $this->mimetypeLoader); + $this->internalCache->set($cacheEntry->getId(), $cacheEntry); + $this->internalCache->set($cacheEntry->getPath(), $cacheEntry); + return $cacheEntry; }, $files); } return []; } /** - * insert or update meta data for a file or folder + * Insert or update meta data for a file or folder * * @param string $file * @param array $data @@ -263,7 +251,7 @@ public function put($file, array $data) { } /** - * insert meta data for a new file or folder + * Insert meta data for a new file or folder * * @param string $file * @param array $data @@ -316,10 +304,34 @@ public function insert($file, array $data) { $query->setValue('fileid', $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT)); foreach ($extensionValues as $column => $value) { $query->setValue($column, $query->createNamedParameter($value)); + $values[$column] = $value; } $query->execute(); } + // Add possible missing values + $values['fileid'] = $fileId; + $valuesDefault = [ + 'encrypted' => 0, + 'storage_mtime' => 0, + 'permissions' => 0, + 'etag' => null, + 'checksum' => null, + 'metadata_etag' => null, + 'creation_time' => null, + 'upload_time' => null, + ]; + foreach ($valuesDefault as $key => $default) { + if (!array_key_exists($key, $values)) { + $values[$key] = $default; // not set => set it to 0 as default + } + } + // FIXME maybe we should fix the test instead + // $values['storageid'] = (string)$values['storageid']; + + // Save in in-memory cache + $this->internalCache->set($fileId, self::cacheEntryFromData($values, $this->mimetypeLoader)); + $event = new CacheEntryInsertedEvent($this->storage, $file, $fileId, $storageId); $this->eventDispatcher->dispatch(CacheInsertEvent::class, $event); $this->eventDispatcher->dispatchTyped($event); @@ -360,6 +372,7 @@ public function update($id, array $data) { } [$values, $extensionValues] = $this->normalizeData($data); + $valuesToUpdateInCache = []; if (count($values)) { $query = $this->getQueryBuilder(); @@ -375,6 +388,7 @@ public function update($id, array $data) { foreach ($values as $key => $value) { $query->set($key, $query->createNamedParameter($value)); + $valuesToUpdateInCache[$key] = $value; } $query->execute(); @@ -388,6 +402,7 @@ public function update($id, array $data) { $query->setValue('fileid', $query->createNamedParameter($id, IQueryBuilder::PARAM_INT)); foreach ($extensionValues as $column => $value) { $query->setValue($column, $query->createNamedParameter($value)); + $valuesToUpdateInCache[$column] = $value; } $query->execute(); @@ -413,6 +428,16 @@ public function update($id, array $data) { $path = $this->getPathById($id); // path can still be null if the file doesn't exist if ($path !== null) { + if ($this->internalCache->hasKey($id)) { + // Only update when we already have an entry in the cache + // otherwise we don't have all the values + $cacheEntry = $this->internalCache->get($id); + foreach ($valuesToUpdateInCache as $key => $value) { + $cacheEntry[$key] = $value; + } + $this->internalCache->set($id, $cacheEntry); + } + $event = new CacheEntryUpdatedEvent($this->storage, $path, $id, $this->getNumericStorageId()); $this->eventDispatcher->dispatch(CacheUpdateEvent::class, $event); $this->eventDispatcher->dispatchTyped($event); @@ -469,7 +494,7 @@ protected function normalizeData(array $data): array { } /** - * get the file id for a file + * Get the file id for a file * * A file id is a numeric id for a file or folder that's unique within an owncloud instance which stays the same for the lifetime of a file * @@ -482,6 +507,10 @@ public function getId($file) { // normalize file $file = $this->normalize($file); + if ($this->internalCache->hasKey($file)) { + return $this->internalCache->get($file)->getId(); + } + $query = $this->getQueryBuilder(); $query->select('fileid') ->from('filecache') @@ -505,6 +534,9 @@ public function getParentId($file) { if ($file === '') { return -1; } else { + if ($this->internalCache->hasKey($file)) { + return $this->internalCache->get($file)['parent']; + } $parent = $this->getParentPath($file); return (int)$this->getId($parent); } @@ -529,9 +561,9 @@ public function inCache($file) { } /** - * remove a file or folder from the cache + * Remove a file or folder from the cache * - * when removing a folder from the cache all files and folders inside the folder will be removed as well + * When removing a folder from the cache all files and folders inside the folder will be removed as well. * * @param string $file */ @@ -549,14 +581,22 @@ public function remove($file) { ->whereFileId($entry->getId()); $query->execute(); - if ($entry->getMimeType() == FileInfo::MIMETYPE_FOLDER) { + if ($entry->getMimeType() === FileInfo::MIMETYPE_FOLDER) { $this->removeChildren($entry); + $this->internalCache->clear(); + } else { + $this->internalCache->remove($entry->getId()); + $this->internalCache->remove($entry->getPath()); } $this->eventDispatcher->dispatchTyped(new CacheEntryRemovedEvent($this->storage, $entry->getPath(), $entry->getId(), $this->getNumericStorageId())); } } + public function clearInternalCache(): void { + $this->internalCache->clear(); + } + /** * Get all sub folders of a folder * @@ -565,8 +605,8 @@ public function remove($file) { */ private function getSubFolders(ICacheEntry $entry) { $children = $this->getFolderContentsById($entry->getId()); - return array_filter($children, function ($child) { - return $child->getMimeType() == FileInfo::MIMETYPE_FOLDER; + return array_filter($children, function ($child):bool { + return $child->getMimeType() === FileInfo::MIMETYPE_FOLDER; }); } @@ -576,7 +616,7 @@ private function getSubFolders(ICacheEntry $entry) { * @param ICacheEntry $entry the cache entry of the folder to remove the children of * @throws \OC\DatabaseException */ - private function removeChildren(ICacheEntry $entry) { + private function removeChildren(ICacheEntry $entry): void { $parentIds = [$entry->getId()]; $queue = [$entry->getId()]; @@ -690,6 +730,7 @@ public function moveFromCache(ICache $sourceCache, $sourcePath, $targetPath) { try { $query->execute(); + $this->internalCache->clear(); } catch (\OC\DatabaseException $e) { $this->connection->rollBack(); throw $e; @@ -705,6 +746,9 @@ public function moveFromCache(ICache $sourceCache, $sourcePath, $targetPath) { ->set('parent', $query->createNamedParameter($newParentId, IQueryBuilder::PARAM_INT)) ->whereFileId($sourceId); $query->execute(); + $this->internalCache->clear(); + $this->internalCache->remove($sourceId); + $this->internalCache->remove($sourceData->getPath()); $this->connection->commit(); diff --git a/lib/private/Files/Cache/Propagator.php b/lib/private/Files/Cache/Propagator.php index f8b2548a747de..e086d129d5a20 100644 --- a/lib/private/Files/Cache/Propagator.php +++ b/lib/private/Files/Cache/Propagator.php @@ -111,6 +111,7 @@ public function propagateChange($internalPath, $time, $sizeDifference = 0) { $builder->execute(); } + $this->storage->getCache()->clearInternalCache(); } protected function getParents($path) { @@ -195,5 +196,6 @@ public function commitBatch() { $this->batch = []; $this->connection->commit(); + $this->storage->getCache()->clearInternalCache(); } } diff --git a/lib/private/Files/Cache/Wrapper/CacheWrapper.php b/lib/private/Files/Cache/Wrapper/CacheWrapper.php index e5300dc75f514..94cbf100315d4 100644 --- a/lib/private/Files/Cache/Wrapper/CacheWrapper.php +++ b/lib/private/Files/Cache/Wrapper/CacheWrapper.php @@ -31,6 +31,7 @@ use OC\Files\Cache\Cache; use OC\Files\Cache\QuerySearchHelper; +use OC\Cache\CappedMemoryCache; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\Search\ISearchOperator; @@ -50,6 +51,7 @@ public function __construct($cache) { $this->mimetypeLoader = \OC::$server->getMimeTypeLoader(); $this->connection = \OC::$server->getDatabaseConnection(); $this->querySearchHelper = \OC::$server->get(QuerySearchHelper::class); + $this->internalCache = new CappedMemoryCache(); } protected function getCache() { diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index 7b735720ff13b..6ebd0b246245d 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -217,7 +217,7 @@ public function testCacheAPI() { $this->assertFalse($cachedData['encrypted']); $id = $rootView->putFileInfo('/foo.txt', ['encrypted' => true]); $cachedData = $rootView->getFileInfo('/foo.txt'); - $this->assertTrue($cachedData['encrypted']); + $this->assertTrue((bool)$cachedData['encrypted']); $this->assertEquals($cachedData['fileid'], $id); $this->assertFalse($rootView->getFileInfo('/non/existing'));