Page MenuHomePhabricator
Files F14390230

D8792.diff
No OneTemporary
Actions

D8792.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2239,6 +2239,7 @@
'PhabricatorWorkerTaskDetailController' => 'applications/daemon/controller/PhabricatorWorkerTaskDetailController.php',
'PhabricatorWorkerTaskUpdateController' => 'applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php',
'PhabricatorWorkerTestCase' => 'infrastructure/daemon/workers/__tests__/PhabricatorWorkerTestCase.php',
+ 'PhabricatorWorkerYieldException' => 'infrastructure/daemon/workers/exception/PhabricatorWorkerYieldException.php',
'PhabricatorWorkingCopyDiscoveryTestCase' => 'applications/repository/engine/__tests__/PhabricatorWorkingCopyDiscoveryTestCase.php',
'PhabricatorWorkingCopyPullTestCase' => 'applications/repository/engine/__tests__/PhabricatorWorkingCopyPullTestCase.php',
'PhabricatorWorkingCopyTestCase' => 'applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php',
@@ -5126,6 +5127,7 @@
'PhabricatorWorkerTaskDetailController' => 'PhabricatorDaemonController',
'PhabricatorWorkerTaskUpdateController' => 'PhabricatorDaemonController',
'PhabricatorWorkerTestCase' => 'PhabricatorTestCase',
+ 'PhabricatorWorkerYieldException' => 'Exception',
'PhabricatorWorkingCopyDiscoveryTestCase' => 'PhabricatorWorkingCopyTestCase',
'PhabricatorWorkingCopyPullTestCase' => 'PhabricatorWorkingCopyTestCase',
'PhabricatorWorkingCopyTestCase' => 'PhabricatorTestCase',
diff --git a/src/applications/harbormaster/query/HarbormasterBuildQuery.php b/src/applications/harbormaster/query/HarbormasterBuildQuery.php
--- a/src/applications/harbormaster/query/HarbormasterBuildQuery.php
+++ b/src/applications/harbormaster/query/HarbormasterBuildQuery.php
@@ -132,35 +132,35 @@
private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
$where = array();
- if ($this->ids) {
+ if ($this->ids !== null) {
$where[] = qsprintf(
$conn_r,
'id IN (%Ld)',
$this->ids);
}
- if ($this->phids) {
+ if ($this->phids !== null) {
$where[] = qsprintf(
$conn_r,
'phid in (%Ls)',
$this->phids);
}
- if ($this->buildStatuses) {
+ if ($this->buildStatuses !== null) {
$where[] = qsprintf(
$conn_r,
'buildStatus in (%Ls)',
$this->buildStatuses);
}
- if ($this->buildablePHIDs) {
+ if ($this->buildablePHIDs !== null) {
$where[] = qsprintf(
$conn_r,
'buildablePHID IN (%Ls)',
$this->buildablePHIDs);
}
- if ($this->buildPlanPHIDs) {
+ if ($this->buildPlanPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
'buildPlanPHID IN (%Ls)',
diff --git a/src/applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php
--- a/src/applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php
@@ -28,24 +28,17 @@
// finished.
$plan = $build->getBuildPlan();
- $log = null;
- $log_start = null;
- $blockers = $this->getBlockers($object, $plan, $build);
- while (count($blockers) > 0) {
- if ($log === null) {
- $log = $build->createLog($build_target, "waiting", "blockers");
- $log_start = $log->start();
- }
+ $log = $build->createLog($build_target, "waiting", "blockers");
+ $log_start = $log->start();
+ $blockers = $this->getBlockers($object, $plan, $build);
+ if ($blockers) {
$log->append("Blocked by: ".implode(",", $blockers)."\n");
-
- // TODO: This should fail temporarily instead after setting the target to
- // waiting, and thereby push the build into a waiting status.
- sleep(1);
- $blockers = $this->getBlockers($object, $plan, $build);
}
- if ($log !== null) {
- $log->finalize($log_start);
+ $log->finalize($log_start);
+
+ if ($blockers) {
+ throw new PhabricatorWorkerYieldException(15);
}
}
@@ -87,6 +80,10 @@
->execute();
$buildable_phids = mpull($buildables, 'getPHID');
+ if (!$buildable_phids) {
+ return array();
+ }
+
$builds = id(new HarbormasterBuildQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withBuildablePHIDs($buildable_phids)
@@ -101,4 +98,5 @@
return $blockers;
}
+
}
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
@@ -52,6 +52,10 @@
$target->setTargetStatus($next_status);
$target->save();
+ } catch (PhabricatorWorkerYieldException $ex) {
+ // If the target wants to yield, let that escape without further
+ // processing. We'll resume after the task retries.
+ throw $ex;
} catch (Exception $ex) {
phlog($ex);
diff --git a/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php b/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php
--- a/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php
+++ b/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php
@@ -21,6 +21,8 @@
if ($ex) {
if ($ex instanceof PhabricatorWorkerPermanentFailureException) {
$this->log("Task {$id} failed permanently.");
+ } else if ($ex instanceof PhabricatorWorkerYieldException) {
+ $this->log(pht('Task %s yielded.', $id));
} else {
$this->log("Task {$id} failed!");
throw new PhutilProxyException(
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
@@ -93,7 +93,21 @@
if (self::$runAllTasksInProcess) {
// Do the work in-process.
$worker = newv($task_class, array($data));
- $worker->doWork();
+
+ while (true) {
+ try {
+ $worker->doWork();
+ break;
+ } catch (PhabricatorWorkerYieldException $ex) {
+ phlog(
+ pht(
+ 'In-process task "%s" yielded for %s seconds, sleeping...',
+ $task_class,
+ $ex->getDuration()));
+
+ sleep($ex->getDuration());
+ }
+ }
// Now, save a task row and immediately archive it so we can return an
// object with a valid ID.
diff --git a/src/infrastructure/daemon/workers/exception/PhabricatorWorkerYieldException.php b/src/infrastructure/daemon/workers/exception/PhabricatorWorkerYieldException.php
new file mode 100644
--- /dev/null
+++ b/src/infrastructure/daemon/workers/exception/PhabricatorWorkerYieldException.php
@@ -0,0 +1,22 @@
+<?php
+
+/**
+ * Allows tasks to yield to other tasks.
+ *
+ * If a worker throws this exception while processing a task, the task will be
+ * pushed toward the back of the queue and tried again later.
+ */
+final class PhabricatorWorkerYieldException extends Exception {
+
+ private $duration;
+
+ public function __construct($duration) {
+ $this->duration = $duration;
+ parent::__construct();
+ }
+
+ public function getDuration() {
+ return $this->duration;
+ }
+
+}
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
@@ -133,6 +133,16 @@
PhabricatorWorkerArchiveTask::RESULT_FAILURE,
0);
$result->setExecutionException($ex);
+ } catch (PhabricatorWorkerYieldException $ex) {
+ $this->setExecutionException($ex);
+
+ $retry = $ex->getDuration();
+ $retry = max($retry, 5);
+
+ // NOTE: As a side effect, this saves the object.
+ $this->setLeaseDuration($retry);
+
+ $result = $this;
} catch (Exception $ex) {
$this->setExecutionException($ex);
$this->setFailureCount($this->getFailureCount() + 1);

File Metadata

Mime Type
text/plain
Expires
Sun, Dec 22, 6:15 PM (7 h, 39 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6919349
Default Alt Text
D8792.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility