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 @@ +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);