From 52eb6d87263302a520e0d78a3ec3f9e86ecf477f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 21 Dec 2021 08:57:53 +0100 Subject: [PATCH 01/12] feat(bg-jobs): Allow calling cron.php with a background job class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- cron.php | 3 ++- lib/private/BackgroundJob/JobList.php | 10 +++++++--- tests/lib/BackgroundJob/DummyJobList.php | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/cron.php b/cron.php index e39c998ad5d..f6ca95fe226 100644 --- a/cron.php +++ b/cron.php @@ -160,7 +160,8 @@ try { $endTime = time() + 14 * 60; $executedJobs = []; - while ($job = $jobList->getNext($onlyTimeSensitive)) { + $jobClass = isset($argv[1]) ? $argv[1] : null; + while ($job = $jobList->getNext($onlyTimeSensitive, $jobClass)) { if (isset($executedJobs[$job->getId()])) { $jobList->unlockJob($job); break; diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 4e5d11604e6..3e8b5cf2988 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -214,7 +214,7 @@ class JobList implements IJobList { * Get the next job in the list * @return ?IJob the next job to run. Beware that this object may be a singleton and may be modified by the next call to buildJob. */ - public function getNext(bool $onlyTimeSensitive = false): ?IJob { + public function getNext(bool $onlyTimeSensitive = false, string $jobClass = null): ?IJob { $query = $this->connection->getQueryBuilder(); $query->select('*') ->from('jobs') @@ -227,6 +227,10 @@ class JobList implements IJobList { $query->andWhere($query->expr()->eq('time_sensitive', $query->createNamedParameter(IJob::TIME_SENSITIVE, IQueryBuilder::PARAM_INT))); } + if ($jobClass) { + $query->andWhere($query->expr()->eq('class', $query->createNamedParameter($jobClass))); + } + $result = $query->executeQuery(); $row = $result->fetch(); $result->closeCursor(); @@ -261,7 +265,7 @@ class JobList implements IJobList { if ($count === 0) { // Background job already executed elsewhere, try again. - return $this->getNext($onlyTimeSensitive); + return $this->getNext($onlyTimeSensitive, $jobClass); } if ($job === null) { @@ -274,7 +278,7 @@ class JobList implements IJobList { $reset->executeStatement(); // Background job from disabled app, try again. - return $this->getNext($onlyTimeSensitive); + return $this->getNext($onlyTimeSensitive, $jobClass); } return $job; diff --git a/tests/lib/BackgroundJob/DummyJobList.php b/tests/lib/BackgroundJob/DummyJobList.php index 64c0cf8038e..d480b93cc4a 100644 --- a/tests/lib/BackgroundJob/DummyJobList.php +++ b/tests/lib/BackgroundJob/DummyJobList.php @@ -100,7 +100,7 @@ class DummyJobList extends \OC\BackgroundJob\JobList { /** * get the next job in the list */ - public function getNext(bool $onlyTimeSensitive = false): ?IJob { + public function getNext(bool $onlyTimeSensitive = false, string $jobClass = null): ?IJob { if (count($this->jobs) > 0) { if ($this->last < (count($this->jobs) - 1)) { $i = $this->last + 1; From 8400bfee019f310494a345aa35643a44137a33cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 14 Jul 2022 14:50:12 +0200 Subject: [PATCH 02/12] feat(bg-jobs): Add background worker occ command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- core/Command/Background/Worker.php | 206 ++++++++++++++++++++++++++ core/register_command.php | 7 + lib/private/BackgroundJob/JobList.php | 19 +++ 3 files changed, 232 insertions(+) create mode 100644 core/Command/Background/Worker.php diff --git a/core/Command/Background/Worker.php b/core/Command/Background/Worker.php new file mode 100644 index 00000000000..61f8fbc495e --- /dev/null +++ b/core/Command/Background/Worker.php @@ -0,0 +1,206 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Core\Command\Background; + +use OCP\BackgroundJob\IJob; +use OCP\BackgroundJob\IJobList; +use Psr\Log\LoggerInterface; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +class Worker extends Command { + protected IJobList $jobList; + protected LoggerInterface $logger; + + const DEFAULT_INTERVAL = 5; + + public function __construct(IJobList $jobList, + LoggerInterface $logger) { + parent::__construct(); + $this->jobList = $jobList; + $this->logger = $logger; + } + + protected function configure(): void { + $this + ->setName('background-job:worker') + ->setDescription('Run a background job worker') + ->addArgument( + 'job-class', + InputArgument::OPTIONAL, + 'The class of the job in the database' + ) + ->addOption( + 'once', + null, + InputOption::VALUE_NONE, + 'Only execute the worker once (as a regular cron execution would do it)' + ) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output): int { + $jobClass = $input->getArgument('job-class'); + + $executedJobs = []; + + $ended = false; + pcntl_signal(SIGINT, function () use (&$ended, $output, $executedJobs) { + $output->writeln('SIGINT'); + if ($ended) { + foreach ($executedJobs as $id => $time) { + unset($executedJobs[$id]); + $job = $this->jobList->getById($id); + $this->jobList->unlockJob($job); + } + $output->writeln('Killed'); + exit(1); + } + $ended = true; + $output->writeln('Waiting for job to finish. Press Ctrl-C again to kill, but this may have unexpected side effects.'); + }); + + while (true) { + if ($ended) { + break; + } + $count = 0; + $total = 0; + foreach($this->jobList->countByClass() as $row) { + if ((int)$row['count'] === 1) { + $count++; + } else { + $output->writeln($row['class'] . " " . $row['count']); + } + $total += $row['count']; + } + $output->writeln("Other jobs " . $count); + $output->writeln("Total jobs " . $count); + + + + foreach ($executedJobs as $id => $time) { + if ($time < time() - self::DEFAULT_INTERVAL) { + unset($executedJobs[$id]); + $job = $this->jobList->getById($id); + $this->jobList->unlockJob($job); + } + } + + $job = $this->jobList->getNext(false, $jobClass); + if (!$job) { + $output->writeln("Waiting for new jobs to be queued"); + sleep(1); + continue; + } + + + if (isset($executedJobs[$job->getId()])) { + continue; + } + + $output->writeln("- Running job " . get_class($job) . " " . $job->getId()); + + if ($output->isVerbose()) { + $this->printJobInfo($job->getId(), $job, $output); + } + + $job->execute($this->jobList, \OC::$server->getLogger()); + + // clean up after unclean jobs + \OC_Util::tearDownFS(); + \OC::$server->getTempManager()->clean(); + + $this->jobList->setLastJob($job); + $executedJobs[$job->getId()] = time(); + unset($job); + + if ($input->getOption('once')) { + break; + } + } + + foreach ($executedJobs as $id => $time) { + unset($executedJobs[$id]); + $job = $this->jobList->getById($id); + $this->jobList->unlockJob($job); + } + + return 0; + } + + protected function printJobInfo(int $jobId, IJob $job, OutputInterface$output): void { + $row = $this->jobList->getDetailsById($jobId); + + $lastRun = new \DateTime(); + $lastRun->setTimestamp((int) $row['last_run']); + $lastChecked = new \DateTime(); + $lastChecked->setTimestamp((int) $row['last_checked']); + $reservedAt = new \DateTime(); + $reservedAt->setTimestamp((int) $row['reserved_at']); + + $output->writeln('Job class: ' . get_class($job)); + $output->writeln('Arguments: ' . json_encode($job->getArgument())); + + $isTimedJob = $job instanceof \OC\BackgroundJob\TimedJob || $job instanceof \OCP\BackgroundJob\TimedJob; + if ($isTimedJob) { + $output->writeln('Type: timed'); + } elseif ($job instanceof \OC\BackgroundJob\QueuedJob || $job instanceof \OCP\BackgroundJob\QueuedJob) { + $output->writeln('Type: queued'); + } else { + $output->writeln('Type: job'); + } + + $output->writeln(''); + $output->writeln('Last checked: ' . $lastChecked->format(\DateTimeInterface::ATOM)); + if ((int) $row['reserved_at'] === 0) { + $output->writeln('Reserved at: -'); + } else { + $output->writeln('Reserved at: ' . $reservedAt->format(\DateTimeInterface::ATOM) . ''); + } + $output->writeln('Last executed: ' . $lastRun->format(\DateTimeInterface::ATOM)); + $output->writeln('Last duration: ' . $row['execution_duration']); + + if ($isTimedJob) { + $reflection = new \ReflectionClass($job); + $intervalProperty = $reflection->getProperty('interval'); + $intervalProperty->setAccessible(true); + $interval = $intervalProperty->getValue($job); + + $nextRun = new \DateTime(); + $nextRun->setTimestamp($row['last_run'] + $interval); + + if ($nextRun > new \DateTime()) { + $output->writeln('Next execution: ' . $nextRun->format(\DateTimeInterface::ATOM) . ''); + } else { + $output->writeln('Next execution: ' . $nextRun->format(\DateTimeInterface::ATOM) . ''); + } + } + } +} diff --git a/core/register_command.php b/core/register_command.php index 97b75f17625..96a821b6f8c 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -50,6 +50,10 @@ declare(strict_types=1); * along with this program. If not, see * */ +use OC\Core\Command; +use OCP\IConfig; +use OCP\Server; +use Stecman\Component\Symfony\Console\BashCompletion\CompletionCommand; use OC\Core\Command; use OCP\IConfig; @@ -81,6 +85,8 @@ if ($config->getSystemValueBool('installed', false)) { $application->add(Server::get(Command\TwoFactorAuth\Enable::class)); $application->add(Server::get(Command\TwoFactorAuth\Disable::class)); $application->add(Server::get(Command\TwoFactorAuth\State::class)); + $application->add(Server::get(Command\TwoFactorAuth\State::class)); + $application->add(Server::get(Command\Background\Cron::class)); $application->add(Server::get(Command\Background\WebCron::class)); @@ -88,6 +94,7 @@ if ($config->getSystemValueBool('installed', false)) { $application->add(Server::get(Command\Background\Job::class)); $application->add(Server::get(Command\Background\ListCommand::class)); $application->add(Server::get(Command\Background\Delete::class)); + $application->add(Server::get(Command\Background\Worker::class)); $application->add(Server::get(Command\Broadcast\Test::class)); diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 3e8b5cf2988..c7291d3ce04 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -436,4 +436,23 @@ class JobList implements IJobList { return false; } } + + public function countByClass(): array { + $query = $this->connection->getQueryBuilder(); + $query->select('class') + ->selectAlias($query->func()->count('id'), 'count') + ->from('jobs') + ->orderBy('count') + ->groupBy('class'); + + $result = $query->executeQuery(); + + $jobs = []; + while ($row = $result->fetch()) { + $jobs[] = $row; + } + + return $jobs; + + } } From d69b8ecf953f752efae3bb1b4f1ff3034d5cdcb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 1 Sep 2022 12:13:05 +0200 Subject: [PATCH 03/12] fix(bg-jobs): Fix running once when no job was scheduled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- core/Command/Background/JobBase.php | 93 ++++++++++++ core/Command/Background/JobWorker.php | 173 +++++++++++++++++++++ core/Command/Background/Worker.php | 206 -------------------------- core/register_command.php | 2 +- 4 files changed, 267 insertions(+), 207 deletions(-) create mode 100644 core/Command/Background/JobBase.php create mode 100644 core/Command/Background/JobWorker.php delete mode 100644 core/Command/Background/Worker.php diff --git a/core/Command/Background/JobBase.php b/core/Command/Background/JobBase.php new file mode 100644 index 00000000000..fe2880c0988 --- /dev/null +++ b/core/Command/Background/JobBase.php @@ -0,0 +1,93 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + + +namespace OC\Core\Command\Background; + +use OCP\BackgroundJob\IJob; +use OCP\BackgroundJob\IJobList; +use Psr\Log\LoggerInterface; +use Symfony\Component\Console\Output\OutputInterface; + +abstract class JobBase extends \OC\Core\Command\Base { + protected IJobList $jobList; + protected LoggerInterface $logger; + + public function __construct(IJobList $jobList, + LoggerInterface $logger) { + parent::__construct(); + $this->jobList = $jobList; + $this->logger = $logger; + } + + protected function printJobInfo(int $jobId, IJob $job, OutputInterface $output): void { + $row = $this->jobList->getDetailsById($jobId); + + $lastRun = new \DateTime(); + $lastRun->setTimestamp((int) $row['last_run']); + $lastChecked = new \DateTime(); + $lastChecked->setTimestamp((int) $row['last_checked']); + $reservedAt = new \DateTime(); + $reservedAt->setTimestamp((int) $row['reserved_at']); + + $output->writeln('Job class: ' . get_class($job)); + $output->writeln('Arguments: ' . json_encode($job->getArgument())); + + $isTimedJob = $job instanceof \OC\BackgroundJob\TimedJob || $job instanceof \OCP\BackgroundJob\TimedJob; + if ($isTimedJob) { + $output->writeln('Type: timed'); + } elseif ($job instanceof \OC\BackgroundJob\QueuedJob || $job instanceof \OCP\BackgroundJob\QueuedJob) { + $output->writeln('Type: queued'); + } else { + $output->writeln('Type: job'); + } + + $output->writeln(''); + $output->writeln('Last checked: ' . $lastChecked->format(\DateTimeInterface::ATOM)); + if ((int) $row['reserved_at'] === 0) { + $output->writeln('Reserved at: -'); + } else { + $output->writeln('Reserved at: ' . $reservedAt->format(\DateTimeInterface::ATOM) . ''); + } + $output->writeln('Last executed: ' . $lastRun->format(\DateTimeInterface::ATOM)); + $output->writeln('Last duration: ' . $row['execution_duration']); + + if ($isTimedJob) { + $reflection = new \ReflectionClass($job); + $intervalProperty = $reflection->getProperty('interval'); + $intervalProperty->setAccessible(true); + $interval = $intervalProperty->getValue($job); + + $nextRun = new \DateTime(); + $nextRun->setTimestamp($row['last_run'] + $interval); + + if ($nextRun > new \DateTime()) { + $output->writeln('Next execution: ' . $nextRun->format(\DateTimeInterface::ATOM) . ''); + } else { + $output->writeln('Next execution: ' . $nextRun->format(\DateTimeInterface::ATOM) . ''); + } + } + } +} diff --git a/core/Command/Background/JobWorker.php b/core/Command/Background/JobWorker.php new file mode 100644 index 00000000000..2ca4af73474 --- /dev/null +++ b/core/Command/Background/JobWorker.php @@ -0,0 +1,173 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Core\Command\Background; + +use OC\Core\Command\InterruptedException; +use OCP\BackgroundJob\IJobList; +use Psr\Log\LoggerInterface; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +class JobWorker extends JobBase { + private array $executedJobs = []; + + public function __construct(IJobList $jobList, + LoggerInterface $logger) { + parent::__construct($jobList, $logger); + } + + protected function configure(): void { + parent::configure(); + + $this + ->setName('background-job:worker') + ->setDescription('Run a background job worker') + ->addArgument( + 'job-class', + InputArgument::OPTIONAL, + 'The class of the job in the database' + ) + ->addOption( + 'once', + null, + InputOption::VALUE_NONE, + 'Only execute the worker once (as a regular cron execution would do it)' + ) + ->addOption( + 'interval', + 'i', + InputOption::VALUE_OPTIONAL, + 'Interval in seconds in which the worker should repeat already processed jobs (set to 0 for no repeat)', + 5 + ) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output): int { + $jobClass = $input->getArgument('job-class'); + + if ($jobClass && !class_exists($jobClass)) { + $output->writeln('Invalid job class'); + return 1; + } + + while (true) { + // Handle canceling of the process + try { + $this->abortIfInterrupted(); + } catch (InterruptedException $e) { + $output->writeln('Cleaning up before quitting. Press Ctrl-C again to kill, but this may have unexpected side effects.'); + $this->unlockExecuted(); + $output->writeln('Background job worker stopped'); + break; + } + + $this->printSummary($input, $output); + + $interval = (int)($input->getOption('interval') ?? 5); + + // Unlock jobs that should be executed again after the interval + // Alternative could be to set last_checked to interval in the future to avoid the extra locks + foreach ($this->executedJobs as $id => $time) { + if ($time <= time() - $interval) { + unset($this->executedJobs[$id]); + $job = $this->jobList->getById($id); + if ($job !== null) { + $this->jobList->unlockJob($job); + } + } + } + + usleep(50000); + $job = $this->jobList->getNext(false, $jobClass); + if (!$job) { + if ($input->getOption('once') === true || $interval === 0) { + break; + } + + $output->writeln("Waiting for new jobs to be queued", OutputInterface::VERBOSITY_VERBOSE); + // Re-check interval for new jobs + sleep(1); + continue; + } + + + if (isset($this->executedJobs[$job->getId()]) && ($this->executedJobs[$job->getId()] + $interval > time())) { + $output->writeln("Job already executed within timeframe " . get_class($job) . " " . $job->getId() . '', OutputInterface::VERBOSITY_VERBOSE); + continue; + } + + $output->writeln("Running job " . get_class($job) . " with ID " . $job->getId()); + + if ($output->isVerbose()) { + $this->printJobInfo($job->getId(), $job, $output); + } + + $job->execute($this->jobList, \OC::$server->getLogger()); + + // clean up after unclean jobs + \OC_Util::tearDownFS(); + \OC::$server->getTempManager()->clean(); + + $this->jobList->setLastJob($job); + $this->jobList->unlockJob($job); + $this->executedJobs[$job->getId()] = time(); + + if ($input->getOption('once') === true) { + break; + } + } + + $this->unlockExecuted(); + + return 0; + } + + private function printSummary(InputInterface $input, OutputInterface $output): void { + if (!$output->isVeryVerbose()) { + return; + } + $output->writeln("Summary"); + + $counts = []; + foreach ($this->jobList->countByClass() as $row) { + $counts[] = $row; + } + $this->writeTableInOutputFormat($input, $output, $counts); + } + + private function unlockExecuted() { + foreach ($this->executedJobs as $id => $time) { + unset($this->executedJobs[$id]); + $job = $this->jobList->getById($id); + if ($job !== null) { + $this->jobList->unlockJob($job); + } + } + } +} diff --git a/core/Command/Background/Worker.php b/core/Command/Background/Worker.php deleted file mode 100644 index 61f8fbc495e..00000000000 --- a/core/Command/Background/Worker.php +++ /dev/null @@ -1,206 +0,0 @@ - - * - * @author Joas Schilling - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * - */ - -namespace OC\Core\Command\Background; - -use OCP\BackgroundJob\IJob; -use OCP\BackgroundJob\IJobList; -use Psr\Log\LoggerInterface; -use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputArgument; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; -use Symfony\Component\Console\Output\OutputInterface; - -class Worker extends Command { - protected IJobList $jobList; - protected LoggerInterface $logger; - - const DEFAULT_INTERVAL = 5; - - public function __construct(IJobList $jobList, - LoggerInterface $logger) { - parent::__construct(); - $this->jobList = $jobList; - $this->logger = $logger; - } - - protected function configure(): void { - $this - ->setName('background-job:worker') - ->setDescription('Run a background job worker') - ->addArgument( - 'job-class', - InputArgument::OPTIONAL, - 'The class of the job in the database' - ) - ->addOption( - 'once', - null, - InputOption::VALUE_NONE, - 'Only execute the worker once (as a regular cron execution would do it)' - ) - ; - } - - protected function execute(InputInterface $input, OutputInterface $output): int { - $jobClass = $input->getArgument('job-class'); - - $executedJobs = []; - - $ended = false; - pcntl_signal(SIGINT, function () use (&$ended, $output, $executedJobs) { - $output->writeln('SIGINT'); - if ($ended) { - foreach ($executedJobs as $id => $time) { - unset($executedJobs[$id]); - $job = $this->jobList->getById($id); - $this->jobList->unlockJob($job); - } - $output->writeln('Killed'); - exit(1); - } - $ended = true; - $output->writeln('Waiting for job to finish. Press Ctrl-C again to kill, but this may have unexpected side effects.'); - }); - - while (true) { - if ($ended) { - break; - } - $count = 0; - $total = 0; - foreach($this->jobList->countByClass() as $row) { - if ((int)$row['count'] === 1) { - $count++; - } else { - $output->writeln($row['class'] . " " . $row['count']); - } - $total += $row['count']; - } - $output->writeln("Other jobs " . $count); - $output->writeln("Total jobs " . $count); - - - - foreach ($executedJobs as $id => $time) { - if ($time < time() - self::DEFAULT_INTERVAL) { - unset($executedJobs[$id]); - $job = $this->jobList->getById($id); - $this->jobList->unlockJob($job); - } - } - - $job = $this->jobList->getNext(false, $jobClass); - if (!$job) { - $output->writeln("Waiting for new jobs to be queued"); - sleep(1); - continue; - } - - - if (isset($executedJobs[$job->getId()])) { - continue; - } - - $output->writeln("- Running job " . get_class($job) . " " . $job->getId()); - - if ($output->isVerbose()) { - $this->printJobInfo($job->getId(), $job, $output); - } - - $job->execute($this->jobList, \OC::$server->getLogger()); - - // clean up after unclean jobs - \OC_Util::tearDownFS(); - \OC::$server->getTempManager()->clean(); - - $this->jobList->setLastJob($job); - $executedJobs[$job->getId()] = time(); - unset($job); - - if ($input->getOption('once')) { - break; - } - } - - foreach ($executedJobs as $id => $time) { - unset($executedJobs[$id]); - $job = $this->jobList->getById($id); - $this->jobList->unlockJob($job); - } - - return 0; - } - - protected function printJobInfo(int $jobId, IJob $job, OutputInterface$output): void { - $row = $this->jobList->getDetailsById($jobId); - - $lastRun = new \DateTime(); - $lastRun->setTimestamp((int) $row['last_run']); - $lastChecked = new \DateTime(); - $lastChecked->setTimestamp((int) $row['last_checked']); - $reservedAt = new \DateTime(); - $reservedAt->setTimestamp((int) $row['reserved_at']); - - $output->writeln('Job class: ' . get_class($job)); - $output->writeln('Arguments: ' . json_encode($job->getArgument())); - - $isTimedJob = $job instanceof \OC\BackgroundJob\TimedJob || $job instanceof \OCP\BackgroundJob\TimedJob; - if ($isTimedJob) { - $output->writeln('Type: timed'); - } elseif ($job instanceof \OC\BackgroundJob\QueuedJob || $job instanceof \OCP\BackgroundJob\QueuedJob) { - $output->writeln('Type: queued'); - } else { - $output->writeln('Type: job'); - } - - $output->writeln(''); - $output->writeln('Last checked: ' . $lastChecked->format(\DateTimeInterface::ATOM)); - if ((int) $row['reserved_at'] === 0) { - $output->writeln('Reserved at: -'); - } else { - $output->writeln('Reserved at: ' . $reservedAt->format(\DateTimeInterface::ATOM) . ''); - } - $output->writeln('Last executed: ' . $lastRun->format(\DateTimeInterface::ATOM)); - $output->writeln('Last duration: ' . $row['execution_duration']); - - if ($isTimedJob) { - $reflection = new \ReflectionClass($job); - $intervalProperty = $reflection->getProperty('interval'); - $intervalProperty->setAccessible(true); - $interval = $intervalProperty->getValue($job); - - $nextRun = new \DateTime(); - $nextRun->setTimestamp($row['last_run'] + $interval); - - if ($nextRun > new \DateTime()) { - $output->writeln('Next execution: ' . $nextRun->format(\DateTimeInterface::ATOM) . ''); - } else { - $output->writeln('Next execution: ' . $nextRun->format(\DateTimeInterface::ATOM) . ''); - } - } - } -} diff --git a/core/register_command.php b/core/register_command.php index 96a821b6f8c..f3ae8efa300 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -94,7 +94,7 @@ if ($config->getSystemValueBool('installed', false)) { $application->add(Server::get(Command\Background\Job::class)); $application->add(Server::get(Command\Background\ListCommand::class)); $application->add(Server::get(Command\Background\Delete::class)); - $application->add(Server::get(Command\Background\Worker::class)); + $application->add(Server::get(Command\Background\JobWorker::class)); $application->add(Server::get(Command\Broadcast\Test::class)); From a3d8632fbe5e255a30343c1950d9ec8a1849f5af Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 20 Dec 2023 14:12:27 +0100 Subject: [PATCH 04/12] fix(bg-jobs): fix minor issues Signed-off-by: Marcel Klehr --- core/Command/Background/JobWorker.php | 9 ++------- lib/private/BackgroundJob/JobList.php | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/core/Command/Background/JobWorker.php b/core/Command/Background/JobWorker.php index 2ca4af73474..c0122e84694 100644 --- a/core/Command/Background/JobWorker.php +++ b/core/Command/Background/JobWorker.php @@ -36,11 +36,6 @@ use Symfony\Component\Console\Output\OutputInterface; class JobWorker extends JobBase { private array $executedJobs = []; - public function __construct(IJobList $jobList, - LoggerInterface $logger) { - parent::__construct($jobList, $logger); - } - protected function configure(): void { parent::configure(); @@ -76,6 +71,8 @@ class JobWorker extends JobBase { return 1; } + $interval = (int)($input->getOption('interval') ?? 5); + while (true) { // Handle canceling of the process try { @@ -89,8 +86,6 @@ class JobWorker extends JobBase { $this->printSummary($input, $output); - $interval = (int)($input->getOption('interval') ?? 5); - // Unlock jobs that should be executed again after the interval // Alternative could be to set last_checked to interval in the future to avoid the extra locks foreach ($this->executedJobs as $id => $time) { diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index c7291d3ce04..36f46d7bad7 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -448,7 +448,7 @@ class JobList implements IJobList { $result = $query->executeQuery(); $jobs = []; - while ($row = $result->fetch()) { + while (($row = $result->fetch()) !== false) { $jobs[] = $row; } From 993398b88a2dbb148b008b25abba3f6761dd0668 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 20 Dec 2023 14:16:16 +0100 Subject: [PATCH 05/12] fix(bg-jobs): Remove interval bookkeeping Signed-off-by: Marcel Klehr --- core/Command/Background/JobWorker.php | 37 +-------------------------- 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/core/Command/Background/JobWorker.php b/core/Command/Background/JobWorker.php index c0122e84694..aa991c96137 100644 --- a/core/Command/Background/JobWorker.php +++ b/core/Command/Background/JobWorker.php @@ -71,37 +71,21 @@ class JobWorker extends JobBase { return 1; } - $interval = (int)($input->getOption('interval') ?? 5); - while (true) { // Handle canceling of the process try { $this->abortIfInterrupted(); } catch (InterruptedException $e) { - $output->writeln('Cleaning up before quitting. Press Ctrl-C again to kill, but this may have unexpected side effects.'); - $this->unlockExecuted(); $output->writeln('Background job worker stopped'); break; } $this->printSummary($input, $output); - // Unlock jobs that should be executed again after the interval - // Alternative could be to set last_checked to interval in the future to avoid the extra locks - foreach ($this->executedJobs as $id => $time) { - if ($time <= time() - $interval) { - unset($this->executedJobs[$id]); - $job = $this->jobList->getById($id); - if ($job !== null) { - $this->jobList->unlockJob($job); - } - } - } - usleep(50000); $job = $this->jobList->getNext(false, $jobClass); if (!$job) { - if ($input->getOption('once') === true || $interval === 0) { + if ($input->getOption('once') === true) { break; } @@ -111,12 +95,6 @@ class JobWorker extends JobBase { continue; } - - if (isset($this->executedJobs[$job->getId()]) && ($this->executedJobs[$job->getId()] + $interval > time())) { - $output->writeln("Job already executed within timeframe " . get_class($job) . " " . $job->getId() . '', OutputInterface::VERBOSITY_VERBOSE); - continue; - } - $output->writeln("Running job " . get_class($job) . " with ID " . $job->getId()); if ($output->isVerbose()) { @@ -131,15 +109,12 @@ class JobWorker extends JobBase { $this->jobList->setLastJob($job); $this->jobList->unlockJob($job); - $this->executedJobs[$job->getId()] = time(); if ($input->getOption('once') === true) { break; } } - $this->unlockExecuted(); - return 0; } @@ -155,14 +130,4 @@ class JobWorker extends JobBase { } $this->writeTableInOutputFormat($input, $output, $counts); } - - private function unlockExecuted() { - foreach ($this->executedJobs as $id => $time) { - unset($this->executedJobs[$id]); - $job = $this->jobList->getById($id); - if ($job !== null) { - $this->jobList->unlockJob($job); - } - } - } } From 352d79deeeb12e4544d1b7e9f791a3c11afc12e4 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 20 Dec 2023 14:29:44 +0100 Subject: [PATCH 06/12] fix(bg-jobs): fix psalm issues Signed-off-by: Marcel Klehr --- core/Command/Background/JobBase.php | 10 +++++++--- core/Command/Background/JobWorker.php | 5 +++-- core/register_command.php | 5 ----- lib/private/BackgroundJob/JobList.php | 5 ++++- lib/public/BackgroundJob/IJobList.php | 12 ++++++++++-- 5 files changed, 24 insertions(+), 13 deletions(-) diff --git a/core/Command/Background/JobBase.php b/core/Command/Background/JobBase.php index fe2880c0988..4385783a7ee 100644 --- a/core/Command/Background/JobBase.php +++ b/core/Command/Background/JobBase.php @@ -45,6 +45,10 @@ abstract class JobBase extends \OC\Core\Command\Base { protected function printJobInfo(int $jobId, IJob $job, OutputInterface $output): void { $row = $this->jobList->getDetailsById($jobId); + if ($row === null) { + return; + } + $lastRun = new \DateTime(); $lastRun->setTimestamp((int) $row['last_run']); $lastChecked = new \DateTime(); @@ -55,10 +59,10 @@ abstract class JobBase extends \OC\Core\Command\Base { $output->writeln('Job class: ' . get_class($job)); $output->writeln('Arguments: ' . json_encode($job->getArgument())); - $isTimedJob = $job instanceof \OC\BackgroundJob\TimedJob || $job instanceof \OCP\BackgroundJob\TimedJob; + $isTimedJob = $job instanceof \OCP\BackgroundJob\TimedJob; if ($isTimedJob) { $output->writeln('Type: timed'); - } elseif ($job instanceof \OC\BackgroundJob\QueuedJob || $job instanceof \OCP\BackgroundJob\QueuedJob) { + } elseif ($job instanceof \OCP\BackgroundJob\QueuedJob) { $output->writeln('Type: queued'); } else { $output->writeln('Type: job'); @@ -81,7 +85,7 @@ abstract class JobBase extends \OC\Core\Command\Base { $interval = $intervalProperty->getValue($job); $nextRun = new \DateTime(); - $nextRun->setTimestamp($row['last_run'] + $interval); + $nextRun->setTimestamp((int)$row['last_run'] + $interval); if ($nextRun > new \DateTime()) { $output->writeln('Next execution: ' . $nextRun->format(\DateTimeInterface::ATOM) . ''); diff --git a/core/Command/Background/JobWorker.php b/core/Command/Background/JobWorker.php index aa991c96137..8fb7ed139f6 100644 --- a/core/Command/Background/JobWorker.php +++ b/core/Command/Background/JobWorker.php @@ -27,6 +27,7 @@ namespace OC\Core\Command\Background; use OC\Core\Command\InterruptedException; use OCP\BackgroundJob\IJobList; +use OCP\ITempManager; use Psr\Log\LoggerInterface; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -101,11 +102,11 @@ class JobWorker extends JobBase { $this->printJobInfo($job->getId(), $job, $output); } - $job->execute($this->jobList, \OC::$server->getLogger()); + $job->start($this->jobList); // clean up after unclean jobs \OC_Util::tearDownFS(); - \OC::$server->getTempManager()->clean(); + \OC::$server->get(ITempManager::class)->clean(); $this->jobList->setLastJob($job); $this->jobList->unlockJob($job); diff --git a/core/register_command.php b/core/register_command.php index f3ae8efa300..88ece906846 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -55,11 +55,6 @@ use OCP\IConfig; use OCP\Server; use Stecman\Component\Symfony\Console\BashCompletion\CompletionCommand; -use OC\Core\Command; -use OCP\IConfig; -use OCP\Server; -use Stecman\Component\Symfony\Console\BashCompletion\CompletionCommand; - $application->add(new CompletionCommand()); $application->add(Server::get(Command\Status::class)); $application->add(Server::get(Command\Check::class)); diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 36f46d7bad7..9d54fa45bfc 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -448,11 +448,14 @@ class JobList implements IJobList { $result = $query->executeQuery(); $jobs = []; + while (($row = $result->fetch()) !== false) { + /** + * @var array{count:int, class:class-string} $row + */ $jobs[] = $row; } return $jobs; - } } diff --git a/lib/public/BackgroundJob/IJobList.php b/lib/public/BackgroundJob/IJobList.php index 07b5ebcf48b..e2e9eca9415 100644 --- a/lib/public/BackgroundJob/IJobList.php +++ b/lib/public/BackgroundJob/IJobList.php @@ -110,9 +110,9 @@ interface IJobList { /** * get the next job in the list * - * @since 7.0.0 - In 24.0.0 parameter $onlyTimeSensitive got added + * @since 7.0.0 - In 24.0.0 parameter $onlyTimeSensitive got added; In 29.0.0 parameter $jobClass got added */ - public function getNext(bool $onlyTimeSensitive = false): ?IJob; + public function getNext(bool $onlyTimeSensitive = false, string $jobClass = null): ?IJob; /** * @since 7.0.0 @@ -168,4 +168,12 @@ interface IJobList { * @since 27.0.0 */ public function hasReservedJob(?string $className): bool; + + /** + * Returns a count of jobs per Job class + * + * @return list + * @since 29.0.0 + */ + public function countByClass(): array; } From 9a3b341932b64e811fb043204c3f47cb8c62deb3 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 21 Dec 2023 12:11:08 +0100 Subject: [PATCH 07/12] fix(bg-jobs): cs:fix Signed-off-by: Marcel Klehr --- core/Command/Background/JobWorker.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/Command/Background/JobWorker.php b/core/Command/Background/JobWorker.php index 8fb7ed139f6..995bb1a3fd7 100644 --- a/core/Command/Background/JobWorker.php +++ b/core/Command/Background/JobWorker.php @@ -26,17 +26,13 @@ declare(strict_types=1); namespace OC\Core\Command\Background; use OC\Core\Command\InterruptedException; -use OCP\BackgroundJob\IJobList; use OCP\ITempManager; -use Psr\Log\LoggerInterface; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class JobWorker extends JobBase { - private array $executedJobs = []; - protected function configure(): void { parent::configure(); From 9814bffb7799cac8050c8c72b93fc90e7cac5e9a Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 8 Apr 2024 13:04:14 +0200 Subject: [PATCH 08/12] chore(bg-jobs): add -h help option to cron.php Signed-off-by: Julien Veyssier --- cron.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cron.php b/cron.php index f6ca95fe226..dc2d821fc57 100644 --- a/cron.php +++ b/cron.php @@ -57,6 +57,21 @@ use Psr\Log\LoggerInterface; try { require_once __DIR__ . '/lib/base.php'; + if ($argv[1] === '-h' || $argv[1] === '--help') { + echo 'Description: + Run the background job routine + +Usage: + php -f cron.php -- [-h] [] + +Arguments: + job-class Optional job class to only run those jobs + +Options: + -h, --help Display this help message' . PHP_EOL; + exit(0); + } + if (Util::needUpgrade()) { Server::get(LoggerInterface::class)->debug('Update required, skipping cron', ['app' => 'cron']); exit; @@ -160,6 +175,7 @@ try { $endTime = time() + 14 * 60; $executedJobs = []; + // a specific job class can optionally be given as first argument $jobClass = isset($argv[1]) ? $argv[1] : null; while ($job = $jobList->getNext($onlyTimeSensitive, $jobClass)) { if (isset($executedJobs[$job->getId()])) { From a5f244a58b6cfcc4feb8d8bdf33b40f983c342cb Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 8 Apr 2024 13:21:32 +0200 Subject: [PATCH 09/12] chore(bg-jobs): more output in verbose mode in the bg job worker Signed-off-by: Julien Veyssier --- core/Command/Background/JobWorker.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/core/Command/Background/JobWorker.php b/core/Command/Background/JobWorker.php index 995bb1a3fd7..d6aab0e83bc 100644 --- a/core/Command/Background/JobWorker.php +++ b/core/Command/Background/JobWorker.php @@ -83,16 +83,18 @@ class JobWorker extends JobBase { $job = $this->jobList->getNext(false, $jobClass); if (!$job) { if ($input->getOption('once') === true) { + $output->writeln('No job of class ' . $jobClass . ' is currently queued', OutputInterface::VERBOSITY_VERBOSE); + $output->writeln('Exiting...', OutputInterface::VERBOSITY_VERBOSE); break; } - $output->writeln("Waiting for new jobs to be queued", OutputInterface::VERBOSITY_VERBOSE); + $output->writeln('Waiting for new jobs to be queued', OutputInterface::VERBOSITY_VERBOSE); // Re-check interval for new jobs sleep(1); continue; } - $output->writeln("Running job " . get_class($job) . " with ID " . $job->getId()); + $output->writeln('Running job ' . get_class($job) . ' with ID ' . $job->getId()); if ($output->isVerbose()) { $this->printJobInfo($job->getId(), $job, $output); @@ -100,6 +102,8 @@ class JobWorker extends JobBase { $job->start($this->jobList); + $output->writeln('Job ' . $job->getId() . ' has finished', OutputInterface::VERBOSITY_VERBOSE); + // clean up after unclean jobs \OC_Util::tearDownFS(); \OC::$server->get(ITempManager::class)->clean(); @@ -119,7 +123,7 @@ class JobWorker extends JobBase { if (!$output->isVeryVerbose()) { return; } - $output->writeln("Summary"); + $output->writeln('Summary'); $counts = []; foreach ($this->jobList->countByClass() as $row) { From 1acc57b5c03b80ad5b6b5df7aff9df0fef83f14f Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 8 Apr 2024 17:25:51 +0200 Subject: [PATCH 10/12] feat(bg-jobs): allow setting a job class list instead of a single class in cron.php and the job worker occ command Signed-off-by: Julien Veyssier --- core/Command/Background/JobBase.php | 6 ++-- core/Command/Background/JobWorker.php | 36 ++++++++++++++++----- cron.php | 18 ++++++++--- lib/composer/composer/autoload_classmap.php | 2 ++ lib/composer/composer/autoload_static.php | 2 ++ lib/private/BackgroundJob/JobList.php | 14 +++++--- lib/public/BackgroundJob/IJobList.php | 7 ++-- tests/lib/BackgroundJob/DummyJobList.php | 2 +- 8 files changed, 64 insertions(+), 23 deletions(-) diff --git a/core/Command/Background/JobBase.php b/core/Command/Background/JobBase.php index 4385783a7ee..94c9e1611ea 100644 --- a/core/Command/Background/JobBase.php +++ b/core/Command/Background/JobBase.php @@ -35,8 +35,10 @@ abstract class JobBase extends \OC\Core\Command\Base { protected IJobList $jobList; protected LoggerInterface $logger; - public function __construct(IJobList $jobList, - LoggerInterface $logger) { + public function __construct( + IJobList $jobList, + LoggerInterface $logger + ) { parent::__construct(); $this->jobList = $jobList; $this->logger = $logger; diff --git a/core/Command/Background/JobWorker.php b/core/Command/Background/JobWorker.php index d6aab0e83bc..8b7bb6b6fba 100644 --- a/core/Command/Background/JobWorker.php +++ b/core/Command/Background/JobWorker.php @@ -40,9 +40,9 @@ class JobWorker extends JobBase { ->setName('background-job:worker') ->setDescription('Run a background job worker') ->addArgument( - 'job-class', + 'job-classes', InputArgument::OPTIONAL, - 'The class of the job in the database' + 'The classes of the jobs to look for in the database, comma-separated' ) ->addOption( 'once', @@ -61,11 +61,31 @@ class JobWorker extends JobBase { } protected function execute(InputInterface $input, OutputInterface $output): int { - $jobClass = $input->getArgument('job-class'); + $jobClassesString = $input->getArgument('job-classes'); + // only keep non-empty strings + $jobClasses = $jobClassesString === null + ? null + : array_filter( + explode(',', $jobClassesString), + static function (string $jobClass) { + return strlen($jobClass) > 0; + } + ); + + if ($jobClasses !== null) { + // no class + if (count($jobClasses) === 0) { + $output->writeln('Invalid job class list supplied'); + return 1; + } - if ($jobClass && !class_exists($jobClass)) { - $output->writeln('Invalid job class'); - return 1; + // at least one invalid class + foreach ($jobClasses as $jobClass) { + if (!class_exists($jobClass)) { + $output->writeln('Invalid job class: ' . $jobClass . ''); + return 1; + } + } } while (true) { @@ -80,10 +100,10 @@ class JobWorker extends JobBase { $this->printSummary($input, $output); usleep(50000); - $job = $this->jobList->getNext(false, $jobClass); + $job = $this->jobList->getNext(false, $jobClasses); if (!$job) { if ($input->getOption('once') === true) { - $output->writeln('No job of class ' . $jobClass . ' is currently queued', OutputInterface::VERBOSITY_VERBOSE); + $output->writeln('No job of classes ' . $jobClassesString . ' is currently queued', OutputInterface::VERBOSITY_VERBOSE); $output->writeln('Exiting...', OutputInterface::VERBOSITY_VERBOSE); break; } diff --git a/cron.php b/cron.php index dc2d821fc57..d0a8b770dfc 100644 --- a/cron.php +++ b/cron.php @@ -62,10 +62,10 @@ try { Run the background job routine Usage: - php -f cron.php -- [-h] [] + php -f cron.php -- [-h] [] Arguments: - job-class Optional job class to only run those jobs + job-classes Optional job class comma-separated list to only run those jobs Options: -h, --help Display this help message' . PHP_EOL; @@ -175,9 +175,17 @@ Options: $endTime = time() + 14 * 60; $executedJobs = []; - // a specific job class can optionally be given as first argument - $jobClass = isset($argv[1]) ? $argv[1] : null; - while ($job = $jobList->getNext($onlyTimeSensitive, $jobClass)) { + // a specific job class list can optionally be given as first argument + // only keep non-empty strings + $jobClasses = isset($argv[1]) + ? array_filter( + explode(',', $argv[1]), + static function (string $jobClass) { + return strlen($jobClass) > 0; + } + ) + : null; + while ($job = $jobList->getNext($onlyTimeSensitive, $jobClasses)) { if (isset($executedJobs[$job->getId()])) { $jobList->unlockJob($job); break; diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 7baad0e5169..2f93910197d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1056,6 +1056,8 @@ return array( 'OC\\Core\\Command\\Background\\Cron' => $baseDir . '/core/Command/Background/Cron.php', 'OC\\Core\\Command\\Background\\Delete' => $baseDir . '/core/Command/Background/Delete.php', 'OC\\Core\\Command\\Background\\Job' => $baseDir . '/core/Command/Background/Job.php', + 'OC\\Core\\Command\\Background\\JobBase' => $baseDir . '/core/Command/Background/JobBase.php', + 'OC\\Core\\Command\\Background\\JobWorker' => $baseDir . '/core/Command/Background/JobWorker.php', 'OC\\Core\\Command\\Background\\ListCommand' => $baseDir . '/core/Command/Background/ListCommand.php', 'OC\\Core\\Command\\Background\\WebCron' => $baseDir . '/core/Command/Background/WebCron.php', 'OC\\Core\\Command\\Base' => $baseDir . '/core/Command/Base.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index d96a1b2d1b5..f7b35e24d8f 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1089,6 +1089,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Command\\Background\\Cron' => __DIR__ . '/../../..' . '/core/Command/Background/Cron.php', 'OC\\Core\\Command\\Background\\Delete' => __DIR__ . '/../../..' . '/core/Command/Background/Delete.php', 'OC\\Core\\Command\\Background\\Job' => __DIR__ . '/../../..' . '/core/Command/Background/Job.php', + 'OC\\Core\\Command\\Background\\JobBase' => __DIR__ . '/../../..' . '/core/Command/Background/JobBase.php', + 'OC\\Core\\Command\\Background\\JobWorker' => __DIR__ . '/../../..' . '/core/Command/Background/JobWorker.php', 'OC\\Core\\Command\\Background\\ListCommand' => __DIR__ . '/../../..' . '/core/Command/Background/ListCommand.php', 'OC\\Core\\Command\\Background\\WebCron' => __DIR__ . '/../../..' . '/core/Command/Background/WebCron.php', 'OC\\Core\\Command\\Base' => __DIR__ . '/../../..' . '/core/Command/Base.php', diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 9d54fa45bfc..26bc198c6e0 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -214,7 +214,7 @@ class JobList implements IJobList { * Get the next job in the list * @return ?IJob the next job to run. Beware that this object may be a singleton and may be modified by the next call to buildJob. */ - public function getNext(bool $onlyTimeSensitive = false, string $jobClass = null): ?IJob { + public function getNext(bool $onlyTimeSensitive = false, ?array $jobClasses = null): ?IJob { $query = $this->connection->getQueryBuilder(); $query->select('*') ->from('jobs') @@ -227,8 +227,12 @@ class JobList implements IJobList { $query->andWhere($query->expr()->eq('time_sensitive', $query->createNamedParameter(IJob::TIME_SENSITIVE, IQueryBuilder::PARAM_INT))); } - if ($jobClass) { - $query->andWhere($query->expr()->eq('class', $query->createNamedParameter($jobClass))); + if ($jobClasses !== null && count($jobClasses) > 0) { + $orClasses = $query->expr()->orx(); + foreach ($jobClasses as $jobClass) { + $orClasses->add($query->expr()->eq('class', $query->createNamedParameter($jobClass, IQueryBuilder::PARAM_STR))); + } + $query->andWhere($orClasses); } $result = $query->executeQuery(); @@ -265,7 +269,7 @@ class JobList implements IJobList { if ($count === 0) { // Background job already executed elsewhere, try again. - return $this->getNext($onlyTimeSensitive, $jobClass); + return $this->getNext($onlyTimeSensitive, $jobClasses); } if ($job === null) { @@ -278,7 +282,7 @@ class JobList implements IJobList { $reset->executeStatement(); // Background job from disabled app, try again. - return $this->getNext($onlyTimeSensitive, $jobClass); + return $this->getNext($onlyTimeSensitive, $jobClasses); } return $job; diff --git a/lib/public/BackgroundJob/IJobList.php b/lib/public/BackgroundJob/IJobList.php index e2e9eca9415..396a28fc120 100644 --- a/lib/public/BackgroundJob/IJobList.php +++ b/lib/public/BackgroundJob/IJobList.php @@ -110,9 +110,12 @@ interface IJobList { /** * get the next job in the list * - * @since 7.0.0 - In 24.0.0 parameter $onlyTimeSensitive got added; In 29.0.0 parameter $jobClass got added + * @param bool $onlyTimeSensitive Whether we get only time sensitive jobs or not + * @param array|null $jobClasses List of job classes to restrict which next job we get + * @return IJob|null + * @since 7.0.0 - In 24.0.0 parameter $onlyTimeSensitive got added; In 30.0.0 parameter $jobClasses got added */ - public function getNext(bool $onlyTimeSensitive = false, string $jobClass = null): ?IJob; + public function getNext(bool $onlyTimeSensitive = false, ?array $jobClasses = null): ?IJob; /** * @since 7.0.0 diff --git a/tests/lib/BackgroundJob/DummyJobList.php b/tests/lib/BackgroundJob/DummyJobList.php index d480b93cc4a..f19e26cd7fd 100644 --- a/tests/lib/BackgroundJob/DummyJobList.php +++ b/tests/lib/BackgroundJob/DummyJobList.php @@ -100,7 +100,7 @@ class DummyJobList extends \OC\BackgroundJob\JobList { /** * get the next job in the list */ - public function getNext(bool $onlyTimeSensitive = false, string $jobClass = null): ?IJob { + public function getNext(bool $onlyTimeSensitive = false, ?array $jobClasses = null): ?IJob { if (count($this->jobs) > 0) { if ($this->last < (count($this->jobs) - 1)) { $i = $this->last + 1; From d967151f520ba6c860ed9a727e3363ec04a6506d Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 9 Apr 2024 11:12:48 +0200 Subject: [PATCH 11/12] fix(bg-jobs): review adjustments Signed-off-by: Julien Veyssier --- core/Command/Background/JobBase.php | 8 ++------ core/Command/Background/JobWorker.php | 25 +++++++++++++++++++++---- core/register_command.php | 2 -- lib/private/BackgroundJob/JobList.php | 3 +-- lib/public/BackgroundJob/IJobList.php | 8 ++++---- 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/core/Command/Background/JobBase.php b/core/Command/Background/JobBase.php index 94c9e1611ea..2de5d061378 100644 --- a/core/Command/Background/JobBase.php +++ b/core/Command/Background/JobBase.php @@ -32,16 +32,12 @@ use Psr\Log\LoggerInterface; use Symfony\Component\Console\Output\OutputInterface; abstract class JobBase extends \OC\Core\Command\Base { - protected IJobList $jobList; - protected LoggerInterface $logger; public function __construct( - IJobList $jobList, - LoggerInterface $logger + protected IJobList $jobList, + protected LoggerInterface $logger ) { parent::__construct(); - $this->jobList = $jobList; - $this->logger = $logger; } protected function printJobInfo(int $jobId, IJob $job, OutputInterface $output): void { diff --git a/core/Command/Background/JobWorker.php b/core/Command/Background/JobWorker.php index 8b7bb6b6fba..0cfe9db940b 100644 --- a/core/Command/Background/JobWorker.php +++ b/core/Command/Background/JobWorker.php @@ -26,13 +26,25 @@ declare(strict_types=1); namespace OC\Core\Command\Background; use OC\Core\Command\InterruptedException; +use OC\Files\SetupManager; +use OCP\BackgroundJob\IJobList; use OCP\ITempManager; +use Psr\Log\LoggerInterface; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class JobWorker extends JobBase { + + public function __construct( + protected IJobList $jobList, + protected LoggerInterface $logger, + private ITempManager $tempManager, + private SetupManager $setupManager, + ) { + parent::__construct($jobList, $logger); + } protected function configure(): void { parent::configure(); @@ -103,7 +115,11 @@ class JobWorker extends JobBase { $job = $this->jobList->getNext(false, $jobClasses); if (!$job) { if ($input->getOption('once') === true) { - $output->writeln('No job of classes ' . $jobClassesString . ' is currently queued', OutputInterface::VERBOSITY_VERBOSE); + if ($jobClassesString === null) { + $output->writeln('No job is currently queued', OutputInterface::VERBOSITY_VERBOSE); + } else { + $output->writeln('No job of classes ' . $jobClassesString . ' is currently queued', OutputInterface::VERBOSITY_VERBOSE); + } $output->writeln('Exiting...', OutputInterface::VERBOSITY_VERBOSE); break; } @@ -120,13 +136,14 @@ class JobWorker extends JobBase { $this->printJobInfo($job->getId(), $job, $output); } - $job->start($this->jobList); + /** @psalm-suppress DeprecatedMethod Calling execute until it is removed, then will switch to start */ + $job->execute($this->jobList); $output->writeln('Job ' . $job->getId() . ' has finished', OutputInterface::VERBOSITY_VERBOSE); // clean up after unclean jobs - \OC_Util::tearDownFS(); - \OC::$server->get(ITempManager::class)->clean(); + $this->setupManager->tearDown(); + $this->tempManager->clean(); $this->jobList->setLastJob($job); $this->jobList->unlockJob($job); diff --git a/core/register_command.php b/core/register_command.php index 88ece906846..818fc1e54f4 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -80,8 +80,6 @@ if ($config->getSystemValueBool('installed', false)) { $application->add(Server::get(Command\TwoFactorAuth\Enable::class)); $application->add(Server::get(Command\TwoFactorAuth\Disable::class)); $application->add(Server::get(Command\TwoFactorAuth\State::class)); - $application->add(Server::get(Command\TwoFactorAuth\State::class)); - $application->add(Server::get(Command\Background\Cron::class)); $application->add(Server::get(Command\Background\WebCron::class)); diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 26bc198c6e0..bc3416e3528 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -211,8 +211,7 @@ class JobList implements IJobList { } /** - * Get the next job in the list - * @return ?IJob the next job to run. Beware that this object may be a singleton and may be modified by the next call to buildJob. + * @inheritDoc */ public function getNext(bool $onlyTimeSensitive = false, ?array $jobClasses = null): ?IJob { $query = $this->connection->getQueryBuilder(); diff --git a/lib/public/BackgroundJob/IJobList.php b/lib/public/BackgroundJob/IJobList.php index 396a28fc120..f200988695d 100644 --- a/lib/public/BackgroundJob/IJobList.php +++ b/lib/public/BackgroundJob/IJobList.php @@ -108,11 +108,11 @@ interface IJobList { public function getJobsIterator($job, ?int $limit, int $offset): iterable; /** - * get the next job in the list + * Get the next job in the list * * @param bool $onlyTimeSensitive Whether we get only time sensitive jobs or not - * @param array|null $jobClasses List of job classes to restrict which next job we get - * @return IJob|null + * @param class-string[]|null $jobClasses List of job classes to restrict which next job we get + * @return ?IJob the next job to run. Beware that this object may be a singleton and may be modified by the next call to buildJob. * @since 7.0.0 - In 24.0.0 parameter $onlyTimeSensitive got added; In 30.0.0 parameter $jobClasses got added */ public function getNext(bool $onlyTimeSensitive = false, ?array $jobClasses = null): ?IJob; @@ -176,7 +176,7 @@ interface IJobList { * Returns a count of jobs per Job class * * @return list - * @since 29.0.0 + * @since 30.0.0 */ public function countByClass(): array; } From 0eb4cddc54c3b499c901bdd806b7b2aaeb5060e2 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Thu, 11 Apr 2024 10:55:39 +0200 Subject: [PATCH 12/12] feat(bg-jobs): support multiple arguments in cron.php and occ:background-job:worker for the job class list Signed-off-by: Julien Veyssier --- core/Command/Background/JobWorker.php | 28 +++++++-------------------- cron.php | 18 ++++++----------- 2 files changed, 13 insertions(+), 33 deletions(-) diff --git a/core/Command/Background/JobWorker.php b/core/Command/Background/JobWorker.php index 0cfe9db940b..0f160e44278 100644 --- a/core/Command/Background/JobWorker.php +++ b/core/Command/Background/JobWorker.php @@ -53,8 +53,8 @@ class JobWorker extends JobBase { ->setDescription('Run a background job worker') ->addArgument( 'job-classes', - InputArgument::OPTIONAL, - 'The classes of the jobs to look for in the database, comma-separated' + InputArgument::OPTIONAL | InputArgument::IS_ARRAY, + 'The classes of the jobs to look for in the database' ) ->addOption( 'once', @@ -73,25 +73,11 @@ class JobWorker extends JobBase { } protected function execute(InputInterface $input, OutputInterface $output): int { - $jobClassesString = $input->getArgument('job-classes'); - // only keep non-empty strings - $jobClasses = $jobClassesString === null - ? null - : array_filter( - explode(',', $jobClassesString), - static function (string $jobClass) { - return strlen($jobClass) > 0; - } - ); + $jobClasses = $input->getArgument('job-classes'); + $jobClasses = empty($jobClasses) ? null : $jobClasses; if ($jobClasses !== null) { - // no class - if (count($jobClasses) === 0) { - $output->writeln('Invalid job class list supplied'); - return 1; - } - - // at least one invalid class + // at least one class is invalid foreach ($jobClasses as $jobClass) { if (!class_exists($jobClass)) { $output->writeln('Invalid job class: ' . $jobClass . ''); @@ -115,10 +101,10 @@ class JobWorker extends JobBase { $job = $this->jobList->getNext(false, $jobClasses); if (!$job) { if ($input->getOption('once') === true) { - if ($jobClassesString === null) { + if ($jobClasses === null) { $output->writeln('No job is currently queued', OutputInterface::VERBOSITY_VERBOSE); } else { - $output->writeln('No job of classes ' . $jobClassesString . ' is currently queued', OutputInterface::VERBOSITY_VERBOSE); + $output->writeln('No job of classes [' . implode(', ', $jobClasses) . '] is currently queued', OutputInterface::VERBOSITY_VERBOSE); } $output->writeln('Exiting...', OutputInterface::VERBOSITY_VERBOSE); break; diff --git a/cron.php b/cron.php index d0a8b770dfc..9b0489653ef 100644 --- a/cron.php +++ b/cron.php @@ -62,10 +62,10 @@ try { Run the background job routine Usage: - php -f cron.php -- [-h] [] + php -f cron.php -- [-h] [...] Arguments: - job-classes Optional job class comma-separated list to only run those jobs + job-classes Optional job class list to only run those jobs Options: -h, --help Display this help message' . PHP_EOL; @@ -175,16 +175,10 @@ Options: $endTime = time() + 14 * 60; $executedJobs = []; - // a specific job class list can optionally be given as first argument - // only keep non-empty strings - $jobClasses = isset($argv[1]) - ? array_filter( - explode(',', $argv[1]), - static function (string $jobClass) { - return strlen($jobClass) > 0; - } - ) - : null; + // a specific job class list can optionally be given as argument + $jobClasses = array_slice($argv, 1); + $jobClasses = empty($jobClasses) ? null : $jobClasses; + while ($job = $jobList->getNext($onlyTimeSensitive, $jobClasses)) { if (isset($executedJobs[$job->getId()])) { $jobList->unlockJob($job);