diff --git a/resources/sql/autopatches/20141123.taskpriority.1.sql b/resources/sql/autopatches/20141123.taskpriority.1.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20141123.taskpriority.1.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_worker.worker_activetask + SET priority = 5000 - priority; diff --git a/resources/sql/autopatches/20141123.taskpriority.2.sql b/resources/sql/autopatches/20141123.taskpriority.2.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20141123.taskpriority.2.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_worker.worker_archivetask + SET priority = 5000 - priority; diff --git a/src/infrastructure/daemon/workers/PhabricatorWorker.php b/src/infrastructure/daemon/workers/PhabricatorWorker.php --- a/src/infrastructure/daemon/workers/PhabricatorWorker.php +++ b/src/infrastructure/daemon/workers/PhabricatorWorker.php @@ -9,10 +9,14 @@ private static $runAllTasksInProcess = false; private $queuedTasks = array(); - const PRIORITY_ALERTS = 4000; - const PRIORITY_DEFAULT = 3000; - const PRIORITY_BULK = 2000; - const PRIORITY_IMPORT = 1000; + // NOTE: Lower priority numbers execute first. The priority numbers have to + // have the same ordering that IDs do (lowest first) so MySQL can use a + // multipart key across both of them efficiently. + + const PRIORITY_ALERTS = 1000; + const PRIORITY_DEFAULT = 2000; + const PRIORITY_BULK = 3000; + const PRIORITY_IMPORT = 4000; /* -( Configuring Retries and Failures )----------------------------------- */ diff --git a/src/infrastructure/daemon/workers/__tests__/PhabricatorWorkerTestCase.php b/src/infrastructure/daemon/workers/__tests__/PhabricatorWorkerTestCase.php --- a/src/infrastructure/daemon/workers/__tests__/PhabricatorWorkerTestCase.php +++ b/src/infrastructure/daemon/workers/__tests__/PhabricatorWorkerTestCase.php @@ -160,14 +160,14 @@ $this->expectNextLease($task1); } - public function testLeasedIsHighestPriority() { - $task1 = $this->scheduleTask(array(), 1); - $task2 = $this->scheduleTask(array(), 1); - $task3 = $this->scheduleTask(array(), 2); + public function testLeasedIsLowestPriority() { + $task1 = $this->scheduleTask(array(), 2); + $task2 = $this->scheduleTask(array(), 2); + $task3 = $this->scheduleTask(array(), 1); $this->expectNextLease( $task3, - 'Tasks with a higher priority should be scheduled first.'); + 'Tasks with a lower priority should be scheduled first.'); $this->expectNextLease( $task1, 'Tasks with the same priority should be FIFO.'); diff --git a/src/infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php b/src/infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php --- a/src/infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php +++ b/src/infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php @@ -215,8 +215,8 @@ switch ($phase) { case self::PHASE_UNLEASED: // When selecting new tasks, we want to consume them in order of - // decreasing priority (and then FIFO). - return qsprintf($conn_w, 'ORDER BY priority DESC, id ASC'); + // increasing priority (and then FIFO). + return qsprintf($conn_w, 'ORDER BY priority ASC, id ASC'); case self::PHASE_EXPIRED: // When selecting failed tasks, we want to consume them in roughly // FIFO order of their failures, which is not necessarily their original