mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-18 19:40:55 +01:00
Reverse meaning of task priority column
Summary: Ref T6615. Mixing ASC and DESC ordering on a multipart key makes it dramatically less effective (or perhaps totally ineffective). Reverse the meaning of the `priority` column so it goes in the same direction as the `id` column (both ascending, lower values execute sooner). Test Plan: - Queued 1.2M tasks with `bin/worker flood`. - Processed ~1 task/second with `bin/phd debug taskmaster` before patch. - Applied patch, took ~5 seconds for ~1.2M rows. - Processed ~100-200 tasks/second with `bin/phd debug taskmaster` after patch. - "Next in Queue" query on daemon page dropped from 1.5s to <1ms. Reviewers: btrahan Reviewed By: btrahan Subscribers: aklapper, 20after4, epriestley Maniphest Tasks: T6615 Differential Revision: https://secure.phabricator.com/D10895
This commit is contained in:
parent
7e1c312183
commit
b5f7e9eec6
5 changed files with 19 additions and 11 deletions
2
resources/sql/autopatches/20141123.taskpriority.1.sql
Normal file
2
resources/sql/autopatches/20141123.taskpriority.1.sql
Normal file
|
@ -0,0 +1,2 @@
|
||||||
|
UPDATE {$NAMESPACE}_worker.worker_activetask
|
||||||
|
SET priority = 5000 - priority;
|
2
resources/sql/autopatches/20141123.taskpriority.2.sql
Normal file
2
resources/sql/autopatches/20141123.taskpriority.2.sql
Normal file
|
@ -0,0 +1,2 @@
|
||||||
|
UPDATE {$NAMESPACE}_worker.worker_archivetask
|
||||||
|
SET priority = 5000 - priority;
|
|
@ -9,10 +9,14 @@ abstract class PhabricatorWorker {
|
||||||
private static $runAllTasksInProcess = false;
|
private static $runAllTasksInProcess = false;
|
||||||
private $queuedTasks = array();
|
private $queuedTasks = array();
|
||||||
|
|
||||||
const PRIORITY_ALERTS = 4000;
|
// NOTE: Lower priority numbers execute first. The priority numbers have to
|
||||||
const PRIORITY_DEFAULT = 3000;
|
// have the same ordering that IDs do (lowest first) so MySQL can use a
|
||||||
const PRIORITY_BULK = 2000;
|
// multipart key across both of them efficiently.
|
||||||
const PRIORITY_IMPORT = 1000;
|
|
||||||
|
const PRIORITY_ALERTS = 1000;
|
||||||
|
const PRIORITY_DEFAULT = 2000;
|
||||||
|
const PRIORITY_BULK = 3000;
|
||||||
|
const PRIORITY_IMPORT = 4000;
|
||||||
|
|
||||||
|
|
||||||
/* -( Configuring Retries and Failures )----------------------------------- */
|
/* -( Configuring Retries and Failures )----------------------------------- */
|
||||||
|
|
|
@ -160,14 +160,14 @@ final class PhabricatorWorkerTestCase extends PhabricatorTestCase {
|
||||||
$this->expectNextLease($task1);
|
$this->expectNextLease($task1);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testLeasedIsHighestPriority() {
|
public function testLeasedIsLowestPriority() {
|
||||||
$task1 = $this->scheduleTask(array(), 1);
|
$task1 = $this->scheduleTask(array(), 2);
|
||||||
$task2 = $this->scheduleTask(array(), 1);
|
$task2 = $this->scheduleTask(array(), 2);
|
||||||
$task3 = $this->scheduleTask(array(), 2);
|
$task3 = $this->scheduleTask(array(), 1);
|
||||||
|
|
||||||
$this->expectNextLease(
|
$this->expectNextLease(
|
||||||
$task3,
|
$task3,
|
||||||
'Tasks with a higher priority should be scheduled first.');
|
'Tasks with a lower priority should be scheduled first.');
|
||||||
$this->expectNextLease(
|
$this->expectNextLease(
|
||||||
$task1,
|
$task1,
|
||||||
'Tasks with the same priority should be FIFO.');
|
'Tasks with the same priority should be FIFO.');
|
||||||
|
|
|
@ -215,8 +215,8 @@ final class PhabricatorWorkerLeaseQuery extends PhabricatorQuery {
|
||||||
switch ($phase) {
|
switch ($phase) {
|
||||||
case self::PHASE_UNLEASED:
|
case self::PHASE_UNLEASED:
|
||||||
// When selecting new tasks, we want to consume them in order of
|
// When selecting new tasks, we want to consume them in order of
|
||||||
// decreasing priority (and then FIFO).
|
// increasing priority (and then FIFO).
|
||||||
return qsprintf($conn_w, 'ORDER BY priority DESC, id ASC');
|
return qsprintf($conn_w, 'ORDER BY priority ASC, id ASC');
|
||||||
case self::PHASE_EXPIRED:
|
case self::PHASE_EXPIRED:
|
||||||
// When selecting failed tasks, we want to consume them in roughly
|
// When selecting failed tasks, we want to consume them in roughly
|
||||||
// FIFO order of their failures, which is not necessarily their original
|
// FIFO order of their failures, which is not necessarily their original
|
||||||
|
|
Loading…
Reference in a new issue