Prevent a race in Phabricator workers

Summary: See D133. Workers can also be subject to the same race, invert the
row relationship in the same way.

Test Plan: Launched repository master daemons and some taskmasters and used
the Daemon console to veify that they were able to process tasks. Manually
checked the database to make sure data got linked correctly and that new data
was inserted correctly.

Reviewers: jungejason

CC: tuomaspelkonen

Differential Revision: 135

resources/sql/patches/031.workerrace.sql

@@ -0,0 +1,13 @@
+ALTER TABLE phabricator_worker.worker_task
+  ADD dataID int unsigned;
+
+ALTER TABLE phabricator_worker.worker_task
+  ADD UNIQUE KEY (dataID);
+
+UPDATE phabricator_worker.worker_task t,
+       phabricator_worker.worker_taskdata d
+  SET t.dataID = d.id
+  WHERE d.taskID = t.id;
+
+ALTER TABLE phabricator_worker.worker_taskdata
+  DROP taskID;

src/applications/daemon/controller/workertaskdetail/PhabricatorWorkerTaskDetailController.php

@@ -44,8 +44,8 @@ class PhabricatorWorkerTaskDetailController
     }
 
     $data = id(new PhabricatorWorkerTaskData())->loadOneWhere(
-      'taskID = %d',
+      'id = %d',
-      $task->getID());
+      $task->getDataID());
     if ($data) {
       $data = json_encode($data->getData());
     }

src/infrastructure/daemon/workers/storage/task/PhabricatorWorkerTask.php

@@ -22,6 +22,7 @@ class PhabricatorWorkerTask extends PhabricatorWorkerDAO {
   protected $leaseOwner;
   protected $leaseExpires;
   protected $failureCount;
+  protected $dataID;
 
   private $serverTime;
   private $localTime;
@@ -58,16 +59,15 @@ class PhabricatorWorkerTask extends PhabricatorWorkerDAO {
       $this->failureCount = 0;
     }
 
-    $ret = parent::save();
-
     if ($is_new && $this->data) {
       $data = new PhabricatorWorkerTaskData();
-      $data->setTaskID($this->getID());
       $data->setData($this->data);
       $data->save();
+
+      $this->setDataID($data->getID());
     }
 
-    return $ret;
+    return parent::save();
   }
 
   public function setData($data) {

src/infrastructure/daemon/workers/storage/taskdata/PhabricatorWorkerTaskData.php

@@ -18,7 +18,6 @@
 
 class PhabricatorWorkerTaskData extends PhabricatorWorkerDAO {
 
-  protected $taskID;
   protected $data;
 
   public function getConfiguration() {

src/infrastructure/daemon/workers/taskmaster/PhabricatorTaskmasterDaemon.php

@@ -50,7 +50,7 @@ class PhabricatorTaskmasterDaemon extends PhabricatorDaemon {
           $conn_w,
           'SELECT task.*, taskdata.data _taskData, UNIX_TIMESTAMP() _serverTime
             FROM %T task LEFT JOIN %T taskdata
-              ON taskdata.taskID = task.id
+              ON taskdata.id = task.dataID
             WHERE leaseOwner = %s AND leaseExpires > UNIX_TIMESTAMP()
             LIMIT 1',
           $task_table->getTableName(),
@@ -97,9 +97,9 @@ class PhabricatorTaskmasterDaemon extends PhabricatorDaemon {
             if ($data !== null) {
               queryfx(
                 $conn_w,
-                'DELETE FROM %T WHERE taskID = %d',
+                'DELETE FROM %T WHERE id = %d',
                 $taskdata_table->getTableName(),
-                $task->getID());
+                $task->getDataID());
             }
           } catch (Exception $ex) {
             $task->setFailureCount($task->getFailureCount() + 1);