Change the sort order of background jobs to be DESC instead of ASC
In theory, if your instance ever creates more jobs then your system cron can handle, the default background jobs get never executed anymore. Because everytime when the joblist returns the next job it looks for the next ID, however there is always a new next ID, so it will never wrap back to execute the low IDs. But when we change the sort order to be DESC, we make sure that these low IDs are always executed, before the system jumps back up to execute the new IDs.
This commit is contained in:
parent
7be1094ba5
commit
04cee6a7db
|
@ -172,8 +172,8 @@ class JobList implements IJobList {
|
||||||
$query = $this->connection->getQueryBuilder();
|
$query = $this->connection->getQueryBuilder();
|
||||||
$query->select('*')
|
$query->select('*')
|
||||||
->from('jobs')
|
->from('jobs')
|
||||||
->where($query->expr()->gt('id', $query->createNamedParameter($lastId, IQueryBuilder::PARAM_INT)))
|
->where($query->expr()->lt('id', $query->createNamedParameter($lastId, IQueryBuilder::PARAM_INT)))
|
||||||
->orderBy('id', 'ASC')
|
->orderBy('id', 'DESC')
|
||||||
->setMaxResults(1);
|
->setMaxResults(1);
|
||||||
$result = $query->execute();
|
$result = $query->execute();
|
||||||
$row = $result->fetch();
|
$row = $result->fetch();
|
||||||
|
@ -187,7 +187,7 @@ class JobList implements IJobList {
|
||||||
$query = $this->connection->getQueryBuilder();
|
$query = $this->connection->getQueryBuilder();
|
||||||
$query->select('*')
|
$query->select('*')
|
||||||
->from('jobs')
|
->from('jobs')
|
||||||
->orderBy('id', 'ASC')
|
->orderBy('id', 'DESC')
|
||||||
->setMaxResults(1);
|
->setMaxResults(1);
|
||||||
$result = $query->execute();
|
$result = $query->execute();
|
||||||
$row = $result->fetch();
|
$row = $result->fetch();
|
||||||
|
|
|
@ -9,6 +9,7 @@
|
||||||
namespace Test\BackgroundJob;
|
namespace Test\BackgroundJob;
|
||||||
|
|
||||||
use OCP\BackgroundJob\IJob;
|
use OCP\BackgroundJob\IJob;
|
||||||
|
use OCP\IDBConnection;
|
||||||
use Test\TestCase;
|
use Test\TestCase;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -28,10 +29,17 @@ class JobList extends TestCase {
|
||||||
parent::setUp();
|
parent::setUp();
|
||||||
|
|
||||||
$connection = \OC::$server->getDatabaseConnection();
|
$connection = \OC::$server->getDatabaseConnection();
|
||||||
|
$this->clearJobsList($connection);
|
||||||
$this->config = $this->getMock('\OCP\IConfig');
|
$this->config = $this->getMock('\OCP\IConfig');
|
||||||
$this->instance = new \OC\BackgroundJob\JobList($connection, $this->config);
|
$this->instance = new \OC\BackgroundJob\JobList($connection, $this->config);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function clearJobsList(IDBConnection $connection) {
|
||||||
|
$query = $connection->getQueryBuilder();
|
||||||
|
$query->delete('jobs');
|
||||||
|
$query->execute();
|
||||||
|
}
|
||||||
|
|
||||||
protected function getAllSorted() {
|
protected function getAllSorted() {
|
||||||
$jobs = $this->instance->getAll();
|
$jobs = $this->instance->getAll();
|
||||||
|
|
||||||
|
@ -149,11 +157,11 @@ class JobList extends TestCase {
|
||||||
$this->config->expects($this->once())
|
$this->config->expects($this->once())
|
||||||
->method('getAppValue')
|
->method('getAppValue')
|
||||||
->with('backgroundjob', 'lastjob', 0)
|
->with('backgroundjob', 'lastjob', 0)
|
||||||
->will($this->returnValue($savedJob1->getId()));
|
->will($this->returnValue($savedJob2->getId()));
|
||||||
|
|
||||||
$nextJob = $this->instance->getNext();
|
$nextJob = $this->instance->getNext();
|
||||||
|
|
||||||
$this->assertEquals($savedJob2, $nextJob);
|
$this->assertEquals($savedJob1, $nextJob);
|
||||||
|
|
||||||
$this->instance->remove($job, 1);
|
$this->instance->remove($job, 1);
|
||||||
$this->instance->remove($job, 2);
|
$this->instance->remove($job, 2);
|
||||||
|
@ -166,16 +174,17 @@ class JobList extends TestCase {
|
||||||
|
|
||||||
$jobs = $this->getAllSorted();
|
$jobs = $this->getAllSorted();
|
||||||
|
|
||||||
|
$savedJob1 = $jobs[count($jobs) - 2];
|
||||||
$savedJob2 = $jobs[count($jobs) - 1];
|
$savedJob2 = $jobs[count($jobs) - 1];
|
||||||
|
|
||||||
$this->config->expects($this->once())
|
$this->config->expects($this->once())
|
||||||
->method('getAppValue')
|
->method('getAppValue')
|
||||||
->with('backgroundjob', 'lastjob', 0)
|
->with('backgroundjob', 'lastjob', 0)
|
||||||
->will($this->returnValue($savedJob2->getId()));
|
->will($this->returnValue($savedJob1->getId()));
|
||||||
|
|
||||||
$nextJob = $this->instance->getNext();
|
$nextJob = $this->instance->getNext();
|
||||||
|
|
||||||
$this->assertEquals($jobs[0], $nextJob);
|
$this->assertEquals($savedJob2, $nextJob);
|
||||||
|
|
||||||
$this->instance->remove($job, 1);
|
$this->instance->remove($job, 1);
|
||||||
$this->instance->remove($job, 2);
|
$this->instance->remove($job, 2);
|
||||||
|
|
Loading…
Reference in New Issue