Skip to content

Commit

Permalink
refactor: don't join on filecache in usermountcache
Browse files Browse the repository at this point in the history 
Signed-off-by: Robin Appelman <[email protected]>
  • Loading branch information
@icewind1991
icewind1991 committed May 2, 2024
1 parent 19602a0 commit a235164
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 94 deletions.
109 changes: 44 additions & 65 deletions lib/private/Files/Config/UserMountCache.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

namespace OC\Files\Config;

use OC\Files\Cache\FileAccess;
use OC\User\LazyUser;
use OCP\Cache\CappedMemoryCache;
use OCP\DB\QueryBuilder\IQueryBuilder;
Expand All @@ -45,37 +47,31 @@
* Cache mounts points per user in the cache so we can easily look them up
*/
class UserMountCache implements IUserMountCache {
private IDBConnection $connection;
private IUserManager $userManager;

/**
* Cached mount info.
*
* @var CappedMemoryCache<ICachedMountInfo[]>
**/
private CappedMemoryCache $mountsForUsers;
/**
* fileid => internal path mapping for cached mount info.
*
* @var CappedMemoryCache<string>
**/
private CappedMemoryCache $internalPathCache;
private LoggerInterface $logger;
/** @var CappedMemoryCache<array> */
private CappedMemoryCache $cacheInfoCache;
private IEventLogger $eventLogger;

/**
* UserMountCache constructor.
*/
public function __construct(
IDBConnection $connection,
IUserManager $userManager,
LoggerInterface $logger,
IEventLogger $eventLogger
private IDBConnection $connection,
private IUserManager $userManager,
private LoggerInterface $logger,
private IEventLogger $eventLogger,
private FileAccess $cacheAccess,
) {
$this->connection = $connection;
$this->userManager = $userManager;
$this->logger = $logger;
$this->eventLogger = $eventLogger;
$this->cacheInfoCache = new CappedMemoryCache();
$this->internalPathCache = new CappedMemoryCache();
$this->mountsForUsers = new CappedMemoryCache();
Expand Down Expand Up @@ -282,23 +278,19 @@ public function getInternalPathForMountInfo(CachedMountInfo $info): string {
if ($cached !== null) {
return $cached;
}
$builder = $this->connection->getQueryBuilder();
$query = $builder->select('path')
->from('filecache')
->where($builder->expr()->eq('fileid', $builder->createPositionalParameter($info->getRootId())));
return $query->executeQuery()->fetchOne() ?: '';
$entry = $this->cacheAccess->getByFileIdInStorage($info->getRootId(), $info->getStorageId());
return $entry ? $entry->getPath() : '';
}

/**
* @param int $numericStorageId
* @param string|null $user limit the results to a single user
* @return CachedMountInfo[]
*/
public function getMountsForStorageId($numericStorageId, $user = null) {
public function getMountsForStorageId($numericStorageId, $user = null, bool $preloadPaths = false) {
$builder = $this->connection->getQueryBuilder();
$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'f.path', 'mount_provider_class')
$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'mount_provider_class')
->from('mounts', 'm')
->innerJoin('m', 'filecache', 'f', $builder->expr()->eq('m.root_id', 'f.fileid'))
->where($builder->expr()->eq('storage_id', $builder->createPositionalParameter($numericStorageId, IQueryBuilder::PARAM_INT)));

if ($user) {
Expand All @@ -309,7 +301,21 @@ public function getMountsForStorageId($numericStorageId, $user = null) {
$rows = $result->fetchAll();
$result->closeCursor();

return array_filter(array_map([$this, 'dbRowToMountInfo'], $rows));
if ($preloadPaths) {
$fileIds = array_map(fn (array $row) => $row['root_id'], $rows);
$files = $this->cacheAccess->getByFileIds($fileIds);

foreach ($rows as &$row) {
$mountFileId = $row['root_id'];
if (isset($files[$mountFileId])) {
$row['path'] = $files[$mountFileId]->getPath();
}
}
}

return array_filter(array_map(function (array $row) use ($preloadPaths) {
return $this->dbRowToMountInfo($row, $preloadPaths ? null : [$this, 'getInternalPathForMountInfo']);
}, $rows));
}

/**
Expand All @@ -318,45 +324,17 @@ public function getMountsForStorageId($numericStorageId, $user = null) {
*/
public function getMountsForRootId($rootFileId) {
$builder = $this->connection->getQueryBuilder();
$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'f.path', 'mount_provider_class')
$query = $builder->select('storage_id', 'root_id', 'user_id', 'mount_point', 'mount_id', 'mount_provider_class')
->from('mounts', 'm')
->innerJoin('m', 'filecache', 'f', $builder->expr()->eq('m.root_id', 'f.fileid'))
->where($builder->expr()->eq('root_id', $builder->createPositionalParameter($rootFileId, IQueryBuilder::PARAM_INT)));

$result = $query->execute();
$rows = $result->fetchAll();
$result->closeCursor();

return array_filter(array_map([$this, 'dbRowToMountInfo'], $rows));
}

/**
* @param $fileId
* @return array{int, string, int}
* @throws \OCP\Files\NotFoundException
*/
private function getCacheInfoFromFileId($fileId): array {
if (!isset($this->cacheInfoCache[$fileId])) {
$builder = $this->connection->getQueryBuilder();
$query = $builder->select('storage', 'path', 'mimetype')
->from('filecache')
->where($builder->expr()->eq('fileid', $builder->createNamedParameter($fileId, IQueryBuilder::PARAM_INT)));

$result = $query->execute();
$row = $result->fetch();
$result->closeCursor();

if (is_array($row)) {
$this->cacheInfoCache[$fileId] = [
(int)$row['storage'],
(string)$row['path'],
(int)$row['mimetype']
];
} else {
throw new NotFoundException('File with id "' . $fileId . '" not found');
}
}
return $this->cacheInfoCache[$fileId];
return array_filter(array_map(function (array $row) {
return $this->dbRowToMountInfo($row, [$this, 'getInternalPathForMountInfo']);
}, $rows));
}

/**
Expand All @@ -366,12 +344,13 @@ private function getCacheInfoFromFileId($fileId): array {
* @since 9.0.0
*/
public function getMountsForFileId($fileId, $user = null) {
try {
[$storageId, $internalPath] = $this->getCacheInfoFromFileId($fileId);
} catch (NotFoundException $e) {
$cacheEntry = $this->cacheAccess->getByFileId($fileId);
if (!$cacheEntry) {
return [];
}
$mountsForStorage = $this->getMountsForStorageId($storageId, $user);
$internalPath = $cacheEntry->getPath();

$mountsForStorage = $this->getMountsForStorageId($cacheEntry->getStorageId(), $user, true);

// filter mounts that are from the same storage but not a parent of the file we care about
$filteredMounts = array_filter($mountsForStorage, function (ICachedMountInfo $mount) use ($internalPath, $fileId) {
Expand Down Expand Up @@ -449,26 +428,26 @@ public function getUsedSpaceForUsers(array $users) {
return $user->getUID();
}, $users);

$query = $builder->select('m.user_id', 'f.size')
$query = $builder->select('m.user_id', 'storage_id')
->from('mounts', 'm')
->innerJoin('m', 'filecache', 'f',
$builder->expr()->andX(
$builder->expr()->eq('m.storage_id', 'f.storage'),
$builder->expr()->eq('f.path_hash', $builder->createNamedParameter(md5('files')))
))
->where($builder->expr()->eq('m.mount_point', $mountPoint))
->andWhere($builder->expr()->in('m.user_id', $builder->createNamedParameter($userIds, IQueryBuilder::PARAM_STR_ARRAY)));

$result = $query->execute();

$results = [];
while ($row = $result->fetch()) {
$results[$row['user_id']] = $row['size'];
$results[$row['user_id']] = $this->getSizeForHomeStorage($row['storage_id']);
}
$result->closeCursor();
return $results;
}

private function getSizeForHomeStorage(int $storage): int {
$entry = $this->cacheAccess->getByPathInStorage('files', $storage);
return $entry->getSize();
}

public function clear(): void {
$this->cacheInfoCache = new CappedMemoryCache();
$this->mountsForUsers = new CappedMemoryCache();
Expand Down
72 changes: 43 additions & 29 deletions tests/lib/Files/Config/UserMountCacheTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,22 @@

namespace Test\Files\Config;

use OC\DB\Exceptions\DbalException;
use OC\DB\QueryBuilder\Literal;
use OC\Files\Cache\FileAccess;
use OC\Files\Mount\MountPoint;
use OC\Files\Storage\Storage;
use OC\User\Manager;
use OCP\Cache\CappedMemoryCache;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Diagnostics\IEventLogger;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Config\ICachedMountInfo;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IUserManager;
use OCP\Server;
use Psr\Log\LoggerInterface;
use Test\TestCase;
use Test\Util\User\Dummy;
Expand All @@ -28,10 +32,8 @@
* @group DB
*/
class UserMountCacheTest extends TestCase {
/**
* @var IDBConnection
*/
private $connection;
private IDBConnection $connection;
private FileAccess $cacheAccess;

/**
* @var IUserManager
Expand All @@ -49,7 +51,8 @@ protected function setUp(): void {
parent::setUp();

$this->fileIds = [];
$this->connection = \OC::$server->getDatabaseConnection();
$this->connection = Server::get(IDBConnection::class);
$this->cacheAccess = Server::get(FileAccess::class);
$config = $this->getMockBuilder(IConfig::class)
->disableOriginalConstructor()
->getMock();
Expand All @@ -67,7 +70,13 @@ protected function setUp(): void {
$userBackend->createUser('u2', '');
$userBackend->createUser('u3', '');
$this->userManager->registerBackend($userBackend);
$this->cache = new \OC\Files\Config\UserMountCache($this->connection, $this->userManager, $this->createMock(LoggerInterface::class), $this->createMock(IEventLogger::class));
$this->cache = new \OC\Files\Config\UserMountCache(
$this->connection,
$this->userManager,
$this->createMock(LoggerInterface::class),
$this->createMock(IEventLogger::class),
$this->cacheAccess,
);
}

protected function tearDown(): void {
Expand Down Expand Up @@ -336,31 +345,36 @@ private function sortMounts(&$mounts) {

private function createCacheEntry($internalPath, $storageId, $size = 0) {
$internalPath = trim($internalPath, '/');
$inserted = $this->connection->insertIfNotExist('*PREFIX*filecache', [
'storage' => $storageId,
'path' => $internalPath,
'path_hash' => md5($internalPath),
'parent' => -1,
'name' => basename($internalPath),
'mimetype' => 0,
'mimepart' => 0,
'size' => $size,
'storage_mtime' => 0,
'encrypted' => 0,
'unencrypted_size' => 0,
'etag' => '',
'permissions' => 31
], ['storage', 'path_hash']);
if ($inserted) {
$id = (int)$this->connection->lastInsertId('*PREFIX*filecache');
$query = $this->connection->getQueryBuilder();
$query->insert('filecache')
->values([
'storage' => $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT),
'path' => $query->createNamedParameter($internalPath),
'path_hash' => $query->createNamedParameter(md5($internalPath)),
'parent' => $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT),
'name' => $query->createNamedParameter(basename($internalPath)),
'mimetype' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
'mimepart' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
'size' => $query->createNamedParameter($size, IQueryBuilder::PARAM_INT),
'storage_mtime' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
'encrypted' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
'unencrypted_size' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
'etag' => $query->createNamedParameter(''),
'permissions' => $query->createNamedParameter(31, IQueryBuilder::PARAM_INT),
]);
try {
$query->executeStatement();
$id = (int)$query->getLastInsertId();
$this->fileIds[] = $id;
} else {
$sql = 'SELECT `fileid` FROM `*PREFIX*filecache` WHERE `storage` = ? AND `path_hash` =?';
$query = $this->connection->prepare($sql);
$query->execute([$storageId, md5($internalPath)]);
return (int)$query->fetchOne();
return $id;
} catch (DbalException $e) {
$query = $this->connection->getQueryBuilder();
$query->select('fileid')
->from('filecache')
->where($query->expr()->eq('storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
->andWhere($query->expr()->eq('path_hash', $query->createNamedParameter(md5($internalPath))));
return (int)$query->executeQuery()->fetchOne();
}
return $id;
}

public function testGetMountsForFileIdRootId() {
Expand Down

0 comments on commit a235164

Please sign in to comment.