From f5bfa5ee7d60c020c59845ed72554e9d40460cbc Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 27 Oct 2026 17:41:08 +0100 Subject: [PATCH] feat: Allow to limit the amount of preview migration job running Signed-off-by: Carl Schwan --- core/AppInfo/ConfigLexicon.php | 7 +++ core/BackgroundJobs/PreviewMigrationJob.php | 30 +++++++++++ tests/lib/Preview/PreviewMigrationJobTest.php | 52 +++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/core/AppInfo/ConfigLexicon.php b/core/AppInfo/ConfigLexicon.php index c0b872d5360be..4b141f289ca89 100644 --- a/core/AppInfo/ConfigLexicon.php +++ b/core/AppInfo/ConfigLexicon.php @@ -40,6 +40,7 @@ class ConfigLexicon implements ILexicon { public const LASTCRON_TIMESTAMP = 'lastcron'; public const ON_DEMAND_PREVIEW_MIGRATION = 'on_demand_preview_migration'; + public const PREVIEW_MIGRATION_MAX_PARALLEL_JOBS = 'preview_migration_max_parallel_jobs'; public const APPSTORE_LINK_SHOWN = 'appstore_link_shown'; @@ -107,6 +108,12 @@ public function getAppConfigs(): array { defaultRaw: true, definition: 'Whether on demand preview migration is enabled.' ), + new Entry( + key: self::PREVIEW_MIGRATION_MAX_PARALLEL_JOBS, + type: ValueType::INT, + defaultRaw: 0, + definition: 'Maximum number of preview migration background jobs running in parallel, 0 for no limit.', + ), new Entry( key: self::APPSTORE_LINK_SHOWN, type: ValueType::BOOL, diff --git a/core/BackgroundJobs/PreviewMigrationJob.php b/core/BackgroundJobs/PreviewMigrationJob.php index e36bb9579d788..2e6ba98e5d1eb 100644 --- a/core/BackgroundJobs/PreviewMigrationJob.php +++ b/core/BackgroundJobs/PreviewMigrationJob.php @@ -9,13 +9,16 @@ namespace OC\Core\BackgroundJobs; +use OC\Core\AppInfo\ConfigLexicon; 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\IRootFolder; use OCP\IAppConfig; use OCP\IConfig; +use OCP\IDBConnection; use Override; use Psr\Log\LoggerInterface; @@ -32,6 +35,7 @@ public function __construct( private readonly PreviewMigrationService $migrationService, private readonly IJobList $jobList, private readonly LoggerInterface $logger, + private readonly IDBConnection $connection, ) { parent::__construct($time); $this->setTimeSensitivity(self::TIME_INSENSITIVE); @@ -52,6 +56,15 @@ protected function run(mixed $argument): void { return; } + $maxParallelJobs = $this->appConfig->getValueInt('core', ConfigLexicon::PREVIEW_MIGRATION_MAX_PARALLEL_JOBS); + if ($maxParallelJobs > 0 && $this->countOtherRunningJobs() >= $maxParallelJobs) { + $this->logger->debug('Preview migration of partition {partition} postponed: {max} jobs are already running.', [ + 'partition' => $partition, + 'max' => $maxParallelJobs, + ]); + return; + } + if (!$this->runPartition($partition)) { return; } @@ -130,6 +143,23 @@ private function runPartition(int $partition): bool { return true; } + /** + * Count the other preview migration jobs currently reserved by a cron worker, + * using the same staleness window as {@see \OCP\BackgroundJob\IJobList::hasReservedJob()}. + */ + private function countOtherRunningJobs(): int { + $qb = $this->connection->getQueryBuilder(); + $qb->select($qb->func()->count('*')) + ->from('jobs') + ->where($qb->expr()->eq('class', $qb->createNamedParameter(self::class))) + ->andWhere($qb->expr()->gt('reserved_at', $qb->createNamedParameter($this->time->getTime() - 6 * 3600, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->neq('id', $qb->createNamedParameter((int)$this->getId(), IQueryBuilder::PARAM_INT))); + $result = $qb->executeQuery(); + $count = (int)$result->fetchOne(); + $result->closeCursor(); + return $count; + } + private function belongsToPartition(string $folderName, int $partition): bool { if (ctype_digit($folderName)) { return ((int)$folderName % self::PARTITIONS) === $partition; diff --git a/tests/lib/Preview/PreviewMigrationJobTest.php b/tests/lib/Preview/PreviewMigrationJobTest.php index 9cf8b2dd1460b..53e5e8542fe31 100644 --- a/tests/lib/Preview/PreviewMigrationJobTest.php +++ b/tests/lib/Preview/PreviewMigrationJobTest.php @@ -9,6 +9,7 @@ namespace lib\Preview; +use OC\Core\AppInfo\ConfigLexicon; use OC\Core\BackgroundJobs\PreviewMigrationJob; use OC\Preview\Db\Preview; use OC\Preview\Db\PreviewMapper; @@ -17,6 +18,7 @@ use OC\Preview\Storage\StorageFactory; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\FileInfo; use OCP\Files\IAppData; @@ -149,6 +151,7 @@ private function createJob(): PreviewMigrationJob { ), Server::get(IJobList::class), $this->logger, + $this->db, ); } @@ -287,6 +290,55 @@ public function testMigrationOnlyMigratesOwnPartition(): void { $this->assertEquals(1, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); } + #[TestDox('A partition must not be migrated while the maximum number of parallel jobs are already running')] + public function testMigrationRespectsMaxParallelJobs(): void { + $this->appConfig->method('getValueInt') + ->with('core', ConfigLexicon::PREVIEW_MIGRATION_MAX_PARALLEL_JOBS) + ->willReturn(2); + + $folder = $this->previewAppData->newFolder('5'); + $folder->newFile('64-64-crop.jpg', 'abcdefg'); + + $firstRunningJobId = $this->insertReservedJob(); + $this->insertReservedJob(); + + try { + $job = $this->createJob(); + $this->invokePrivate($job, 'run', [['partition' => 5]]); + $this->assertEquals(1, count($this->previewAppData->getDirectoryListing())); + $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); + + Server::get(IJobList::class)->removeById($firstRunningJobId); + + $this->invokePrivate($job, 'run', [['partition' => 5]]); + $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); + $this->assertEquals(1, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); + } finally { + Server::get(IJobList::class)->remove(PreviewMigrationJob::class); + } + } + + private function insertReservedJob(): string { + $argument = ['partition' => $this->getUniqueID()]; + Server::get(IJobList::class)->add(PreviewMigrationJob::class, $argument); + + $argumentHash = hash('sha256', json_encode($argument)); + $qb = $this->db->getQueryBuilder(); + $qb->update('jobs') + ->set('reserved_at', $qb->createNamedParameter(time(), IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->eq('argument_hash', $qb->createNamedParameter($argumentHash))) + ->executeStatement(); + + $qb = $this->db->getQueryBuilder(); + $result = $qb->select('id') + ->from('jobs') + ->where($qb->expr()->eq('argument_hash', $qb->createNamedParameter($argumentHash))) + ->executeQuery(); + $id = (string)$result->fetchOne(); + $result->closeCursor(); + return $id; + } + #[TestDox('run() must complete without error when there is nothing to migrate')] public function testMigrationWithoutAnyPreviews(): void { $this->assertEquals(0, count($this->previewAppData->getDirectoryListing()));