From 2e68d8909a4f3541b4ad865272b56bf97ef86ff6 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Fri, 18 Sep 2026 11:27:43 +0200 Subject: [PATCH 1/4] perf: Partition preview migration job Use 16 partitions Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Carl Schwan --- core/BackgroundJobs/PreviewMigrationJob.php | 75 +++++++++++++------ lib/private/Repair/AddMovePreviewJob.php | 11 ++- lib/private/Setup.php | 2 - tests/lib/Preview/PreviewMigrationJobTest.php | 75 ++++++++----------- 4 files changed, 94 insertions(+), 69 deletions(-) diff --git a/core/BackgroundJobs/PreviewMigrationJob.php b/core/BackgroundJobs/PreviewMigrationJob.php index 45f71e561244f..88596d4940260 100644 --- a/core/BackgroundJobs/PreviewMigrationJob.php +++ b/core/BackgroundJobs/PreviewMigrationJob.php @@ -11,6 +11,7 @@ use OC\Preview\PreviewMigrationService; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; use OCP\BackgroundJob\TimedJob; use OCP\Files\FileInfo; use OCP\Files\IRootFolder; @@ -20,64 +21,79 @@ use Psr\Log\LoggerInterface; class PreviewMigrationJob extends TimedJob { + public const int PARTITIONS = 16; private string $previewRootPath; public function __construct( ITimeFactory $time, private readonly IAppConfig $appConfig, - private readonly IConfig $config, + IConfig $config, private readonly IRootFolder $rootFolder, private readonly PreviewMigrationService $migrationService, + private readonly IJobList $jobList, private readonly LoggerInterface $logger, ) { parent::__construct($time); - $this->setTimeSensitivity(self::TIME_INSENSITIVE); - $this->setInterval(24 * 60 * 60); - $this->previewRootPath = 'appdata_' . $this->config->getSystemValueString('instanceid') . '/preview/'; + $this->setInterval(62 * 60); // every 62 min as the job takes 60 min + $this->previewRootPath = 'appdata_' . $config->getSystemValueString('instanceid') . '/preview/'; } #[Override] protected function run(mixed $argument): void { if ($this->appConfig->getValueBool('core', 'previewMovedDone')) { + $this->jobList->removeById($this->getId()); + return; + } + + $partition = (int)($argument['partition'] ?? 0); + if ($partition < 0 || $partition >= self::PARTITIONS) { + $this->jobList->removeById($this->getId()); return; } + if (!$this->runPartition($partition)) { + return; + } + + $this->appConfig->setValueBool('core', 'previewMigrationPartition' . $partition, true); + $this->jobList->removeById($this->getId()); + for ($i = 0; $i < self::PARTITIONS; $i++) { + if (!$this->appConfig->getValueBool('core', 'previewMigrationPartition' . $i, false)) { + return; + } + } + + $this->appConfig->setValueBool('core', 'previewMovedDone', true); + } + + private function runPartition(int $partition): bool { $storage = $this->rootFolder->getMountPoint()->getStorage(); if ($storage === null) { $this->logger->warning('Preview migration skipped: the root mount point has no storage.'); - $this->appConfig->setValueBool('core', 'previewMovedDone', true); - return; + return true; } $cache = $storage->getCache(); $previewRootId = $cache->getId(rtrim($this->previewRootPath, '/')); if ($previewRootId === -1) { - // No previews were ever generated, or the storage config no longer - // matches the one the filecache data was recorded under. $this->logger->warning('Preview migration skipped: no preview root found at "{path}" on storage "{storageId}".', [ 'path' => $this->previewRootPath, 'storageId' => $storage->getId(), ]); - $this->appConfig->setValueBool('core', 'previewMovedDone', true); - return; + return true; } $startTime = time(); - - // Walk the preview folder tree via the `parent` column, which is indexed on - // every supported database platform. - // - // Depth from the preview root tells us which structure a leaf folder holds: - // - depth 1: legacy flat structure, e.g. preview//.png - // - depth 8: hierarchical structure, e.g. preview/a/b/c/d/e/f/g//.png $foldersToVisit = [[$previewRootId, '', 0]]; while ($foldersToVisit !== []) { [$folderId, $folderName, $depth] = array_pop($foldersToVisit); - // Collect the actual preview files here so migrateFileId() doesn't need to - // list this folder's contents a second time. + if ($depth === 1 && !$this->belongsToPartition($folderName, $partition)) { + continue; + } + $previewEntries = []; foreach ($cache->getFolderContentsById($folderId) as $entry) { if ($entry->getMimeType() === FileInfo::MIMETYPE_FOLDER) { @@ -99,12 +115,27 @@ protected function run(mixed $argument): void { ]); } - // Stop if execution time is more than one hour. if (time() - $startTime > 3600) { - return; + return false; } } - $this->appConfig->setValueBool('core', 'previewMovedDone', true); + return true; + } + + private function belongsToPartition(string $folderName, int $partition): bool { + if ($partition < 0 || $partition >= self::PARTITIONS) { + return false; + } + + if (ctype_digit($folderName)) { + return ((int)$folderName % self::PARTITIONS) === $partition; + } + + if (strlen($folderName) === 1 && ctype_xdigit($folderName)) { + return (hexdec($folderName) % self::PARTITIONS) === $partition; + } + + return $partition === 0; } } diff --git a/lib/private/Repair/AddMovePreviewJob.php b/lib/private/Repair/AddMovePreviewJob.php index bf89464583b87..3251f0978f1f7 100644 --- a/lib/private/Repair/AddMovePreviewJob.php +++ b/lib/private/Repair/AddMovePreviewJob.php @@ -11,6 +11,7 @@ use OC\Core\BackgroundJobs\PreviewMigrationJob; use OCP\BackgroundJob\IJobList; +use OCP\IAppConfig; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; use Override; @@ -18,6 +19,7 @@ class AddMovePreviewJob implements IRepairStep { public function __construct( private readonly IJobList $jobList, + private readonly IAppConfig $appConfig, ) { } @@ -28,6 +30,13 @@ public function getName(): string { #[Override] public function run(IOutput $output): void { - $this->jobList->add(PreviewMigrationJob::class); + // Remove the unpartitioned job registered by older server versions. + $this->jobList->remove(PreviewMigrationJob::class); + for ($partition = 0; $partition < PreviewMigrationJob::PARTITIONS; $partition++) { + $this->appConfig->setValueBool('core', 'previewMigrationPartition' . $partition, false); + $this->jobList->add(PreviewMigrationJob::class, [ + 'partition' => $partition, + ]); + } } } diff --git a/lib/private/Setup.php b/lib/private/Setup.php index 8033bd520998b..411f7231afa5a 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -19,7 +19,6 @@ use OC\Core\BackgroundJobs\CleanupBackgroundJobsJob; use OC\Core\BackgroundJobs\ExpirePreviewsJob; use OC\Core\BackgroundJobs\GenerateMetadataJob; -use OC\Core\BackgroundJobs\PreviewMigrationJob; use OC\Log\Rotate; use OC\Preview\BackgroundCleanupJob; use OC\Setup\AbstractDatabase; @@ -533,7 +532,6 @@ public static function installBackgroundJobs(): void { $jobList->add(CleanupDeletedUsers::class); $jobList->add(CleanupLoginTokens::class); $jobList->add(GenerateMetadataJob::class); - $jobList->add(PreviewMigrationJob::class); $jobList->add(ExpirePreviewsJob::class); $jobList->add(CleanupBackgroundJobsJob::class); } diff --git a/tests/lib/Preview/PreviewMigrationJobTest.php b/tests/lib/Preview/PreviewMigrationJobTest.php index dcd57464f0d1e..bc01eff98d1ce 100644 --- a/tests/lib/Preview/PreviewMigrationJobTest.php +++ b/tests/lib/Preview/PreviewMigrationJobTest.php @@ -16,6 +16,7 @@ use OC\Preview\PreviewService; use OC\Preview\Storage\StorageFactory; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\IAppData; use OCP\Files\IMimeTypeDetector; @@ -116,8 +117,7 @@ public function testMigrationLegacyPath(): void { $this->assertEquals(2, count($folder->getDirectoryListing())); $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); - $job = $this->createJob(); - $this->invokePrivate($job, 'run', [[]]); + $this->runAllPartitions(); $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); $this->assertEquals(2, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); } @@ -143,10 +143,18 @@ private function createJob(): PreviewMigrationJob { $this->storageFactory, Server::get(IAppDataFactory::class), ), + Server::get(IJobList::class), $this->logger, ); } + private function runAllPartitions(): void { + $job = $this->createJob(); + for ($partition = 0; $partition < PreviewMigrationJob::PARTITIONS; $partition++) { + $this->invokePrivate($job, 'run', [['partition' => $partition]]); + } + } + private function insertFilecacheRow(string $path, string $etag): int { $qb = $this->db->getQueryBuilder(); $qb->insert('filecache') @@ -187,7 +195,7 @@ public function testMigrationMultipleFileIds(): void { $hierFolder = $this->previewAppData->newFolder(self::getInternalFolder((string)$otherFileId)); $hierFolder->newFile('128-128.png', 'abcdefg'); - $this->invokePrivate($this->createJob(), 'run', [[]]); + $this->runAllPartitions(); $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); $this->assertEquals(1, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); @@ -218,7 +226,7 @@ public function testMigrationSkipsDuplicatePreview(): void { $existing->generateId(); $this->previewMapper->insert($existing); - $this->invokePrivate($this->createJob(), 'run', [[]]); + $this->runAllPartitions(); // No duplicate preview row was inserted, but the legacy folder and its stale // filecache row were still cleaned up. @@ -232,17 +240,32 @@ public function testMigrationDeletesOrphanedPreview(): void { $folder = $this->previewAppData->newFolder((string)$orphanFileId); $folder->newFile('64-64-crop.jpg', 'abcdefg'); - $this->invokePrivate($this->createJob(), 'run', [[]]); + $this->runAllPartitions(); $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile($orphanFileId)))); } + #[TestDox('A partition must only migrate the legacy preview folders belonging to it')] + public function testMigrationOnlyMigratesOwnPartition(): void { + $folder = $this->previewAppData->newFolder('5'); + $folder->newFile('64-64-crop.jpg', 'abcdefg'); + + $job = $this->createJob(); + $this->invokePrivate($job, 'run', [['partition' => 4]]); + $this->assertEquals(1, count($this->previewAppData->getDirectoryListing())); + $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); + + $this->invokePrivate($job, 'run', [['partition' => 5]]); + $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); + $this->assertEquals(1, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); + } + #[TestDox('run() must complete without error when there is nothing to migrate')] public function testMigrationWithoutAnyPreviews(): void { $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); - $this->invokePrivate($this->createJob(), 'run', [[]]); + $this->runAllPartitions(); $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); @@ -258,25 +281,7 @@ public function testMigrationPath(): void { $this->assertEquals(2, count($folder->getDirectoryListing())); $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); - $job = new PreviewMigrationJob( - Server::get(ITimeFactory::class), - $this->appConfig, - $this->config, - Server::get(IRootFolder::class), - new PreviewMigrationService( - $this->config, - Server::get(IRootFolder::class), - $this->logger, - $this->mimeTypeDetector, - $this->mimeTypeLoader, - Server::get(IDBConnection::class), - $this->previewMapper, - $this->storageFactory, - Server::get(IAppDataFactory::class), - ), - $this->logger, - ); - $this->invokePrivate($job, 'run', [[]]); + $this->runAllPartitions(); $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); $this->assertEquals(2, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); } @@ -303,25 +308,7 @@ public function testMigrationPathWithVersion(): void { $this->assertEquals(9, count($folder->getDirectoryListing())); $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); - $job = new PreviewMigrationJob( - Server::get(ITimeFactory::class), - $this->appConfig, - $this->config, - Server::get(IRootFolder::class), - new PreviewMigrationService( - $this->config, - Server::get(IRootFolder::class), - $this->logger, - $this->mimeTypeDetector, - $this->mimeTypeLoader, - Server::get(IDBConnection::class), - $this->previewMapper, - $this->storageFactory, - Server::get(IAppDataFactory::class), - ), - $this->logger, - ); - $this->invokePrivate($job, 'run', [[]]); + $this->runAllPartitions(); $previews = iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)); $this->assertEquals(9, count($previews)); $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); From 9c5187cf1a8ca5b641c743c406edef6ee302bcaa Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 30 Sep 2026 16:16:10 +0200 Subject: [PATCH 2/4] perf(preview-migration): Batch insert previews Also optimize fetching the previews to migrate by directly querying the filecache with an optimized SQL query. Migrating 100 000 previews now takes 22s instead of 646s on PostgreSQL and 50s instead of 449s on SQLite. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Carl Schwan --- core/BackgroundJobs/PreviewMigrationJob.php | 95 +++++++++++--- lib/private/Preview/Db/PreviewMapper.php | 70 ++++++++++ .../Preview/PreviewMigrationService.php | 121 +++++++++++++++--- tests/lib/Preview/PreviewMigrationJobTest.php | 24 ++++ 4 files changed, 269 insertions(+), 41 deletions(-) diff --git a/core/BackgroundJobs/PreviewMigrationJob.php b/core/BackgroundJobs/PreviewMigrationJob.php index 88596d4940260..35d62382b9add 100644 --- a/core/BackgroundJobs/PreviewMigrationJob.php +++ b/core/BackgroundJobs/PreviewMigrationJob.php @@ -9,19 +9,24 @@ namespace OC\Core\BackgroundJobs; +use OC\Files\Cache\CacheEntry; use OC\Preview\PreviewMigrationService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; use OCP\BackgroundJob\TimedJob; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\FileInfo; +use OCP\Files\IMimeTypeLoader; use OCP\Files\IRootFolder; use OCP\IAppConfig; use OCP\IConfig; +use OCP\IDBConnection; use Override; use Psr\Log\LoggerInterface; class PreviewMigrationJob extends TimedJob { public const int PARTITIONS = 16; + private const int BATCH_SIZE = 500; private string $previewRootPath; public function __construct( @@ -31,6 +36,8 @@ public function __construct( private readonly IRootFolder $rootFolder, private readonly PreviewMigrationService $migrationService, private readonly IJobList $jobList, + private readonly IDBConnection $connection, + private readonly IMimeTypeLoader $mimeTypeLoader, private readonly LoggerInterface $logger, ) { parent::__construct($time); @@ -85,44 +92,92 @@ private function runPartition(int $partition): bool { } $startTime = time(); + $storageId = $cache->getNumericStorageId(); + $folderMimeTypeId = $this->mimeTypeLoader->getId(FileInfo::MIMETYPE_FOLDER); $foldersToVisit = [[$previewRootId, '', 0]]; + $foldersToMigrate = []; + // Folders without preview files, by depth; removed once their children are gone. + $emptyFolders = []; while ($foldersToVisit !== []) { - [$folderId, $folderName, $depth] = array_pop($foldersToVisit); - - if ($depth === 1 && !$this->belongsToPartition($folderName, $partition)) { + $folders = []; + foreach (array_splice($foldersToVisit, -self::BATCH_SIZE) as [$folderId, $folderName, $depth]) { + if ($depth === 1 && !$this->belongsToPartition($folderName, $partition)) { + continue; + } + $folders[$folderId] = [$folderName, $depth]; + } + if ($folders === []) { continue; } - $previewEntries = []; - foreach ($cache->getFolderContentsById($folderId) as $entry) { - if ($entry->getMimeType() === FileInfo::MIMETYPE_FOLDER) { - $foldersToVisit[] = [$entry->getId(), $entry->getName(), $depth + 1]; + $entries = array_fill_keys(array_keys($folders), []); + $qb = $this->connection->getQueryBuilder(); + $qb->select('fileid', 'parent', 'name', 'mimetype', 'size', 'mtime') + ->from('filecache') + ->where($qb->expr()->in('parent', $qb->createNamedParameter(array_keys($folders), IQueryBuilder::PARAM_INT_ARRAY))) + ->hintShardKey('storage', $storageId); + $cursor = $qb->executeQuery(); + while ($row = $cursor->fetchAssociative()) { + $parent = (int)$row['parent']; + if ((int)$row['mimetype'] === $folderMimeTypeId) { + $foldersToVisit[] = [(int)$row['fileid'], $row['name'], $folders[$parent][1] + 1]; } else { - $previewEntries[] = $entry; + $entries[$parent][] = new CacheEntry($row); } } - - if ($previewEntries === [] || !ctype_digit($folderName)) { - continue; + $cursor->closeCursor(); + + foreach ($folders as $folderId => [$folderName, $depth]) { + if ($entries[$folderId] === [] || !ctype_digit($folderName)) { + if ($depth > 0) { + $emptyFolders[$depth][] = $folderId; + } + continue; + } + $foldersToMigrate[] = ['fileId' => (int)$folderName, 'folderId' => $folderId, 'flat' => $depth === 1, 'entries' => $entries[$folderId]]; } - try { - $this->migrationService->migrateFileId((int)$folderName, flatPath: $depth === 1, entries: $previewEntries); - } catch (\Exception $e) { - $this->logger->error('Failed to migrate preview with fileId: ' . $folderName, [ - 'exception' => $e, - ]); - } + if (count($foldersToMigrate) >= self::BATCH_SIZE) { + $this->migrateFolders($foldersToMigrate); + $foldersToMigrate = []; - if (time() - $startTime > 3600) { - return false; + if (time() - $startTime > 3600) { + return false; + } + } + } + $this->migrateFolders($foldersToMigrate); + + krsort($emptyFolders); + foreach ($emptyFolders as $folderIds) { + foreach (array_chunk($folderIds, 1000) as $chunk) { + $qb = $this->connection->getQueryBuilder(); + $qb->selectDistinct('parent') + ->from('filecache') + ->where($qb->expr()->in('parent', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))) + ->hintShardKey('storage', $storageId); + $nonEmpty = array_map('intval', $qb->executeQuery()->fetchFirstColumn()); + $this->migrationService->deleteOldFileCacheEntries(array_values(array_diff($chunk, $nonEmpty))); } } return true; } + /** + * @param list}> $folders + */ + private function migrateFolders(array $folders): void { + try { + $this->migrationService->migrateFolders($folders); + } catch (\Exception $e) { + $this->logger->error('Failed to migrate previews of file ids: ' . implode(', ', array_column($folders, 'fileId')), [ + 'exception' => $e, + ]); + } + } + private function belongsToPartition(string $folderName, int $partition): bool { if ($partition < 0 || $partition >= self::PARTITIONS) { return false; diff --git a/lib/private/Preview/Db/PreviewMapper.php b/lib/private/Preview/Db/PreviewMapper.php index 00d42dac037f0..f7622483926c1 100644 --- a/lib/private/Preview/Db/PreviewMapper.php +++ b/lib/private/Preview/Db/PreviewMapper.php @@ -29,6 +29,7 @@ class PreviewMapper extends QBMapper { private const string TABLE_NAME = 'previews'; private const string LOCATION_TABLE_NAME = 'preview_locations'; private const string VERSION_TABLE_NAME = 'preview_versions'; + private const int MAX_INSERT_PARAMETERS = 900; public const MAX_CHUNK_SIZE = 1000; // Columns selected by joinLocation() that do not belong to the previews table @@ -76,6 +77,75 @@ public function insert(Entity $entity): Entity { return parent::insert($preview); } + /** + * Insert many migrated previews using multi-row INSERT statements. + * + * Only the columns set by the preview migration are written. + * + * @param list $previews + */ + public function insertMany(array $previews): void { + $columns = [ + 'id' => IQueryBuilder::PARAM_STR, + 'file_id' => IQueryBuilder::PARAM_INT, + 'storage_id' => IQueryBuilder::PARAM_INT, + 'old_file_id' => IQueryBuilder::PARAM_INT, + 'width' => IQueryBuilder::PARAM_INT, + 'height' => IQueryBuilder::PARAM_INT, + 'mimetype_id' => IQueryBuilder::PARAM_INT, + 'source_mimetype_id' => IQueryBuilder::PARAM_INT, + 'mtime' => IQueryBuilder::PARAM_INT, + 'size' => IQueryBuilder::PARAM_INT, + 'max' => IQueryBuilder::PARAM_BOOL, + 'cropped' => IQueryBuilder::PARAM_BOOL, + 'encrypted' => IQueryBuilder::PARAM_BOOL, + 'etag' => IQueryBuilder::PARAM_STR, + ]; + + // Oracle before 23ai does not support multi-row VALUES. + $multiRow = $this->db->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE; + $bulk = []; + foreach ($previews as $preview) { + if (!$multiRow || ($preview->getVersion() !== null && $preview->getVersion() !== '')) { + $this->insert($preview); + continue; + } + $preview->generateId(); + $bulk[] = $preview; + } + + $platform = $this->db->getDatabasePlatform(); + $sql = 'INSERT INTO ' . $this->db->getQueryBuilder()->getTableName(self::TABLE_NAME) + . ' (' . implode(', ', array_map($platform->quoteSingleIdentifier(...), array_keys($columns))) . ') VALUES '; + $placeholders = '(' . implode(', ', array_fill(0, count($columns), '?')) . ')'; + $types = array_values($columns); + + foreach (array_chunk($bulk, intdiv(self::MAX_INSERT_PARAMETERS, count($columns))) as $chunk) { + $params = []; + $paramTypes = []; + foreach ($chunk as $preview) { + array_push($params, + $preview->getId(), + $preview->getFileId(), + $preview->getStorageId(), + $preview->getOldFileId(), + $preview->getWidth(), + $preview->getHeight(), + $this->mimeTypeLoader->getId($preview->getMimeType()), + $this->mimeTypeLoader->getId($preview->getSourceMimeType()), + $preview->getMtime(), + $preview->getSize(), + $preview->isMax(), + $preview->isCropped(), + $preview->isEncrypted(), + $preview->getEtag(), + ); + array_push($paramTypes, ...$types); + } + $this->db->executeStatement($sql . implode(', ', array_fill(0, count($chunk), $placeholders)), $params, $paramTypes); + } + } + #[Override] public function update(Entity $entity): Entity { /** @var Preview $preview */ diff --git a/lib/private/Preview/PreviewMigrationService.php b/lib/private/Preview/PreviewMigrationService.php index 8853c4c336ac9..5fc3be2995e3e 100644 --- a/lib/private/Preview/PreviewMigrationService.php +++ b/lib/private/Preview/PreviewMigrationService.php @@ -140,32 +140,111 @@ public function migrateFileId(int $fileId, bool $flatPath, ?array $entries = nul $this->deleteOldFileCacheEntries($oldFileIdsToDelete); } } else { - // No matching fileId, delete the orphaned preview files themselves. - $transactionStarted = false; - try { - $folder = $this->appData->getFolder($internalPath); - $this->connection->beginTransaction(); - $transactionStarted = true; - foreach ($folder->getDirectoryListing() as $file) { - $file->delete(); + $this->deleteOrphanedPreviews($internalPath, $entries); + } + + $this->deleteFolder($internalPath); + + return $previews; + } + + /** + * Migrate the previews of many legacy preview folders at once. + * + * The inserts of the whole batch share one transaction. If any of them fails, + * e.g. because a preview was migrated concurrently, the batch is rolled back + * and each folder is retried individually with migrateFileId(). + * + * @param list}> $folders + */ + public function migrateFolders(array $folders): void { + if ($folders === []) { + return; + } + + $sources = []; + foreach (array_chunk(array_values(array_unique(array_column($folders, 'fileId'))), 1000) as $chunk) { + $qb = $this->connection->getQueryBuilder(); + $qb->select('fileid', 'storage', 'etag', 'mimetype') + ->from('filecache') + ->where($qb->expr()->in('fileid', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + $cursor = $qb->executeQuery(); + while ($row = $cursor->fetchAssociative()) { + $sources[(int)$row['fileid']] = $row; + } + $cursor->closeCursor(); + } + + $previewsToInsert = []; + $rowsToDelete = []; + foreach ($folders as $folder) { + $fileId = $folder['fileId']; + if (!isset($sources[$fileId])) { + $this->deleteOrphanedPreviews(self::getInternalFolder((string)$fileId, $folder['flat']), $folder['entries']); + $rowsToDelete[] = $folder['folderId']; + continue; + } + + $source = $sources[$fileId]; + $sourceMimeType = $this->mimeTypeLoader->getMimetypeById((int)$source['mimetype']); + foreach ($folder['entries'] as $entry) { + $preview = Preview::fromPath($fileId . '/' . $entry->getName(), $this->mimeTypeDetector); + if ($preview === false) { + $this->logger->error('Unable to import old preview at path.'); + continue; } - $this->connection->commit(); - } catch (NotFoundException) { - // Folder already gone, nothing to clean up. - } catch (\Throwable $e) { - // Also catches non-DB failures from $file->delete(), e.g. an unreachable objectstore. - if ($transactionStarted) { - $this->connection->rollback(); + $preview->setSize($entry->getSize()); + $preview->setMtime($entry->getMTime()); + $preview->setOldFileId($entry->getId()); + $preview->setEncrypted(false); + $preview->setStorageId($source['storage']); + $preview->setEtag($source['etag']); + $preview->setSourceMimeType($sourceMimeType); + $previewsToInsert[] = $preview; + $rowsToDelete[] = $entry->getId(); + } + $rowsToDelete[] = $folder['folderId']; + } + + $this->connection->beginTransaction(); + try { + $this->previewMapper->insertMany($previewsToInsert); + foreach ($previewsToInsert as $preview) { + $this->storageFactory->migratePreview($preview); + } + $this->deleteOldFileCacheEntries($rowsToDelete); + $this->connection->commit(); + } catch (\Exception $e) { + $this->connection->rollBack(); + $this->logger->info('Batch preview migration failed, retrying folder by folder.', ['exception' => $e]); + foreach ($folders as $folder) { + if (isset($sources[$folder['fileId']])) { + $this->migrateFileId($folder['fileId'], $folder['flat'], $folder['entries']); } - $this->logger->error('Unable to delete orphaned preview at ' . $internalPath, [ + } + } + } + + /** + * Delete preview files whose source file no longer exists, together with their filecache rows. + * + * @param list $entries + */ + private function deleteOrphanedPreviews(string $internalPath, array $entries): void { + $storage = $this->rootFolder->getMountPoint()->getStorage(); + $fileIds = []; + foreach ($entries as $entry) { + try { + $storage->unlink($this->previewRootPath . $internalPath . '/' . $entry->getName()); + } catch (\Exception $e) { + $this->logger->error('Unable to delete orphaned preview at ' . $internalPath . '/' . $entry->getName(), [ 'exception' => $e, ]); + continue; } + $fileIds[] = $entry->getId(); } - - $this->deleteFolder($internalPath); - - return $previews; + $this->deleteOldFileCacheEntries($fileIds); } private static function getInternalFolder(string $name, bool $flatPath): string { @@ -178,7 +257,7 @@ private static function getInternalFolder(string $name, bool $flatPath): string /** * @param list $fileIds */ - private function deleteOldFileCacheEntries(array $fileIds): void { + public function deleteOldFileCacheEntries(array $fileIds): void { if ($fileIds === []) { return; } diff --git a/tests/lib/Preview/PreviewMigrationJobTest.php b/tests/lib/Preview/PreviewMigrationJobTest.php index bc01eff98d1ce..7fd4f56eb8f51 100644 --- a/tests/lib/Preview/PreviewMigrationJobTest.php +++ b/tests/lib/Preview/PreviewMigrationJobTest.php @@ -144,6 +144,8 @@ private function createJob(): PreviewMigrationJob { Server::get(IAppDataFactory::class), ), Server::get(IJobList::class), + Server::get(IDBConnection::class), + Server::get(IMimeTypeLoader::class), $this->logger, ); } @@ -246,6 +248,28 @@ public function testMigrationDeletesOrphanedPreview(): void { $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile($orphanFileId)))); } + #[TestDox('Orphaned preview files must be removed from the storage and the filecache even when their filecache rows are not readable')] + public function testMigrationDeletesUnreadableOrphanedPreview(): void { + $orphanFileId = 9999997; + $folder = $this->previewAppData->newFolder((string)$orphanFileId); + $file = $folder->newFile('64-64-crop.jpg', 'abcdefg'); + $fileId = $file->getId(); + $storage = Server::get(IRootFolder::class)->getMountPoint()->getStorage(); + $internalPath = $storage->getCache()->getPathById($fileId); + + $qb = $this->db->getQueryBuilder(); + $qb->update('filecache') + ->set('permissions', $qb->createNamedParameter(0)) + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId))) + ->executeStatement(); + + $this->runAllPartitions(); + + $this->assertFalse($storage->file_exists($internalPath)); + $this->assertEquals(-1, $storage->getCache()->getId($internalPath)); + $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); + } + #[TestDox('A partition must only migrate the legacy preview folders belonging to it')] public function testMigrationOnlyMigratesOwnPartition(): void { $folder = $this->previewAppData->newFolder('5'); From 41afe3de1080ea59a7c9f953b771becf978d46b9 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 30 Sep 2026 17:13:12 +0200 Subject: [PATCH 3/4] perf(preview-migration): Batch object store preview migration Set the location of migrated previews before inserting them, resolving it once per bucket and batch, instead of updating every preview after its insert. Orphaned previews are deleted from the object store by urn directly, with their filecache rows removed in bulk. Migrating 100 000 previews on an object store primary storage with PostgreSQL now takes 24s instead of 123s. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Carl Schwan --- lib/private/Preview/Db/PreviewMapper.php | 2 + .../Preview/PreviewMigrationService.php | 43 +++++---- .../Preview/Storage/IPreviewStorage.php | 7 +- .../Preview/Storage/LocalPreviewStorage.php | 8 +- .../Storage/ObjectStorePreviewStorage.php | 49 ++++++---- .../Preview/Storage/StorageFactory.php | 4 +- tests/lib/Preview/PreviewMapperTest.php | 34 +++++++ .../Storage/ObjectStorePreviewStorageTest.php | 90 +++++++++++++++++++ 8 files changed, 200 insertions(+), 37 deletions(-) create mode 100644 tests/lib/Preview/Storage/ObjectStorePreviewStorageTest.php diff --git a/lib/private/Preview/Db/PreviewMapper.php b/lib/private/Preview/Db/PreviewMapper.php index f7622483926c1..e9aaa3fc6bc38 100644 --- a/lib/private/Preview/Db/PreviewMapper.php +++ b/lib/private/Preview/Db/PreviewMapper.php @@ -90,6 +90,7 @@ public function insertMany(array $previews): void { 'file_id' => IQueryBuilder::PARAM_INT, 'storage_id' => IQueryBuilder::PARAM_INT, 'old_file_id' => IQueryBuilder::PARAM_INT, + 'location_id' => IQueryBuilder::PARAM_INT, 'width' => IQueryBuilder::PARAM_INT, 'height' => IQueryBuilder::PARAM_INT, 'mimetype_id' => IQueryBuilder::PARAM_INT, @@ -129,6 +130,7 @@ public function insertMany(array $previews): void { $preview->getFileId(), $preview->getStorageId(), $preview->getOldFileId(), + $preview->getLocationId(), $preview->getWidth(), $preview->getHeight(), $this->mimeTypeLoader->getId($preview->getMimeType()), diff --git a/lib/private/Preview/PreviewMigrationService.php b/lib/private/Preview/PreviewMigrationService.php index 5fc3be2995e3e..5664832970d9c 100644 --- a/lib/private/Preview/PreviewMigrationService.php +++ b/lib/private/Preview/PreviewMigrationService.php @@ -9,7 +9,9 @@ namespace OC\Preview; +use OC\Files\ObjectStore\ObjectStoreStorage; use OC\Files\SimpleFS\SimpleFile; +use OC\Files\Storage\Wrapper\Wrapper; use OC\Preview\Db\Preview; use OC\Preview\Db\PreviewMapper; use OC\Preview\Storage\StorageFactory; @@ -107,10 +109,14 @@ public function migrateFileId(int $fileId, bool $flatPath, ?array $entries = nul $preview->setSourceMimeType($this->mimeTypeLoader->getMimetypeById((int)$result['mimetype'])); $preview->generateId(); - // Commit the insert and the storage migration together, one commit per preview. + // Commit the storage migration and the insert together, one commit per preview. + // Do not delete the old file via a Node afterwards, as that would also + // delete it from the file system; only its filecache row is stale. $this->connection->beginTransaction(); try { + $this->storageFactory->migratePreviews([$preview]); $preview = $this->previewMapper->insert($preview); + $this->connection->commit(); } catch (Exception $e) { $this->connection->rollBack(); if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { @@ -120,15 +126,7 @@ public function migrateFileId(int $fileId, bool $flatPath, ?array $entries = nul // We already have this preview in the preview table, skip $oldFileIdsToDelete[] = $preview->getOldFileId(); continue; - } - - try { - $this->storageFactory->migratePreview($preview); - // Do not delete the old file via a Node here, as that would also - // delete it from the file system; only its filecache row is stale. - $this->connection->commit(); } catch (\Exception $e) { - // Also rolls back the insert above. $this->connection->rollBack(); throw $e; } @@ -208,10 +206,8 @@ public function migrateFolders(array $folders): void { $this->connection->beginTransaction(); try { + $this->storageFactory->migratePreviews($previewsToInsert); $this->previewMapper->insertMany($previewsToInsert); - foreach ($previewsToInsert as $preview) { - $this->storageFactory->migratePreview($preview); - } $this->deleteOldFileCacheEntries($rowsToDelete); $this->connection->commit(); } catch (\Exception $e) { @@ -232,15 +228,28 @@ public function migrateFolders(array $folders): void { */ private function deleteOrphanedPreviews(string $internalPath, array $entries): void { $storage = $this->rootFolder->getMountPoint()->getStorage(); + // Delete objects by urn directly, the filecache rows are removed in bulk below. + $objectStoreStorage = null; + if ($storage->instanceOfStorage(ObjectStoreStorage::class)) { + $objectStoreStorage = $storage instanceof Wrapper ? $storage->getInstanceOfStorage(ObjectStoreStorage::class) : $storage; + } + $fileIds = []; foreach ($entries as $entry) { try { - $storage->unlink($this->previewRootPath . $internalPath . '/' . $entry->getName()); + if ($objectStoreStorage instanceof ObjectStoreStorage) { + $objectStoreStorage->getObjectStore()->deleteObject($objectStoreStorage->getURN($entry->getId())); + } else { + $storage->unlink($this->previewRootPath . $internalPath . '/' . $entry->getName()); + } } catch (\Exception $e) { - $this->logger->error('Unable to delete orphaned preview at ' . $internalPath . '/' . $entry->getName(), [ - 'exception' => $e, - ]); - continue; + // An object that is already gone only leaves its filecache row to remove. + if ($e->getCode() !== 404) { + $this->logger->error('Unable to delete orphaned preview at ' . $internalPath . '/' . $entry->getName(), [ + 'exception' => $e, + ]); + continue; + } } $fileIds[] = $entry->getId(); } diff --git a/lib/private/Preview/Storage/IPreviewStorage.php b/lib/private/Preview/Storage/IPreviewStorage.php index 85e91655a8fca..9394e9ccfa033 100644 --- a/lib/private/Preview/Storage/IPreviewStorage.php +++ b/lib/private/Preview/Storage/IPreviewStorage.php @@ -51,10 +51,15 @@ public function deleteUnreferencedPreview(Preview $preview): void; /** * Migration helper * + * Called before the migrated preview rows are inserted, inside the same + * transaction. It may set storage specific fields on the previews but must + * not write the preview rows themselves. + * * To remove at some point + * @param list $previews * @throws Exception */ - public function migratePreview(Preview $preview): void; + public function migratePreviews(array $previews): void; /** * @throws NotPermittedException diff --git a/lib/private/Preview/Storage/LocalPreviewStorage.php b/lib/private/Preview/Storage/LocalPreviewStorage.php index b2218a4d02da2..fec0da4bfe507 100644 --- a/lib/private/Preview/Storage/LocalPreviewStorage.php +++ b/lib/private/Preview/Storage/LocalPreviewStorage.php @@ -99,7 +99,13 @@ private function createParentFiles(string $path): void { } #[Override] - public function migratePreview(Preview $preview): void { + public function migratePreviews(array $previews): void { + foreach ($previews as $preview) { + $this->migratePreview($preview); + } + } + + private function migratePreview(Preview $preview): void { // legacy flat directory $sourcePath = $this->getPreviewRootFolder() . $preview->getFileId() . '/' . $preview->getName(); if (!file_exists($sourcePath)) { diff --git a/lib/private/Preview/Storage/ObjectStorePreviewStorage.php b/lib/private/Preview/Storage/ObjectStorePreviewStorage.php index 037588e72c825..111a6f8543fa6 100644 --- a/lib/private/Preview/Storage/ObjectStorePreviewStorage.php +++ b/lib/private/Preview/Storage/ObjectStorePreviewStorage.php @@ -98,10 +98,17 @@ public function deletePreview(Preview $preview): void { } #[Override] - public function migratePreview(Preview $preview): void { - // Just set the Preview::bucket and Preview::objectStore - $this->getObjectStoreInfoForNewPreview($preview, migration: true); - $this->previewMapper->update($preview); + public function migratePreviews(array $previews): void { + // The objects keep their urn, only the location of the previews has to be set. + $locationIds = []; + foreach ($previews as $preview) { + [$objectStoreName, $config] = $this->getConfigForNewPreview($preview, migration: true); + $bucketName = $config['arguments']['bucket']; + $locationIds[$objectStoreName][$bucketName] ??= $this->previewMapper->getLocationId($bucketName, $objectStoreName); + $preview->setLocationId($locationIds[$objectStoreName][$bucketName]); + $preview->setObjectStoreName($objectStoreName); + $preview->setBucketName($bucketName); + } } /** @@ -126,7 +133,27 @@ private function getObjectStoreInfoForExistingPreview(Preview $preview): array { /** * @return ObjectStoreDefinition */ - private function getObjectStoreInfoForNewPreview(Preview $preview, bool $migration = false): array { + private function getObjectStoreInfoForNewPreview(Preview $preview): array { + [$objectStoreName, $config] = $this->getConfigForNewPreview($preview); + $bucketName = $config['arguments']['bucket']; + + // Get the locationId corresponding to the bucketName and objectStoreName, this will create + // a new one, if no matching location is found in the DB. + $locationId = $this->previewMapper->getLocationId($bucketName, $objectStoreName); + $preview->setLocationId($locationId); + $preview->setObjectStoreName($objectStoreName); + $preview->setBucketName($bucketName); + + return [ + 'urn' => $this->getUrn($preview, $config), + 'store' => $this->getObjectStore($objectStoreName, $config), + ]; + } + + /** + * @return array{0: string, 1: ObjectStoreConfig} the object store name and its configuration with the bucket of the preview + */ + private function getConfigForNewPreview(Preview $preview, bool $migration = false): array { // When migrating old previews, use the 'root' object store configuration $config = $this->objectStoreConfig->getObjectStoreConfiguration($migration ? 'root' : 'preview'); $objectStoreName = $this->objectStoreConfig->resolveAlias($migration ? 'root' : 'preview'); @@ -145,17 +172,7 @@ private function getObjectStoreInfoForNewPreview(Preview $preview, bool $migrati } $config['arguments']['bucket'] = $bucketName; - // Get the locationId corresponding to the bucketName and objectStoreName, this will create - // a new one, if no matching location is found in the DB. - $locationId = $this->previewMapper->getLocationId($bucketName, $objectStoreName); - $preview->setLocationId($locationId); - $preview->setObjectStoreName($objectStoreName); - $preview->setBucketName($bucketName); - - return [ - 'urn' => $this->getUrn($preview, $config), - 'store' => $this->getObjectStore($objectStoreName, $config), - ]; + return [$objectStoreName, $config]; } private function getObjectStore(string $objectStoreName, array $config): IObjectStore { diff --git a/lib/private/Preview/Storage/StorageFactory.php b/lib/private/Preview/Storage/StorageFactory.php index e61e3402ac785..80bc44a28251e 100644 --- a/lib/private/Preview/Storage/StorageFactory.php +++ b/lib/private/Preview/Storage/StorageFactory.php @@ -57,8 +57,8 @@ private function getBackend(): IPreviewStorage { } #[Override] - public function migratePreview(Preview $preview): void { - $this->getBackend()->migratePreview($preview); + public function migratePreviews(array $previews): void { + $this->getBackend()->migratePreviews($previews); } #[Override] diff --git a/tests/lib/Preview/PreviewMapperTest.php b/tests/lib/Preview/PreviewMapperTest.php index 625147092a5fb..b790e8512615e 100644 --- a/tests/lib/Preview/PreviewMapperTest.php +++ b/tests/lib/Preview/PreviewMapperTest.php @@ -165,6 +165,40 @@ public function testGetPreviewForSpecificationOnJoinedColumn(): void { $this->assertEquals($previewId, $preview->getId()); } + public function testInsertManyStoresLocation(): void { + $locationId = $this->previewMapper->getLocationId('preview-3', 'default'); + $previews = []; + foreach ([4246 => $locationId, 4247 => null] as $fileId => $previewLocationId) { + $preview = new Preview(); + $preview->setFileId($fileId); + $preview->setStorageId(1); + $preview->setOldFileId($fileId + 1000); + $preview->setCropped(false); + $preview->setMax(true); + $preview->setEncrypted(false); + $preview->setWidth(64); + $preview->setHeight(64); + $preview->setSize(100); + $preview->setMtime(time()); + $preview->setMimetype('image/jpeg'); + $preview->setSourceMimeType('image/jpeg'); + $preview->setEtag('abcdefg'); + if ($previewLocationId !== null) { + $preview->setLocationId($previewLocationId); + } + $previews[] = $preview; + } + + $this->previewMapper->insertMany($previews); + + $stored = $this->previewMapper->getAvailablePreviews([4246, 4247]); + $this->assertSame($locationId, (string)$stored[4246][0]->getLocationId()); + $this->assertSame('preview-3', $stored[4246][0]->getBucketName()); + $this->assertSame('default', $stored[4246][0]->getObjectStoreName()); + $this->assertSame(5246, $stored[4246][0]->getOldFileId()); + $this->assertNull($stored[4247][0]->getLocationId()); + } + public function testLargeIdInsertRetrieve(): void { $fileId = PHP_INT_MAX; $originalPreviewId = $this->createPreviewForFileId($fileId); diff --git a/tests/lib/Preview/Storage/ObjectStorePreviewStorageTest.php b/tests/lib/Preview/Storage/ObjectStorePreviewStorageTest.php new file mode 100644 index 0000000000000..512f06841c19c --- /dev/null +++ b/tests/lib/Preview/Storage/ObjectStorePreviewStorageTest.php @@ -0,0 +1,90 @@ +objectStoreConfig = $this->createMock(PrimaryObjectStoreConfig::class); + $this->config = $this->createMock(IConfig::class); + $this->previewMapper = $this->createMock(PreviewMapper::class); + $this->objectStoreConfig->method('resolveAlias')->with('root')->willReturn('default'); + } + + private function createStorage(bool $multibucket, bool $distribution): ObjectStorePreviewStorage { + $this->objectStoreConfig->method('getObjectStoreConfiguration')->with('root')->willReturn([ + 'class' => IObjectStore::class, + 'arguments' => ['bucket' => 'nc', 'multibucket' => $multibucket], + ]); + $this->config->method('getSystemValueBool') + ->with('objectstore.multibucket.preview-distribution') + ->willReturn($distribution); + return new ObjectStorePreviewStorage($this->objectStoreConfig, $this->config, $this->previewMapper); + } + + private static function createPreview(int $fileId): Preview { + $preview = new Preview(); + $preview->setFileId($fileId); + return $preview; + } + + public function testMigratePreviewsLooksUpTheLocationOncePerBucket(): void { + $storage = $this->createStorage(multibucket: false, distribution: false); + $previews = [self::createPreview(1), self::createPreview(2), self::createPreview(3)]; + + $this->previewMapper->expects($this->once()) + ->method('getLocationId') + ->with('nc', 'default') + ->willReturn('42'); + $this->previewMapper->expects($this->never())->method('update'); + $this->previewMapper->expects($this->never())->method('insert'); + + $storage->migratePreviews($previews); + + foreach ($previews as $preview) { + $this->assertSame('42', $preview->getLocationId()); + $this->assertSame('nc', $preview->getBucketName()); + $this->assertSame('default', $preview->getObjectStoreName()); + } + } + + public function testMigratePreviewsWithMultibucketDistribution(): void { + $storage = $this->createStorage(multibucket: true, distribution: true); + // md5('1') and md5('2') start with 'c', md5('3') with 'e' + $previews = [self::createPreview(1), self::createPreview(2), self::createPreview(3)]; + + $this->previewMapper->expects($this->exactly(2)) + ->method('getLocationId') + ->willReturnMap([ + ['nc-preview-204', 'default', '1'], + ['nc-preview-238', 'default', '2'], + ]); + + $storage->migratePreviews($previews); + + $this->assertSame('1', $previews[0]->getLocationId()); + $this->assertSame('1', $previews[1]->getLocationId()); + $this->assertSame('2', $previews[2]->getLocationId()); + $this->assertSame('nc-preview-238', $previews[2]->getBucketName()); + } +} From 3341fcd5fa8a4ced4b6eb7c969eb963c42f0da42 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 30 Sep 2026 18:31:37 +0200 Subject: [PATCH 4/4] refactor(preview-migration): Simplify the preview migration Move the filecache queries from the job into the migration service, so the job only walks the preview folders of its partition, and share the source file lookup and the preview creation between the batch and the single file migration. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Carl Schwan --- core/BackgroundJobs/PreviewMigrationJob.php | 102 ++----- lib/private/Preview/Db/PreviewMapper.php | 4 +- .../Preview/PreviewMigrationService.php | 276 ++++++++++-------- tests/lib/Preview/PreviewMigrationJobTest.php | 8 +- 4 files changed, 192 insertions(+), 198 deletions(-) diff --git a/core/BackgroundJobs/PreviewMigrationJob.php b/core/BackgroundJobs/PreviewMigrationJob.php index 35d62382b9add..e36bb9579d788 100644 --- a/core/BackgroundJobs/PreviewMigrationJob.php +++ b/core/BackgroundJobs/PreviewMigrationJob.php @@ -9,18 +9,13 @@ namespace OC\Core\BackgroundJobs; -use OC\Files\Cache\CacheEntry; use OC\Preview\PreviewMigrationService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; use OCP\BackgroundJob\TimedJob; -use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\Files\FileInfo; -use OCP\Files\IMimeTypeLoader; use OCP\Files\IRootFolder; use OCP\IAppConfig; use OCP\IConfig; -use OCP\IDBConnection; use Override; use Psr\Log\LoggerInterface; @@ -36,8 +31,6 @@ public function __construct( private readonly IRootFolder $rootFolder, private readonly PreviewMigrationService $migrationService, private readonly IJobList $jobList, - private readonly IDBConnection $connection, - private readonly IMimeTypeLoader $mimeTypeLoader, private readonly LoggerInterface $logger, ) { parent::__construct($time); @@ -81,8 +74,7 @@ private function runPartition(int $partition): bool { return true; } - $cache = $storage->getCache(); - $previewRootId = $cache->getId(rtrim($this->previewRootPath, '/')); + $previewRootId = $storage->getCache()->getId(rtrim($this->previewRootPath, '/')); if ($previewRootId === -1) { $this->logger->warning('Preview migration skipped: no preview root found at "{path}" on storage "{storageId}".', [ 'path' => $this->previewRootPath, @@ -92,54 +84,38 @@ private function runPartition(int $partition): bool { } $startTime = time(); - $storageId = $cache->getNumericStorageId(); - $folderMimeTypeId = $this->mimeTypeLoader->getId(FileInfo::MIMETYPE_FOLDER); - $foldersToVisit = [[$previewRootId, '', 0]]; + $foldersToVisit = [['id' => $previewRootId, 'name' => '', 'depth' => 0]]; $foldersToMigrate = []; // Folders without preview files, by depth; removed once their children are gone. $emptyFolders = []; while ($foldersToVisit !== []) { - $folders = []; - foreach (array_splice($foldersToVisit, -self::BATCH_SIZE) as [$folderId, $folderName, $depth]) { - if ($depth === 1 && !$this->belongsToPartition($folderName, $partition)) { - continue; + $folders = array_filter( + array_splice($foldersToVisit, -self::BATCH_SIZE), + fn (array $folder): bool => $folder['depth'] !== 1 || $this->belongsToPartition($folder['name'], $partition), + ); + $children = $this->migrationService->getFolderChildren(array_column($folders, 'id')); + + foreach ($folders as $folder) { + foreach ($children[$folder['id']]['folders'] as $subFolder) { + $foldersToVisit[] = [...$subFolder, 'depth' => $folder['depth'] + 1]; } - $folders[$folderId] = [$folderName, $depth]; - } - if ($folders === []) { - continue; - } - $entries = array_fill_keys(array_keys($folders), []); - $qb = $this->connection->getQueryBuilder(); - $qb->select('fileid', 'parent', 'name', 'mimetype', 'size', 'mtime') - ->from('filecache') - ->where($qb->expr()->in('parent', $qb->createNamedParameter(array_keys($folders), IQueryBuilder::PARAM_INT_ARRAY))) - ->hintShardKey('storage', $storageId); - $cursor = $qb->executeQuery(); - while ($row = $cursor->fetchAssociative()) { - $parent = (int)$row['parent']; - if ((int)$row['mimetype'] === $folderMimeTypeId) { - $foldersToVisit[] = [(int)$row['fileid'], $row['name'], $folders[$parent][1] + 1]; - } else { - $entries[$parent][] = new CacheEntry($row); - } - } - $cursor->closeCursor(); - - foreach ($folders as $folderId => [$folderName, $depth]) { - if ($entries[$folderId] === [] || !ctype_digit($folderName)) { - if ($depth > 0) { - $emptyFolders[$depth][] = $folderId; - } - continue; + $files = $children[$folder['id']]['files']; + if ($files !== [] && ctype_digit($folder['name'])) { + $foldersToMigrate[] = [ + 'fileId' => (int)$folder['name'], + 'folderId' => $folder['id'], + 'flat' => $folder['depth'] === 1, + 'entries' => $files, + ]; + } elseif ($folder['depth'] > 0) { + $emptyFolders[$folder['depth']][] = $folder['id']; } - $foldersToMigrate[] = ['fileId' => (int)$folderName, 'folderId' => $folderId, 'flat' => $depth === 1, 'entries' => $entries[$folderId]]; } if (count($foldersToMigrate) >= self::BATCH_SIZE) { - $this->migrateFolders($foldersToMigrate); + $this->migrationService->migrateFolders($foldersToMigrate); $foldersToMigrate = []; if (time() - $startTime > 3600) { @@ -147,42 +123,14 @@ private function runPartition(int $partition): bool { } } } - $this->migrateFolders($foldersToMigrate); - - krsort($emptyFolders); - foreach ($emptyFolders as $folderIds) { - foreach (array_chunk($folderIds, 1000) as $chunk) { - $qb = $this->connection->getQueryBuilder(); - $qb->selectDistinct('parent') - ->from('filecache') - ->where($qb->expr()->in('parent', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))) - ->hintShardKey('storage', $storageId); - $nonEmpty = array_map('intval', $qb->executeQuery()->fetchFirstColumn()); - $this->migrationService->deleteOldFileCacheEntries(array_values(array_diff($chunk, $nonEmpty))); - } - } - return true; - } + $this->migrationService->migrateFolders($foldersToMigrate); + $this->migrationService->deleteEmptyFolders($emptyFolders); - /** - * @param list}> $folders - */ - private function migrateFolders(array $folders): void { - try { - $this->migrationService->migrateFolders($folders); - } catch (\Exception $e) { - $this->logger->error('Failed to migrate previews of file ids: ' . implode(', ', array_column($folders, 'fileId')), [ - 'exception' => $e, - ]); - } + return true; } private function belongsToPartition(string $folderName, int $partition): bool { - if ($partition < 0 || $partition >= self::PARTITIONS) { - return false; - } - if (ctype_digit($folderName)) { return ((int)$folderName % self::PARTITIONS) === $partition; } diff --git a/lib/private/Preview/Db/PreviewMapper.php b/lib/private/Preview/Db/PreviewMapper.php index e9aaa3fc6bc38..22108ed6446f5 100644 --- a/lib/private/Preview/Db/PreviewMapper.php +++ b/lib/private/Preview/Db/PreviewMapper.php @@ -60,6 +60,8 @@ public function insert(Entity $entity): Entity { /** @var Preview $preview */ $preview = $entity; + // The version row reuses the preview id, so it has to exist before it is written. + $preview->generateId(); $preview->setMimetypeId($this->mimeTypeLoader->getId($preview->getMimeType())); $preview->setSourceMimetypeId($this->mimeTypeLoader->getId($preview->getSourceMimeType())); @@ -90,7 +92,7 @@ public function insertMany(array $previews): void { 'file_id' => IQueryBuilder::PARAM_INT, 'storage_id' => IQueryBuilder::PARAM_INT, 'old_file_id' => IQueryBuilder::PARAM_INT, - 'location_id' => IQueryBuilder::PARAM_INT, + 'location_id' => IQueryBuilder::PARAM_STR, 'width' => IQueryBuilder::PARAM_INT, 'height' => IQueryBuilder::PARAM_INT, 'mimetype_id' => IQueryBuilder::PARAM_INT, diff --git a/lib/private/Preview/PreviewMigrationService.php b/lib/private/Preview/PreviewMigrationService.php index 5664832970d9c..09b218a93d21d 100644 --- a/lib/private/Preview/PreviewMigrationService.php +++ b/lib/private/Preview/PreviewMigrationService.php @@ -9,6 +9,7 @@ namespace OC\Preview; +use OC\Files\Cache\CacheEntry; use OC\Files\ObjectStore\ObjectStoreStorage; use OC\Files\SimpleFS\SimpleFile; use OC\Files\Storage\Wrapper\Wrapper; @@ -19,6 +20,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\FileInfo; use OCP\Files\IAppData; use OCP\Files\IMimeTypeDetector; use OCP\Files\IMimeTypeLoader; @@ -52,8 +54,7 @@ public function __construct( * @return Preview[] */ public function migrateFileId(int $fileId, bool $flatPath, ?array $entries = null): array { - $previews = []; - $internalPath = $this->getInternalFolder((string)$fileId, $flatPath); + $internalPath = self::getInternalFolder((string)$fileId, $flatPath); if ($entries === null) { try { @@ -63,82 +64,39 @@ public function migrateFileId(int $fileId, bool $flatPath, ?array $entries = nul } } - /** - * @var list $previewsToInsert - */ - $previewsToInsert = []; - - foreach ($entries as $entry) { - $path = $fileId . '/' . $entry->getName(); - $preview = Preview::fromPath($path, $this->mimeTypeDetector); - if ($preview === false) { - $this->logger->error('Unable to import old preview at path.'); - continue; - } - $preview->generateId(); - $preview->setSize($entry->getSize()); - $preview->setMtime($entry->getMTime()); - $preview->setOldFileId($entry->getId()); - $preview->setEncrypted(false); - - $previewsToInsert[] = $preview; - } - - if (empty($previewsToInsert)) { + $source = $this->getSourceFiles([$fileId])[$fileId] ?? null; + if ($source === null) { + $this->deleteOrphanedPreviews($internalPath, $entries); $this->deleteFolder($internalPath); - - return $previews; + return []; } - $qb = $this->connection->getQueryBuilder(); - $qb->select('storage', 'etag', 'mimetype') - ->from('filecache') - ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId))) - ->setMaxResults(1); - - $cursor = $qb->executeQuery(); - $result = $cursor->fetchAssociative(); - $cursor->closeCursor(); - - if ($result !== false) { - $oldFileIdsToDelete = []; - try { - foreach ($previewsToInsert as $preview) { - $preview->setStorageId($result['storage']); - $preview->setEtag($result['etag']); - $preview->setSourceMimeType($this->mimeTypeLoader->getMimetypeById((int)$result['mimetype'])); - $preview->generateId(); - - // Commit the storage migration and the insert together, one commit per preview. - // Do not delete the old file via a Node afterwards, as that would also - // delete it from the file system; only its filecache row is stale. - $this->connection->beginTransaction(); - try { - $this->storageFactory->migratePreviews([$preview]); - $preview = $this->previewMapper->insert($preview); - $this->connection->commit(); - } catch (Exception $e) { - $this->connection->rollBack(); - if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { - throw $e; - } - - // We already have this preview in the preview table, skip - $oldFileIdsToDelete[] = $preview->getOldFileId(); - continue; - } catch (\Exception $e) { - $this->connection->rollBack(); + $previews = []; + $oldFileIdsToDelete = []; + try { + foreach ($this->createPreviews($fileId, $entries, $source) as $preview) { + // Commit the storage migration and the insert together, one commit per preview. + // Do not delete the old file via a Node afterwards, as that would also + // delete it from the file system; only its filecache row is stale. + $this->connection->beginTransaction(); + try { + $this->storageFactory->migratePreviews([$preview]); + $previews[] = $this->previewMapper->insert($preview); + $this->connection->commit(); + } catch (Exception $e) { + $this->connection->rollBack(); + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { throw $e; } - - $oldFileIdsToDelete[] = $preview->getOldFileId(); - $previews[] = $preview; + // We already have this preview in the preview table, skip + } catch (\Exception $e) { + $this->connection->rollBack(); + throw $e; } - } finally { - $this->deleteOldFileCacheEntries($oldFileIdsToDelete); + $oldFileIdsToDelete[] = $preview->getOldFileId(); } - } else { - $this->deleteOrphanedPreviews($internalPath, $entries); + } finally { + $this->deleteOldFileCacheEntries($oldFileIdsToDelete); } $this->deleteFolder($internalPath); @@ -160,65 +118,149 @@ public function migrateFolders(array $folders): void { return; } + $sources = $this->getSourceFiles(array_column($folders, 'fileId')); + $previews = []; + $folderIds = []; + foreach ($folders as $folder) { + $source = $sources[$folder['fileId']] ?? null; + if ($source === null) { + $this->deleteOrphanedPreviews(self::getInternalFolder((string)$folder['fileId'], $folder['flat']), $folder['entries']); + } else { + array_push($previews, ...$this->createPreviews($folder['fileId'], $folder['entries'], $source)); + } + $folderIds[] = $folder['folderId']; + } + + $this->connection->beginTransaction(); + try { + $this->storageFactory->migratePreviews($previews); + $this->previewMapper->insertMany($previews); + $this->deleteOldFileCacheEntries([ + ...array_map(static fn (Preview $preview): int => $preview->getOldFileId(), $previews), + ...$folderIds, + ]); + $this->connection->commit(); + return; + } catch (\Exception $e) { + $this->connection->rollBack(); + $this->logger->info('Batch preview migration failed, retrying folder by folder.', ['exception' => $e]); + } + + foreach ($folders as $folder) { + if (!isset($sources[$folder['fileId']])) { + continue; + } + try { + $this->migrateFileId($folder['fileId'], $folder['flat'], $folder['entries']); + } catch (\Exception $e) { + $this->logger->error('Failed to migrate preview with fileId: ' . $folder['fileId'], [ + 'exception' => $e, + ]); + } + } + } + + /** + * List the children of the given folders of the root storage. + * + * @param list $folderIds + * @return array, files: list}> by parent folder id + */ + public function getFolderChildren(array $folderIds): array { + $children = array_fill_keys($folderIds, ['folders' => [], 'files' => []]); + if ($folderIds === []) { + return $children; + } + + $folderMimeTypeId = $this->mimeTypeLoader->getId(FileInfo::MIMETYPE_FOLDER); + $qb = $this->connection->getQueryBuilder(); + $qb->select('fileid', 'parent', 'name', 'mimetype', 'size', 'mtime') + ->from('filecache') + ->where($qb->expr()->in('parent', $qb->createNamedParameter($folderIds, IQueryBuilder::PARAM_INT_ARRAY))) + ->hintShardKey('storage', $this->rootFolder->getMountPoint()->getNumericStorageId()); + $cursor = $qb->executeQuery(); + while ($row = $cursor->fetchAssociative()) { + $parent = (int)$row['parent']; + if ((int)$row['mimetype'] === $folderMimeTypeId) { + $children[$parent]['folders'][] = ['id' => (int)$row['fileid'], 'name' => $row['name']]; + } else { + $children[$parent]['files'][] = new CacheEntry($row); + } + } + $cursor->closeCursor(); + + return $children; + } + + /** + * Delete the given folders of the root storage that have no children anymore. + * + * @param array> $folderIdsByDepth + */ + public function deleteEmptyFolders(array $folderIdsByDepth): void { + // Deepest first, so that a parent is empty once its children are gone. + krsort($folderIdsByDepth); + foreach ($folderIdsByDepth as $folderIds) { + foreach (array_chunk($folderIds, 1000) as $chunk) { + $qb = $this->connection->getQueryBuilder(); + $qb->selectDistinct('parent') + ->from('filecache') + ->where($qb->expr()->in('parent', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))) + ->hintShardKey('storage', $this->rootFolder->getMountPoint()->getNumericStorageId()); + $nonEmpty = array_map('intval', $qb->executeQuery()->fetchFirstColumn()); + $this->deleteOldFileCacheEntries(array_values(array_diff($chunk, $nonEmpty))); + } + } + } + + /** + * @param list $fileIds + * @return array by file id + */ + private function getSourceFiles(array $fileIds): array { $sources = []; - foreach (array_chunk(array_values(array_unique(array_column($folders, 'fileId'))), 1000) as $chunk) { + foreach (array_chunk(array_values(array_unique($fileIds)), 1000) as $chunk) { $qb = $this->connection->getQueryBuilder(); $qb->select('fileid', 'storage', 'etag', 'mimetype') ->from('filecache') ->where($qb->expr()->in('fileid', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); $cursor = $qb->executeQuery(); while ($row = $cursor->fetchAssociative()) { - $sources[(int)$row['fileid']] = $row; + $sources[(int)$row['fileid']] = [ + 'storage' => (int)$row['storage'], + 'etag' => (string)$row['etag'], + 'mimetype' => (int)$row['mimetype'], + ]; } $cursor->closeCursor(); } + return $sources; + } - $previewsToInsert = []; - $rowsToDelete = []; - foreach ($folders as $folder) { - $fileId = $folder['fileId']; - if (!isset($sources[$fileId])) { - $this->deleteOrphanedPreviews(self::getInternalFolder((string)$fileId, $folder['flat']), $folder['entries']); - $rowsToDelete[] = $folder['folderId']; + /** + * @param list $entries + * @param array{storage: int, etag: string, mimetype: int} $source + * @return list + */ + private function createPreviews(int $fileId, array $entries, array $source): array { + $sourceMimeType = $this->mimeTypeLoader->getMimetypeById($source['mimetype']); + $previews = []; + foreach ($entries as $entry) { + $preview = Preview::fromPath($fileId . '/' . $entry->getName(), $this->mimeTypeDetector); + if ($preview === false) { + $this->logger->error('Unable to import old preview at path.'); continue; } - - $source = $sources[$fileId]; - $sourceMimeType = $this->mimeTypeLoader->getMimetypeById((int)$source['mimetype']); - foreach ($folder['entries'] as $entry) { - $preview = Preview::fromPath($fileId . '/' . $entry->getName(), $this->mimeTypeDetector); - if ($preview === false) { - $this->logger->error('Unable to import old preview at path.'); - continue; - } - $preview->setSize($entry->getSize()); - $preview->setMtime($entry->getMTime()); - $preview->setOldFileId($entry->getId()); - $preview->setEncrypted(false); - $preview->setStorageId($source['storage']); - $preview->setEtag($source['etag']); - $preview->setSourceMimeType($sourceMimeType); - $previewsToInsert[] = $preview; - $rowsToDelete[] = $entry->getId(); - } - $rowsToDelete[] = $folder['folderId']; - } - - $this->connection->beginTransaction(); - try { - $this->storageFactory->migratePreviews($previewsToInsert); - $this->previewMapper->insertMany($previewsToInsert); - $this->deleteOldFileCacheEntries($rowsToDelete); - $this->connection->commit(); - } catch (\Exception $e) { - $this->connection->rollBack(); - $this->logger->info('Batch preview migration failed, retrying folder by folder.', ['exception' => $e]); - foreach ($folders as $folder) { - if (isset($sources[$folder['fileId']])) { - $this->migrateFileId($folder['fileId'], $folder['flat'], $folder['entries']); - } - } + $preview->setSize($entry->getSize()); + $preview->setMtime($entry->getMTime()); + $preview->setOldFileId($entry->getId()); + $preview->setEncrypted(false); + $preview->setStorageId($source['storage']); + $preview->setEtag($source['etag']); + $preview->setSourceMimeType($sourceMimeType); + $previews[] = $preview; } + return $previews; } /** @@ -266,7 +308,7 @@ private static function getInternalFolder(string $name, bool $flatPath): string /** * @param list $fileIds */ - public function deleteOldFileCacheEntries(array $fileIds): void { + private function deleteOldFileCacheEntries(array $fileIds): void { if ($fileIds === []) { return; } diff --git a/tests/lib/Preview/PreviewMigrationJobTest.php b/tests/lib/Preview/PreviewMigrationJobTest.php index 7fd4f56eb8f51..9cf8b2dd1460b 100644 --- a/tests/lib/Preview/PreviewMigrationJobTest.php +++ b/tests/lib/Preview/PreviewMigrationJobTest.php @@ -18,6 +18,7 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; use OCP\Files\AppData\IAppDataFactory; +use OCP\Files\FileInfo; use OCP\Files\IAppData; use OCP\Files\IMimeTypeDetector; use OCP\Files\IMimeTypeLoader; @@ -89,7 +90,10 @@ public function setUp(): void { $this->mimeTypeDetector = $this->createMock(IMimeTypeDetector::class); $this->mimeTypeDetector->method('detectPath')->willReturn('image/png'); $this->mimeTypeLoader = $this->createMock(IMimeTypeLoader::class); - $this->mimeTypeLoader->method('getId')->with('image/png')->willReturn(42); + $this->mimeTypeLoader->method('getId')->willReturnMap([ + ['image/png', 42], + [FileInfo::MIMETYPE_FOLDER, Server::get(IMimeTypeLoader::class)->getId(FileInfo::MIMETYPE_FOLDER)], + ]); $this->mimeTypeLoader->method('getMimetypeById')->with(42)->willReturn('image/png'); $this->logger = $this->createMock(LoggerInterface::class); } @@ -144,8 +148,6 @@ private function createJob(): PreviewMigrationJob { Server::get(IAppDataFactory::class), ), Server::get(IJobList::class), - Server::get(IDBConnection::class), - Server::get(IMimeTypeLoader::class), $this->logger, ); }