Skip to content

feat: Allow to limit the amount of preview migration job running #65974

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.

Sign up for GitHub

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

Open
CarlSchwan wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: Allow to limit the amount of preview migration job running #65974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions core/AppInfo/ConfigLexicon.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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,
Expand Down
30 changes: 30 additions & 0 deletions core/BackgroundJobs/PreviewMigrationJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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()}.
Comment on lines +147 to +148

@provokateurin provokateurin Oct 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to move it there?

*/
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;
Expand Down
52 changes: 52 additions & 0 deletions tests/lib/Preview/PreviewMigrationJobTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -149,6 +151,7 @@ private function createJob(): PreviewMigrationJob {
),
Server::get(IJobList::class),
$this->logger,
$this->db,
);
}

Expand Down Expand Up @@ -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()));
Expand Down
Loading