Merge pull request #51820 from nextcloud/backport/50768/stable31

[stable31] perf(cron): Delay (re)checking timed jobs

lib/private/BackgroundJob/JobList.php

@@ -20,6 +20,7 @@ use OCP\IDBConnection;
 use Psr\Log\LoggerInterface;
 use function get_class;
 use function json_encode;
+use function min;
 use function strlen;
 
 class JobList implements IJobList {
@@ -209,6 +210,26 @@ class JobList implements IJobList {
 				return $this->getNext($onlyTimeSensitive, $jobClasses);
 			}
 
+			if ($job instanceof \OCP\BackgroundJob\TimedJob) {
+				$now = $this->timeFactory->getTime();
+				$nextPossibleRun = $job->getLastRun() + $job->getInterval();
+				if ($now < $nextPossibleRun) {
+					// This job is not ready for execution yet. Set timestamps to the future to avoid
+					// re-checking with every cron run.
+					// To avoid bugs that lead to jobs never executing again, the future timestamp is
+					// capped at two days.
+					$nextCheck = min($nextPossibleRun, $now + 48 * 3600);
+					$updateTimedJob = $this->connection->getQueryBuilder();
+					$updateTimedJob->update('jobs')
+						->set('last_checked', $updateTimedJob->createNamedParameter($nextCheck, IQueryBuilder::PARAM_INT))
+						->where($updateTimedJob->expr()->eq('id', $updateTimedJob->createParameter('jobid')));
+					$updateTimedJob->setParameter('jobid', $row['id']);
+					$updateTimedJob->executeStatement();
+
+					return $this->getNext($onlyTimeSensitive, $jobClasses);
+				}
+			}
+
 			$update = $this->connection->getQueryBuilder();
 			$update->update('jobs')
 				->set('reserved_at', $update->createNamedParameter($this->timeFactory->getTime()))

lib/public/BackgroundJob/TimedJob.php

@@ -32,6 +32,15 @@ abstract class TimedJob extends Job {
 		$this->interval = $seconds;
 	}
 
+	/**
+	 * Get the interval [seconds] for the job
+	 *
+	 * @since 32.0.0
+	 */
+	public function getInterval(): int {
+		return $this->interval;
+	}
+
 	/**
 	 * Whether the background job is time sensitive and needs to run soon after
 	 * the scheduled interval, of if it is okay to be delayed until a later time.

tests/lib/BackgroundJob/JobListTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -153,7 +156,11 @@ class JobListTest extends TestCase {
 		$this->assertFalse($this->instance->has($job, 10));
 	}
 
-	protected function createTempJob($class, $argument, $reservedTime = 0, $lastChecked = 0) {
+	protected function createTempJob($class,
+		$argument,
+		int $reservedTime = 0,
+		int $lastChecked = 0,
+		int $lastRun = 0): int {
 		if ($lastChecked === 0) {
 			$lastChecked = time();
 		}
@@ -163,11 +170,12 @@ class JobListTest extends TestCase {
 			->values([
 				'class' => $query->createNamedParameter($class),
 				'argument' => $query->createNamedParameter($argument),
-				'last_run' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
+				'last_run' => $query->createNamedParameter($lastRun, IQueryBuilder::PARAM_INT),
 				'last_checked' => $query->createNamedParameter($lastChecked, IQueryBuilder::PARAM_INT),
 				'reserved_at' => $query->createNamedParameter($reservedTime, IQueryBuilder::PARAM_INT),
 			]);
-		$query->execute();
+		$query->executeStatement();
+		return $query->getLastInsertId();
 	}
 
 	public function testGetNext(): void {
@@ -200,6 +208,21 @@ class JobListTest extends TestCase {
 		$this->assertEquals(2, $nextJob->getArgument());
 	}
 
+	public function testGetNextSkipTimed(): void {
+		$job = new TestTimedJobNew($this->timeFactory);
+		$jobId = $this->createTempJob(get_class($job), 1, 123456789, 12345, 123456789 - 5);
+		$this->timeFactory->expects(self::atLeastOnce())
+			->method('getTime')
+			->willReturn(123456789);
+
+		$nextJob = $this->instance->getNext();
+
+		self::assertNull($nextJob);
+		$job = $this->instance->getById($jobId);
+		self::assertInstanceOf(TestTimedJobNew::class, $job);
+		self::assertEquals(123456789 - 5, $job->getLastRun());
+	}
+
 	public function testGetNextSkipNonExisting(): void {
 		$job = new TestJob();
 		$this->createTempJob('\OC\Non\Existing\Class', 1, 0, 12345);

tests/lib/BackgroundJob/TestTimedJobNew.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace Test\BackgroundJob;
+
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\BackgroundJob\TimedJob;
+
+class TestTimedJobNew extends TimedJob {
+	public bool $ran = false;
+
+	public function __construct(ITimeFactory $timeFactory) {
+		parent::__construct($timeFactory);
+		$this->setInterval(10);
+	}
+
+	public function run($argument) {
+		$this->ran = true;
+	}
+}

tests/lib/BackgroundJob/TimedJobTest.php

@@ -8,20 +8,6 @@
 namespace Test\BackgroundJob;
 
 use OCP\AppFramework\Utility\ITimeFactory;
-use OCP\BackgroundJob\TimedJob;
-
-class TestTimedJobNew extends TimedJob {
-	public bool $ran = false;
-
-	public function __construct(ITimeFactory $timeFactory) {
-		parent::__construct($timeFactory);
-		$this->setInterval(10);
-	}
-
-	public function run($argument) {
-		$this->ran = true;
-	}
-}
 
 class TimedJobTest extends \Test\TestCase {
 	private DummyJobList $jobList;