Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable30] Fix metadata storage with sharding #49165

Merged
merged 2 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/dav/lib/Connector/Sabre/FilesPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
namespace OCA\DAV\Connector\Sabre;

use OC\AppFramework\Http\Request;
use OC\FilesMetadata\Model\FilesMetadata;
use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
use OCP\Constants;
use OCP\Files\ForbiddenException;
Expand Down Expand Up @@ -581,7 +582,9 @@ private function handleUpdatePropertiesMetadata(PropPatch $propPatch, Node $node
$propPatch->handle(
$mutation,
function (mixed $value) use ($accessRight, $knownMetadata, $node, $mutation, $filesMetadataManager): bool {
/** @var FilesMetadata $metadata */
$metadata = $filesMetadataManager->getMetadata((int)$node->getFileId(), true);
$metadata->setStorageId($node->getNode()->getStorage()->getCache()->getNumericStorageId());
$metadataKey = substr($mutation, strlen(self::FILE_METADATA_PREFIX));

// confirm metadata key is editable via PROPPATCH
Expand Down
5 changes: 5 additions & 0 deletions apps/files/lib/Listener/SyncLivePhotosListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace OCA\Files\Listener;

use OC\FilesMetadata\Model\FilesMetadata;
use OCA\Files\Service\LivePhotosService;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
Expand Down Expand Up @@ -154,10 +155,14 @@ private function handleCopy(NodeCopiedEvent $event, Node $peerFile): void {
* We have everything to update metadata and keep the link between the 2 copies.
*/
$newPeerFile = $peerFile->copy($targetParent->getPath() . '/' . $peerTargetName);
/** @var FilesMetadata $targetMetadata */
$targetMetadata = $this->filesMetadataManager->getMetadata($targetFile->getId(), true);
$targetMetadata->setStorageId($targetFile->getStorage()->getCache()->getNumericStorageId());
$targetMetadata->setString('files-live-photo', (string)$newPeerFile->getId());
$this->filesMetadataManager->saveMetadata($targetMetadata);
/** @var FilesMetadata $peerMetadata */
$peerMetadata = $this->filesMetadataManager->getMetadata($newPeerFile->getId(), true);
$peerMetadata->setStorageId($newPeerFile->getStorage()->getCache()->getNumericStorageId());
$peerMetadata->setString('files-live-photo', (string)$targetFile->getId());
$this->filesMetadataManager->saveMetadata($peerMetadata);
}
Expand Down
3 changes: 3 additions & 0 deletions lib/private/FilesMetadata/FilesMetadataManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,14 @@ public function refreshMetadata(
int $process = self::PROCESS_LIVE,
string $namedEvent = ''
): IFilesMetadata {
$storageId = $node->getStorage()->getCache()->getNumericStorageId();
try {
/** @var FilesMetadata $metadata */
$metadata = $this->metadataRequestService->getMetadataFromFileId($node->getId());
} catch (FilesMetadataNotFoundException) {
$metadata = new FilesMetadata($node->getId());
}
$metadata->setStorageId($storageId);

// if $process is LIVE, we enforce LIVE
// if $process is NAMED, we go NAMED
Expand Down
17 changes: 17 additions & 0 deletions lib/private/FilesMetadata/Model/FilesMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class FilesMetadata implements IFilesMetadata {
private bool $updated = false;
private int $lastUpdate = 0;
private string $syncToken = '';
private ?int $storageId = null;

public function __construct(
private int $fileId = 0
Expand All @@ -42,6 +43,22 @@ public function getFileId(): int {
return $this->fileId;
}

public function getStorageId(): ?int {
return $this->storageId;
}

/**
* Set which storage the file this metadata belongs to.
*
* This helps with sharded filecache setups to know where to store the metadata
*
* @param int $storageId
* @return void
*/
public function setStorageId(int $storageId): void {
$this->storageId = $storageId;
}

/**
* @inheritDoc
* @return int timestamp
Expand Down
23 changes: 23 additions & 0 deletions lib/private/FilesMetadata/Service/MetadataRequestService.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,27 @@ public function __construct(
) {
}

private function getStorageId(IFilesMetadata $filesMetadata): int {
if ($filesMetadata instanceof FilesMetadata) {
$storage = $filesMetadata->getStorageId();
if ($storage) {
return $storage;
}
}
// all code paths that lead to saving metadata *should* have the storage id set
// this fallback is there just in case
$query = $this->dbConnection->getQueryBuilder();
$query->select('storage')
->from('filecache')
->where($query->expr()->eq('fileid', $query->createNamedParameter($filesMetadata->getFileId(), IQueryBuilder::PARAM_INT)));
$storageId = $query->executeQuery()->fetchColumn();

if ($filesMetadata instanceof FilesMetadata) {
$filesMetadata->setStorageId($storageId);
}
return $storageId;
}

/**
* store metadata into database
*
Expand All @@ -38,6 +59,7 @@ public function __construct(
public function store(IFilesMetadata $filesMetadata): void {
$qb = $this->dbConnection->getQueryBuilder();
$qb->insert(self::TABLE_METADATA)
->hintShardKey('storage', $this->getStorageId($filesMetadata))
->setValue('file_id', $qb->createNamedParameter($filesMetadata->getFileId(), IQueryBuilder::PARAM_INT))
->setValue('json', $qb->createNamedParameter(json_encode($filesMetadata->jsonSerialize())))
->setValue('sync_token', $qb->createNamedParameter($this->generateSyncToken()))
Expand Down Expand Up @@ -134,6 +156,7 @@ public function updateMetadata(IFilesMetadata $filesMetadata): int {
$expr = $qb->expr();

$qb->update(self::TABLE_METADATA)
->hintShardKey('files_metadata', $this->getStorageId($filesMetadata))
->set('json', $qb->createNamedParameter(json_encode($filesMetadata->jsonSerialize())))
->set('sync_token', $qb->createNamedParameter($this->generateSyncToken()))
->set('last_update', $qb->createFunction('NOW()'))
Expand Down
98 changes: 98 additions & 0 deletions tests/lib/FilesMetadata/FilesMetadataManagerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2024 Robin Appelman <robin@icewind.nl>
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace Test\FilesMetadata;

use OC\BackgroundJob\JobList;
use OC\Files\Storage\Temporary;
use OC\FilesMetadata\FilesMetadataManager;
use OC\FilesMetadata\Service\IndexRequestService;
use OC\FilesMetadata\Service\MetadataRequestService;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Folder;
use OCP\Files\IRootFolder;
use OCP\FilesMetadata\AMetadataEvent;
use OCP\IAppConfig;
use OCP\IDBConnection;
use OCP\Server;
use Psr\Log\LoggerInterface;
use Test\TestCase;
use Test\Traits\MountProviderTrait;
use Test\Traits\UserTrait;

/**
* @group DB
*/
class FilesMetadataManagerTest extends TestCase {
use UserTrait;
use MountProviderTrait;

private IEventDispatcher $eventDispatcher;
private JobList $jobList;
private IAppConfig $appConfig;
private LoggerInterface $logger;
private MetadataRequestService $metadataRequestService;
private IndexRequestService $indexRequestService;
private FilesMetadataManager $manager;
private IDBConnection $connection;
private Folder $userFolder;
private array $metadata = [];

protected function setUp(): void {
parent::setUp();

$this->jobList = $this->createMock(JobList::class);
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->eventDispatcher->method('dispatchTyped')->willReturnCallback(function (Event $event) {
if ($event instanceof AMetadataEvent) {
$name = $event->getNode()->getName();
if (isset($this->metadata[$name])) {
$meta = $event->getMetadata();
foreach ($this->metadata[$name] as $key => $value) {
$meta->setString($key, $value);
}
}
}
});
$this->appConfig = $this->createMock(IAppConfig::class);
$this->logger = $this->createMock(LoggerInterface::class);

$this->connection = Server::get(IDBConnection::class);
$this->metadataRequestService = new MetadataRequestService($this->connection, $this->logger);
$this->indexRequestService = new IndexRequestService($this->connection, $this->logger);
$this->manager = new FilesMetadataManager(
$this->eventDispatcher,
$this->jobList,
$this->appConfig,
$this->logger,
$this->metadataRequestService,
$this->indexRequestService,
);

$this->createUser('metatest', '');
$this->registerMount('metatest', new Temporary([]), '/metatest');

$rootFolder = Server::get(IRootFolder::class);
$this->userFolder = $rootFolder->getUserFolder('metatest');
}

public function testRefreshMetadata(): void {
$this->metadata['test.txt'] = [
'istest' => 'yes'
];
$file = $this->userFolder->newFile('test.txt', 'test');
$stored = $this->manager->refreshMetadata($file);
$this->assertEquals($file->getId(), $stored->getFileId());
$this->assertEquals('yes', $stored->getString('istest'));

$retrieved = $this->manager->getMetadata($file->getId());
$this->assertEquals($file->getId(), $retrieved->getFileId());
$this->assertEquals('yes', $retrieved->getString('istest'));
}
}
Loading