Page MenuHomePhabricator
Files F14410647

D8774.diff
No OneTemporary
Actions

D8774.diff
View Options

diff --git a/src/applications/feed/worker/FeedPublisherWorker.php b/src/applications/feed/worker/FeedPublisherWorker.php
--- a/src/applications/feed/worker/FeedPublisherWorker.php
+++ b/src/applications/feed/worker/FeedPublisherWorker.php
@@ -7,7 +7,7 @@
$uris = PhabricatorEnv::getEnvConfig('feed.http-hooks');
foreach ($uris as $uri) {
- PhabricatorWorker::scheduleTask(
+ $this->queueTask(
'FeedPublisherHTTPWorker',
array(
'key' => $story->getChronologicalKey(),
@@ -27,7 +27,7 @@
if (!$worker->isEnabled()) {
continue;
}
- PhabricatorWorker::scheduleTask(
+ $this->queueTask(
get_class($worker),
array(
'key' => $story->getChronologicalKey(),
diff --git a/src/applications/harbormaster/worker/HarbormasterTargetWorker.php b/src/applications/harbormaster/worker/HarbormasterTargetWorker.php
--- a/src/applications/harbormaster/worker/HarbormasterTargetWorker.php
+++ b/src/applications/harbormaster/worker/HarbormasterTargetWorker.php
@@ -8,7 +8,7 @@
public function getRequiredLeaseTime() {
// This worker performs actual build work, which may involve a long wait
// on external systems.
- return 60 * 60 * 24;
+ return phutil_units('24 hours in seconds');
}
private function loadBuildTarget() {
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
@@ -5,7 +5,7 @@
public function getRequiredLeaseTime() {
// Herald rules may take a long time to process.
- return 4 * 60 * 60;
+ return phutil_units('4 hours in seconds');
}
public function parseCommit(
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
@@ -3,11 +3,6 @@
final class PhabricatorRepositoryCommitOwnersWorker
extends PhabricatorRepositoryCommitParserWorker {
- public function getRequiredLeaseTime() {
- // Commit owner parser may take a while to process for huge commits.
- return 60 * 60;
- }
-
protected function parseCommit(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
@@ -18,7 +13,7 @@
PhabricatorRepositoryCommit::IMPORTED_OWNERS);
if ($this->shouldQueueFollowupTasks()) {
- PhabricatorWorker::scheduleTask(
+ $this->queueTask(
'PhabricatorRepositoryCommitHeraldWorker',
array(
'commitID' => $commit->getID(),
diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
--- a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
+++ b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
@@ -6,7 +6,7 @@
public function getRequiredLeaseTime() {
// It can take a very long time to parse commits; some commits in the
// Facebook repository affect many millions of paths. Acquire 24h leases.
- return 60 * 60 * 24;
+ return phutil_units('24 hours in seconds');
}
abstract protected function parseCommitChanges(
@@ -98,7 +98,7 @@
PhabricatorOwnersPackagePathValidator::updateOwnersPackagePaths($commit);
if ($this->shouldQueueFollowupTasks()) {
- PhabricatorWorker::scheduleTask(
+ $this->queueTask(
'PhabricatorRepositoryCommitOwnersWorker',
array(
'commitID' => $commit->getID(),
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php
@@ -15,7 +15,7 @@
$this->updateCommitData($ref);
if ($this->shouldQueueFollowupTasks()) {
- PhabricatorWorker::scheduleTask(
+ $this->queueTask(
'PhabricatorRepositoryGitCommitChangeParserWorker',
array(
'commitID' => $commit->getID(),
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php
@@ -15,7 +15,7 @@
$this->updateCommitData($ref);
if ($this->shouldQueueFollowupTasks()) {
- PhabricatorWorker::scheduleTask(
+ $this->queueTask(
'PhabricatorRepositoryMercurialCommitChangeParserWorker',
array(
'commitID' => $commit->getID(),
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php
@@ -15,7 +15,7 @@
$this->updateCommitData($ref);
if ($this->shouldQueueFollowupTasks()) {
- PhabricatorWorker::scheduleTask(
+ $this->queueTask(
'PhabricatorRepositorySvnCommitChangeParserWorker',
array(
'commitID' => $commit->getID(),
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,6 +9,7 @@
private $data;
private static $runAllTasksInProcess = false;
+ private $queuedTasks = array();
/* -( Configuring Retries and Failures )----------------------------------- */
@@ -17,13 +18,13 @@
/**
* Return the number of seconds this worker needs hold a lease on the task for
* while it performs work. For most tasks you can leave this at `null`, which
- * will give you a short default lease (currently 60 seconds).
+ * will give you a default lease (currently 2 hours).
*
* For tasks which may take a very long time to complete, you should return
* an upper bound on the amount of time the task may require.
*
* @return int|null Number of seconds this task needs to remain leased for,
- * or null for a default (currently 60 second) lease.
+ * or null for a default lease.
*
* @task config
*/
@@ -194,4 +195,29 @@
return $this;
}
+
+ /**
+ * Queue a task to be executed after this one suceeds.
+ *
+ * The followup task will be queued only if this task completes cleanly.
+ *
+ * @param string Task class to queue.
+ * @param array Data for the followup task.
+ * @return this
+ */
+ protected function queueTask($class, array $data) {
+ $this->queuedTasks[] = array($class, $data);
+ return $this;
+ }
+
+
+ /**
+ * Get tasks queued as followups by @{method:queueTask}.
+ *
+ * @return list<pair<string, wild>> Queued task specifications.
+ */
+ public function getQueuedTasks() {
+ return $this->queuedTasks;
+ }
+
}
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
@@ -10,11 +10,17 @@
const PHASE_UNLEASED = 'unleased';
const PHASE_EXPIRED = 'expired';
- const DEFAULT_LEASE_DURATION = 60; // Seconds
-
private $ids;
private $limit;
+ public static function getDefaultWaitBeforeRetry() {
+ return phutil_units('5 minutes in seconds');
+ }
+
+ public static function getDefaultLeaseDuration() {
+ return phutil_units('2 hours in seconds');
+ }
+
public function withIDs(array $ids) {
$this->ids = $ids;
return $this;
@@ -78,7 +84,7 @@
%Q',
$task_table->getTableName(),
$lease_ownership_name,
- self::DEFAULT_LEASE_DURATION,
+ self::getDefaultLeaseDuration(),
$this->buildUpdateWhereClause($conn_w, $phase, $rows));
$leased += $conn_w->getAffectedRows();
diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php
--- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php
+++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php
@@ -100,6 +100,7 @@
// to release the lease otherwise.
$this->checkLease();
+ $did_succeed = false;
try {
$worker = $this->getWorkerInstance();
@@ -126,6 +127,7 @@
$result = $this->archiveTask(
PhabricatorWorkerArchiveTask::RESULT_SUCCESS,
$duration);
+ $did_succeed = true;
} catch (PhabricatorWorkerPermanentFailureException $ex) {
$result = $this->archiveTask(
PhabricatorWorkerArchiveTask::RESULT_FAILURE,
@@ -139,7 +141,7 @@
$retry = $worker->getWaitBeforeRetry($this);
$retry = coalesce(
$retry,
- PhabricatorWorkerLeaseQuery::DEFAULT_LEASE_DURATION);
+ PhabricatorWorkerLeaseQuery::getDefaultWaitBeforeRetry());
// NOTE: As a side effect, this saves the object.
$this->setLeaseDuration($retry);
@@ -147,6 +149,15 @@
$result = $this;
}
+ // NOTE: If this throws, we don't want it to cause the task to fail again,
+ // so execute it out here and just let the exception escape.
+ if ($did_succeed) {
+ foreach ($worker->getQueuedTasks() as $task) {
+ list($class, $data) = $task;
+ PhabricatorWorker::scheduleTask($class, $data);
+ }
+ }
+
return $result;
}

File Metadata

Mime Type
text/plain
Expires
Wed, Dec 25, 8:15 AM (10 h, 31 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6925552
Default Alt Text
D8774.diff (10 KB)

Event Timeline

Phabricator · Built by Phacility