From 5903ed650c20e1f6960cf2f32288ea030b12a171 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 31 Oct 2012 15:22:16 -0700 Subject: [PATCH] Move completed tasks to an "archive" table and delete them in the GC Summary: Currently, when taskmasters complete a task it is immediately deleted. This prevents us from doing some general things, like: - Supporting the idea of permanent failure (e.g., after N failures just stop trying). - Showing the user how fast taskmasters are completing tasks. - Showing the user how long tasks took to complete. Having better visibility into this is important to Drydock, which builds on the task system. Also, generally buff debug output for task execution. Test Plan: Ran `bin/phd debug taskmaster`. Ran `bin/phd debug garbage`. Queued some tasks via various systems. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2015 Differential Revision: https://secure.phabricator.com/D3852 --- conf/default.conf.php | 1 + resources/sql/patches/daemontaskarchive.sql | 13 +++ scripts/mail/create_worker_tasks.php | 7 +- scripts/repository/reparse.php | 5 +- src/__phutil_library_map__.php | 4 + .../PhabricatorDaemonConsoleController.php | 4 +- .../PhabricatorWorkerTaskDetailController.php | 2 +- .../PhabricatorWorkerTaskUpdateController.php | 2 +- .../drydock/allocator/DrydockAllocator.php | 5 +- .../storage/PhabricatorMetaMTAMail.php | 8 +- .../PhabricatorRepositoryPullLocalDaemon.php | 5 +- ...habricatorRepositoryCommitOwnersWorker.php | 6 +- ...atorRepositoryCommitChangeParserWorker.php | 6 +- ...RepositoryGitCommitMessageParserWorker.php | 6 +- ...toryMercurialCommitMessageParserWorker.php | 7 +- ...RepositorySvnCommitMessageParserWorker.php | 6 +- .../PhabricatorGarbageCollectorDaemon.php | 44 ++++++++ .../workers/PhabricatorTaskmasterDaemon.php | 27 +++-- .../daemon/workers/PhabricatorWorker.php | 10 +- .../storage/PhabricatorWorkerActiveTask.php | 104 ++++++++++++++++++ .../storage/PhabricatorWorkerArchiveTask.php | 66 +++++++++++ .../workers/storage/PhabricatorWorkerDAO.php | 4 +- .../workers/storage/PhabricatorWorkerTask.php | 48 +------- .../patch/PhabricatorBuiltinPatchList.php | 4 + 24 files changed, 292 insertions(+), 102 deletions(-) create mode 100644 resources/sql/patches/daemontaskarchive.sql create mode 100644 src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php create mode 100644 src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php diff --git a/conf/default.conf.php b/conf/default.conf.php index 4a4e7c2d37..725bb0bbdd 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -1126,6 +1126,7 @@ return array( 'gcdaemon.ttl.daemon-logs' => 7 * (24 * 60 * 60), 'gcdaemon.ttl.differential-parse-cache' => 14 * (24 * 60 * 60), 'gcdaemon.ttl.markup-cache' => 30 * (24 * 60 * 60), + 'gcdaemon.ttl.task-archive' => 14 * (24 * 60 * 60), // -- Feed ------------------------------------------------------------------ // diff --git a/resources/sql/patches/daemontaskarchive.sql b/resources/sql/patches/daemontaskarchive.sql new file mode 100644 index 0000000000..bad0027c3a --- /dev/null +++ b/resources/sql/patches/daemontaskarchive.sql @@ -0,0 +1,13 @@ +CREATE TABLE {$NAMESPACE}_worker.worker_archivetask ( + id INT UNSIGNED PRIMARY KEY, + taskClass VARCHAR(255) NOT NULL COLLATE utf8_bin, + leaseOwner VARCHAR(255) COLLATE utf8_bin, + leaseExpires INT UNSIGNED, + failureCount INT UNSIGNED NOT NULL, + dataID INT UNSIGNED NOT NULL, + result INT UNSIGNED NOT NULL, + duration BIGINT UNSIGNED NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + key(dateCreated) +) ENGINE=InnoDB, COLLATE utf8_general_ci; diff --git a/scripts/mail/create_worker_tasks.php b/scripts/mail/create_worker_tasks.php index 8d6c805663..acd5f76049 100755 --- a/scripts/mail/create_worker_tasks.php +++ b/scripts/mail/create_worker_tasks.php @@ -33,10 +33,9 @@ $messages = id(new PhabricatorMetaMTAMail())->loadAllWhere( foreach ($messages as $message) { if (!$message->getWorkerTaskID()) { - $mailer_task = new PhabricatorWorkerTask(); - $mailer_task->setTaskClass('PhabricatorMetaMTAWorker'); - $mailer_task->setData($message->getID()); - $mailer_task->save(); + $mailer_task = PhabricatorWorker::scheduleTask( + 'PhabricatorMetaMTAWorker', + $message->getID()); $message->setWorkerTaskID($mailer_task->getID()); $message->save(); diff --git a/scripts/repository/reparse.php b/scripts/repository/reparse.php index fcf8252c3f..940e23f5aa 100755 --- a/scripts/repository/reparse.php +++ b/scripts/repository/reparse.php @@ -224,10 +224,7 @@ foreach ($commits as $commit) { if ($all_from_repo && !$force_local) { foreach ($classes as $class) { - $task = new PhabricatorWorkerTask(); - $task->setTaskClass($class); - $task->setData($spec); - $task->save(); + PhabricatorWorker::scheduleTask($class, $spec); $commit_name = 'r'.$callsign.$commit->getCommitIdentifier(); echo " Queued '{$class}' for commit '{$commit_name}'.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index dc4cb6ffc3..18852344dd 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1140,6 +1140,8 @@ phutil_register_library_map(array( 'PhabricatorUserStatusOverlapException' => 'applications/people/exception/PhabricatorUserStatusOverlapException.php', 'PhabricatorUserTestCase' => 'applications/people/storage/__tests__/PhabricatorUserTestCase.php', 'PhabricatorWorker' => 'infrastructure/daemon/workers/PhabricatorWorker.php', + 'PhabricatorWorkerActiveTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php', + 'PhabricatorWorkerArchiveTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php', 'PhabricatorWorkerDAO' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerDAO.php', 'PhabricatorWorkerTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php', 'PhabricatorWorkerTaskData' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerTaskData.php', @@ -2300,6 +2302,8 @@ phutil_register_library_map(array( 'PhabricatorUserStatusInvalidEpochException' => 'Exception', 'PhabricatorUserStatusOverlapException' => 'Exception', 'PhabricatorUserTestCase' => 'PhabricatorTestCase', + 'PhabricatorWorkerActiveTask' => 'PhabricatorWorkerTask', + 'PhabricatorWorkerArchiveTask' => 'PhabricatorWorkerTask', 'PhabricatorWorkerDAO' => 'PhabricatorLiskDAO', 'PhabricatorWorkerTask' => 'PhabricatorWorkerDAO', 'PhabricatorWorkerTaskData' => 'PhabricatorWorkerDAO', diff --git a/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php b/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php index 9e22d25f0c..e51bca5a0d 100644 --- a/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php +++ b/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php @@ -34,7 +34,7 @@ final class PhabricatorDaemonConsoleController $daemon_panel->setHeader('Recently Launched Daemons'); $daemon_panel->appendChild($daemon_table); - $tasks = id(new PhabricatorWorkerTask())->loadAllWhere( + $tasks = id(new PhabricatorWorkerActiveTask())->loadAllWhere( 'leaseOwner IS NOT NULL'); $rows = array(); @@ -80,7 +80,7 @@ final class PhabricatorDaemonConsoleController $leased_panel->setHeader('Leased Tasks'); $leased_panel->appendChild($leased_table); - $task_table = new PhabricatorWorkerTask(); + $task_table = new PhabricatorWorkerActiveTask(); $queued = queryfx_all( $task_table->establishConnection('r'), 'SELECT taskClass, count(*) N FROM %T GROUP BY taskClass diff --git a/src/applications/daemon/controller/PhabricatorWorkerTaskDetailController.php b/src/applications/daemon/controller/PhabricatorWorkerTaskDetailController.php index d281626e65..30cb0c5556 100644 --- a/src/applications/daemon/controller/PhabricatorWorkerTaskDetailController.php +++ b/src/applications/daemon/controller/PhabricatorWorkerTaskDetailController.php @@ -29,7 +29,7 @@ final class PhabricatorWorkerTaskDetailController $request = $this->getRequest(); $user = $request->getUser(); - $task = id(new PhabricatorWorkerTask())->load($this->id); + $task = id(new PhabricatorWorkerActiveTask())->load($this->id); if (!$task) { $error_view = new AphrontErrorView(); $error_view->setTitle('No Such Task'); diff --git a/src/applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php b/src/applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php index f52c9f14dd..f5d5d445ca 100644 --- a/src/applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php +++ b/src/applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php @@ -31,7 +31,7 @@ final class PhabricatorWorkerTaskUpdateController $request = $this->getRequest(); $user = $request->getUser(); - $task = id(new PhabricatorWorkerTask())->load($this->id); + $task = id(new PhabricatorWorkerActiveTask())->load($this->id); if (!$task) { return new Aphront404Response(); } diff --git a/src/applications/drydock/allocator/DrydockAllocator.php b/src/applications/drydock/allocator/DrydockAllocator.php index 122bfc8dfc..1df11bcb09 100644 --- a/src/applications/drydock/allocator/DrydockAllocator.php +++ b/src/applications/drydock/allocator/DrydockAllocator.php @@ -59,10 +59,7 @@ final class DrydockAllocator { $worker = new DrydockAllocatorWorker($data); $worker->executeTask(); } else { - $task = new PhabricatorWorkerTask(); - $task->setTaskClass('DrydockAllocatorWorker'); - $task->setData($data); - $task->save(); + PhabricatorWorker::scheduleTask('DrydockAllocatorWorker', $data); } return $lease; diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index f704036d35..de0a662b14 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -298,10 +298,10 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { parent::didWriteData(); if (!$this->getWorkerTaskID()) { - $mailer_task = new PhabricatorWorkerTask(); - $mailer_task->setTaskClass('PhabricatorMetaMTAWorker'); - $mailer_task->setData($this->getID()); - $mailer_task->save(); + $mailer_task = PhabricatorWorker::scheduleTask( + 'PhabricatorMetaMTAWorker', + $this->getID()); + $this->setWorkerTaskID($mailer_task->getID()); $this->save(); } diff --git a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php index 982df11317..f5f546c628 100644 --- a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php +++ b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php @@ -450,12 +450,9 @@ final class PhabricatorRepositoryPullLocalDaemon throw new Exception("Unknown repository type '{$vcs}'!"); } - $task = new PhabricatorWorkerTask(); - $task->setTaskClass($class); $data['commitID'] = $commit->getID(); - $task->setData($data); - $task->save(); + PhabricatorWorker::scheduleTask($class, $data); } diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php index ad80e13d15..6bc3aeb673 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php @@ -73,13 +73,11 @@ final class PhabricatorRepositoryCommitOwnersWorker } if ($this->shouldQueueFollowupTasks()) { - $herald_task = new PhabricatorWorkerTask(); - $herald_task->setTaskClass('PhabricatorRepositoryCommitHeraldWorker'); - $herald_task->setData( + PhabricatorWorker::scheduleTask( + 'PhabricatorRepositoryCommitHeraldWorker', array( 'commitID' => $commit->getID(), )); - $herald_task->save(); } } diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php index 302fe53138..9db2746963 100644 --- a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php +++ b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php @@ -76,13 +76,11 @@ abstract class PhabricatorRepositoryCommitChangeParserWorker PhabricatorSearchCommitIndexer::indexCommit($commit); if ($this->shouldQueueFollowupTasks()) { - $owner_task = new PhabricatorWorkerTask(); - $owner_task->setTaskClass('PhabricatorRepositoryCommitOwnersWorker'); - $owner_task->setData( + PhabricatorWorker::scheduleTask( + 'PhabricatorRepositoryCommitOwnersWorker', array( 'commitID' => $commit->getID(), )); - $owner_task->save(); } } diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php index 079f20123e..9496ad3ace 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php @@ -69,13 +69,11 @@ final class PhabricatorRepositoryGitCommitMessageParserWorker $this->updateCommitData($author, $message, $committer); if ($this->shouldQueueFollowupTasks()) { - $task = new PhabricatorWorkerTask(); - $task->setTaskClass('PhabricatorRepositoryGitCommitChangeParserWorker'); - $task->setData( + PhabricatorWorker::scheduleTask( + 'PhabricatorRepositoryGitCommitChangeParserWorker', array( 'commitID' => $commit->getID(), )); - $task->save(); } } diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php index 22f02753fb..56ae7bb234 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php @@ -37,14 +37,11 @@ final class PhabricatorRepositoryMercurialCommitMessageParserWorker $this->updateCommitData($author, $message); if ($this->shouldQueueFollowupTasks()) { - $task = new PhabricatorWorkerTask(); - $task->setTaskClass( - 'PhabricatorRepositoryMercurialCommitChangeParserWorker'); - $task->setData( + PhabricatorWorker::scheduleTask( + 'PhabricatorRepositoryMercurialCommitChangeParserWorker', array( 'commitID' => $commit->getID(), )); - $task->save(); } } diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php index fafcbf6f41..f9dbeb390d 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php @@ -38,13 +38,11 @@ final class PhabricatorRepositorySvnCommitMessageParserWorker $this->updateCommitData($author, $message); if ($this->shouldQueueFollowupTasks()) { - $task = new PhabricatorWorkerTask(); - $task->setTaskClass('PhabricatorRepositorySvnCommitChangeParserWorker'); - $task->setData( + PhabricatorWorker::scheduleTask( + 'PhabricatorRepositorySvnCommitChangeParserWorker', array( 'commitID' => $commit->getID(), )); - $task->save(); } } diff --git a/src/infrastructure/daemon/PhabricatorGarbageCollectorDaemon.php b/src/infrastructure/daemon/PhabricatorGarbageCollectorDaemon.php index c763a99b6d..6db1e5e2f6 100644 --- a/src/infrastructure/daemon/PhabricatorGarbageCollectorDaemon.php +++ b/src/infrastructure/daemon/PhabricatorGarbageCollectorDaemon.php @@ -64,12 +64,14 @@ final class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon { $n_daemon = $this->collectDaemonLogs(); $n_parse = $this->collectParseCaches(); $n_markup = $this->collectMarkupCaches(); + $n_tasks = $this->collectArchivedTasks(); $collected = array( 'Herald Transcript' => $n_herald, 'Daemon Log' => $n_daemon, 'Differential Parse Cache' => $n_parse, 'Markup Cache' => $n_markup, + 'Archived Tasks' => $n_tasks, ); $collected = array_filter($collected); @@ -172,4 +174,46 @@ final class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon { return $conn_w->getAffectedRows(); } + private function collectArchivedTasks() { + $key = 'gcdaemon.ttl.task-archive'; + $ttl = PhabricatorEnv::getEnvConfig($key); + if ($ttl <= 0) { + return 0; + } + + $table = new PhabricatorWorkerArchiveTask(); + $data_table = new PhabricatorWorkerTaskData(); + $conn_w = $table->establishConnection('w'); + + $rows = queryfx_all( + $conn_w, + 'SELECT id, dataID FROM %T WHERE dateCreated < %d LIMIT 100', + $table->getTableName(), + time() - $ttl); + + if (!$rows) { + return 0; + } + + $data_ids = array_filter(ipull($rows, 'dataID')); + $task_ids = ipull($rows, 'id'); + + $table->openTransaction(); + if ($data_ids) { + queryfx( + $conn_w, + 'DELETE FROM %T WHERE id IN (%Ld)', + $data_table->getTableName(), + $data_ids); + } + queryfx( + $conn_w, + 'DELETE FROM %T WHERE id IN (%Ld)', + $table->getTableName(), + $task_ids); + $table->saveTransaction(); + + return count($task_ids); + } + } diff --git a/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php b/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php index 754b9b5609..973e2c0dca 100644 --- a/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php +++ b/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php @@ -21,11 +21,13 @@ final class PhabricatorTaskmasterDaemon extends PhabricatorDaemon { public function run() { $lease_ownership_name = $this->getLeaseOwnershipName(); - $task_table = new PhabricatorWorkerTask(); + $task_table = new PhabricatorWorkerActiveTask(); $taskdata_table = new PhabricatorWorkerTaskData(); $sleep = 0; do { + $this->log('Dequeuing a task...'); + $conn_w = $task_table->establishConnection('w'); queryfx( $conn_w, @@ -36,6 +38,7 @@ final class PhabricatorTaskmasterDaemon extends PhabricatorDaemon { $rows = $conn_w->getAffectedRows(); if (!$rows) { + $this->log('No unleased tasks. Dequeuing an expired lease...'); queryfx( $conn_w, 'UPDATE %T SET leaseOwner = %s, leaseExpires = UNIX_TIMESTAMP() + 15 @@ -70,6 +73,11 @@ final class PhabricatorTaskmasterDaemon extends PhabricatorDaemon { } foreach ($tasks as $task) { + $id = $task->getID(); + $class = $task->getTaskClass(); + + $this->log("Working on task {$id} ({$class})..."); + // TODO: We should detect if we acquired a task with an expired lease // and log about it / bump up failure count. @@ -77,7 +85,6 @@ final class PhabricatorTaskmasterDaemon extends PhabricatorDaemon { // failure count and fail it permanently. $data = idx($task_data, $task->getID()); - $class = $task->getTaskClass(); try { if (!class_exists($class) || !is_subclass_of($class, 'PhabricatorWorker')) { @@ -91,19 +98,19 @@ final class PhabricatorTaskmasterDaemon extends PhabricatorDaemon { $task->setLeaseDuration($lease); } + $t_start = microtime(true); $worker->executeTask(); + $t_end = microtime(true); - $task->delete(); - if ($data !== null) { - queryfx( - $conn_w, - 'DELETE FROM %T WHERE id = %d', - $taskdata_table->getTableName(), - $task->getDataID()); - } + $task->archiveTask( + PhabricatorWorkerArchiveTask::RESULT_SUCCESS, + (int)(1000000 * ($t_end - $t_start))); + $this->log("Task {$id} complete! Moved to archive."); } catch (Exception $ex) { $task->setFailureCount($task->getFailureCount() + 1); $task->save(); + + $this->log("Task {$id} failed!"); throw $ex; } } diff --git a/src/infrastructure/daemon/workers/PhabricatorWorker.php b/src/infrastructure/daemon/workers/PhabricatorWorker.php index def5a9c444..123028c558 100644 --- a/src/infrastructure/daemon/workers/PhabricatorWorker.php +++ b/src/infrastructure/daemon/workers/PhabricatorWorker.php @@ -1,7 +1,7 @@ setTaskClass($task_class) + ->setData($data) + ->save(); + } + } diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php new file mode 100644 index 0000000000..cf6f14db92 --- /dev/null +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php @@ -0,0 +1,104 @@ + false, + ) + parent::getConfiguration(); + } + + public function setServerTime($server_time) { + $this->serverTime = $server_time; + $this->localTime = time(); + return $this; + } + + public function setLeaseDuration($lease_duration) { + $server_lease_expires = $this->serverTime + $lease_duration; + $this->setLeaseExpires($server_lease_expires); + return $this->save(); + } + + public function save() { + $this->checkLease(); + + $is_new = !$this->getID(); + if ($is_new) { + $this->failureCount = 0; + } + + if ($is_new && $this->getData()) { + $data = new PhabricatorWorkerTaskData(); + $data->setData($this->getData()); + $data->save(); + + $this->setDataID($data->getID()); + } + + return parent::save(); + } + + protected function checkLease() { + if ($this->leaseOwner) { + $current_server_time = $this->serverTime + (time() - $this->localTime); + if ($current_server_time >= $this->leaseExpires) { + throw new Exception("Trying to update task after lease expiration!"); + } + } + } + + public function delete() { + throw new Exception( + "Active tasks can not be deleted directly. ". + "Use archiveTask() to move tasks to the archive."); + } + + public function archiveTask($result, $duration) { + if (!$this->getID()) { + throw new Exception( + "Attempting to archive a task which hasn't been save()d!"); + } + + $this->checkLease(); + + $archive = id(new PhabricatorWorkerArchiveTask()) + ->setID($this->getID()) + ->setTaskClass($this->getTaskClass()) + ->setLeaseOwner($this->getLeaseOwner()) + ->setLeaseExpires($this->getLeaseExpires()) + ->setFailureCount($this->getFailureCount()) + ->setDataID($this->getDataID()) + ->setResult($result) + ->setDuration($duration); + + // NOTE: This deletes the active task (this object)! + $archive->save(); + + return $archive; + } + +} diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php new file mode 100644 index 0000000000..e22370e314 --- /dev/null +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php @@ -0,0 +1,66 @@ +getID()) { + throw new Exception( + "Trying to archive a task with no ID."); + } + + $other = new PhabricatorWorkerActiveTask(); + $conn_w = $this->establishConnection('w'); + + $this->openTransaction(); + queryfx( + $conn_w, + 'DELETE FROM %T WHERE id = %d', + $other->getTableName(), + $this->getID()); + $result = parent::insert(); + $this->saveTransaction(); + + return $result; + } + + public function delete() { + $this->openTransaction(); + if ($this->getDataID()) { + $conn_w = $this->establishConnection('w'); + $data_table = new PhabricatorWorkerTaskData(); + + queryfx( + $conn_w, + 'DELETE FROM %T WHERE id = %d', + $data_table->getTableName(), + $this->getDataID()); + } + + $result = parent::delete(); + $this->saveTransaction(); + return $result; + } + +} diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerDAO.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerDAO.php index 0d0f91c93f..2ccb199b0f 100644 --- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerDAO.php +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerDAO.php @@ -1,7 +1,7 @@ false, - ) + parent::getConfiguration(); - } - - public function setServerTime($server_time) { - $this->serverTime = $server_time; - $this->localTime = time(); - return $this; - } - - public function setLeaseDuration($lease_duration) { - $server_lease_expires = $this->serverTime + $lease_duration; - $this->setLeaseExpires($server_lease_expires); - return $this->save(); - } - - public function save() { - if ($this->leaseOwner) { - $current_server_time = $this->serverTime + (time() - $this->localTime); - if ($current_server_time >= $this->leaseExpires) { - throw new Exception("Trying to update task after lease expiration!"); - } - } - - $is_new = !$this->getID(); - if ($is_new) { - $this->failureCount = 0; - } - - if ($is_new && $this->data) { - $data = new PhabricatorWorkerTaskData(); - $data->setData($this->data); - $data->save(); - - $this->setDataID($data->getID()); - } - - return parent::save(); - } - public function setData($data) { $this->data = $data; return $this; diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 0f40f7504f..7824b41178 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1016,6 +1016,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('statustxt.sql'), ), + 'daemontaskarchive.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('daemontaskarchive.sql'), + ), ); }