From 6c1680addc381414fcf97189bd36b467c797e35f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 22 Apr 2016 10:49:55 +0200 Subject: 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. --- lib/private/backgroundjob/joblist.php | 4 ++-- tests/lib/backgroundjob/joblist.php | 17 +++++++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/private/backgroundjob/joblist.php b/lib/private/backgroundjob/joblist.php index 4c472a6a2bf..31007fdd678 100644 --- a/lib/private/backgroundjob/joblist.php +++ b/lib/private/backgroundjob/joblist.php @@ -138,14 +138,14 @@ class JobList implements IJobList { */ public function getNext() { $lastId = $this->getLastJob(); - $query = $this->conn->prepare('SELECT `id`, `class`, `last_run`, `argument` FROM `*PREFIX*jobs` WHERE `id` > ? ORDER BY `id` ASC', 1); + $query = $this->conn->prepare('SELECT `id`, `class`, `last_run`, `argument` FROM `*PREFIX*jobs` WHERE `id` < ? ORDER BY `id` DESC', 1); $query->execute(array($lastId)); if ($row = $query->fetch()) { $jobId = $row['id']; $job = $this->buildJob($row); } else { //begin at the start of the queue - $query = $this->conn->prepare('SELECT `id`, `class`, `last_run`, `argument` FROM `*PREFIX*jobs` ORDER BY `id` ASC', 1); + $query = $this->conn->prepare('SELECT `id`, `class`, `last_run`, `argument` FROM `*PREFIX*jobs` ORDER BY `id` DESC', 1); $query->execute(); if ($row = $query->fetch()) { $jobId = $row['id']; diff --git a/tests/lib/backgroundjob/joblist.php b/tests/lib/backgroundjob/joblist.php index b329c83009b..e222413391b 100644 --- a/tests/lib/backgroundjob/joblist.php +++ b/tests/lib/backgroundjob/joblist.php @@ -9,6 +9,7 @@ namespace Test\BackgroundJob; use OCP\BackgroundJob\IJob; +use OCP\IDBConnection; class JobList extends \Test\TestCase { /** @@ -25,10 +26,17 @@ class JobList extends \Test\TestCase { parent::setUp(); $conn = \OC::$server->getDatabaseConnection(); + $this->clearJobsList($conn); $this->config = $this->getMock('\OCP\IConfig'); $this->instance = new \OC\BackgroundJob\JobList($conn, $this->config); } + protected function clearJobsList(IDBConnection $connection) { + $query = $connection->getQueryBuilder(); + $query->delete('jobs'); + $query->execute(); + } + protected function getAllSorted() { $jobs = $this->instance->getAll(); @@ -146,11 +154,11 @@ class JobList extends \Test\TestCase { $this->config->expects($this->once()) ->method('getAppValue') ->with('backgroundjob', 'lastjob', 0) - ->will($this->returnValue($savedJob1->getId())); + ->will($this->returnValue($savedJob2->getId())); $nextJob = $this->instance->getNext(); - $this->assertEquals($savedJob2, $nextJob); + $this->assertEquals($savedJob1, $nextJob); $this->instance->remove($job, 1); $this->instance->remove($job, 2); @@ -163,16 +171,17 @@ class JobList extends \Test\TestCase { $jobs = $this->getAllSorted(); + $savedJob1 = $jobs[count($jobs) - 2]; $savedJob2 = $jobs[count($jobs) - 1]; $this->config->expects($this->once()) ->method('getAppValue') ->with('backgroundjob', 'lastjob', 0) - ->will($this->returnValue($savedJob2->getId())); + ->will($this->returnValue($savedJob1->getId())); $nextJob = $this->instance->getNext(); - $this->assertEquals($jobs[0], $nextJob); + $this->assertEquals($savedJob2, $nextJob); $this->instance->remove($job, 1); $this->instance->remove($job, 2); -- cgit v1.2.3