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 @@ -16,6 +16,7 @@ } $argv = array( + $this->getActiveTask(), array(), ); diff --git a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php --- a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php @@ -2,6 +2,19 @@ abstract class HarbormasterBuildStepImplementation { + private $worker; + + final public function setWorker(PhabricatorWorker $worker) { + $this->worker = $worker; + return $this; + } + + final protected function stillWorking() { + if ($this->worker) { + $this->worker->stillWorking(); + } + } + public static function getImplementations() { return id(new PhutilSymbolLoader()) ->setAncestorClass('HarbormasterBuildStepImplementation') diff --git a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php --- a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php @@ -78,8 +78,9 @@ foreach ($futures->setUpdateInterval(1) as $key => $future_iter) { if ($future_iter === null) { - // Check to see if we should abort. + // Flag as still working and check to see if we should abort. if ($build_update <= 0) { + $this->stillWorking(); $build->reload(); if ($this->shouldAbort($build, $build_target)) { $future->resolveKill(); 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 @@ -49,6 +49,7 @@ } $implementation = $target->getImplementation(); + $implementation->setWorker($this); $implementation->execute($build, $target); $next_status = HarbormasterBuildTarget::STATUS_PASSED; diff --git a/src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php b/src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php --- a/src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php +++ b/src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php @@ -1166,7 +1166,11 @@ throw new Exception(pht('No support yet.')); } - $parser_object = newv($parser, array(array())); + $task = id(new PhabricatorWorkerActiveTask()) + ->setData(array()) + ->makeEphemeral(); + + $parser_object = newv($parser, array($task)); return $parser_object->parseChangesForUnitTest($repository, $commit); } 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 @@ -18,6 +18,7 @@ $this->log("Working on task {$id} ({$class})..."); + $task->setDaemon($this); $task = $task->executeTask(); $ex = $task->getExecutionException(); if ($ex) { 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 @@ -5,7 +5,8 @@ */ abstract class PhabricatorWorker { - private $data; + private $daemon = null; + private $activeTask; private static $runAllTasksInProcess = false; private $queuedTasks = array(); @@ -18,6 +19,10 @@ const PRIORITY_BULK = 3000; const PRIORITY_IMPORT = 4000; + public function setDaemon(PhutilDaemon $daemon) { + $this->daemon = $daemon; + } + /* -( Configuring Retries and Failures )----------------------------------- */ @@ -79,12 +84,16 @@ abstract protected function doWork(); - final public function __construct($data) { - $this->data = $data; + final public function __construct(PhabricatorWorkerTask $task) { + $this->activeTask = $task; } final protected function getTaskData() { - return $this->data; + return $this->activeTask->getData(); + } + + final protected function getActiveTask() { + return $this->activeTask; } final public function executeTask() { @@ -110,7 +119,7 @@ if (self::$runAllTasksInProcess) { // Do the work in-process. - $worker = newv($task_class, array($data)); + $worker = $task->getWorkerInstance(); while (true) { try { @@ -147,6 +156,28 @@ } } + /** + * Mark the operation as still working. + */ + final public function stillWorking() { + // If we have less than 80% of our working time left, or less + // than 10 seconds (whichever is greater), extend it by the + // required lease time. + $time_left = + $this->activeTask->getLeaseExpires() - PhabricatorTime::getNow(); + $renew_threshold = max($this->getRequiredLeaseTime() * 0.2, 10); + if ($time_left < $renew_threshold) { + $this->activeTask->setLeaseDuration( + $this->activeTask->getLeaseExpires() - + $this->activeTask->getServerTime() + + $this->getRequiredLeaseTime()); + + if ($this->daemon !== null) { + $this->daemon->stillWorking(); + } + } + } + /** * Wait for tasks to complete. If tasks are not leased by other workers, they @@ -190,6 +221,10 @@ continue; } + // TODO: This won't behave correctly when run from a daemon; calls to + // stillWorking() won't propagate to the daemon instance. Currently + // the only call site to waitForTasks is inside Drydock, which doesn't + // perform stillWorking calls during allocation anyway. $task = head($tasks)->executeTask(); $ex = $task->getExecutionException(); @@ -209,7 +244,8 @@ } public function renderForDisplay(PhabricatorUser $viewer) { - $data = PhutilReadableSerializer::printableValue($this->data); + $data = PhutilReadableSerializer::printableValue( + $this->activeTask->getData()); return phutil_tag('pre', array(), $data); } 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 @@ -6,6 +6,7 @@ private $serverTime; private $localTime; + private $daemon; protected function getConfiguration() { $parent = parent::getConfiguration(); @@ -52,6 +53,10 @@ return $this; } + public function getServerTime() { + return $this->serverTime; + } + public function setLeaseDuration($lease_duration) { $this->checkLease(); $server_lease_expires = $this->serverTime + $lease_duration; @@ -129,6 +134,11 @@ return $archive; } + public function setDaemon(PhutilDaemon $daemon) { + $this->daemon = $daemon; + return $this; + } + public function executeTask() { // We do this outside of the try .. catch because we don't have permission // to release the lease otherwise. @@ -138,6 +148,9 @@ $worker = null; try { $worker = $this->getWorkerInstance(); + if ($this->daemon) { + $worker->setDaemon($this->daemon); + } $maximum_failures = $worker->getMaximumRetryCount(); if ($maximum_failures !== null) { diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php --- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php @@ -57,7 +57,6 @@ } final public function getWorkerInstance() { - $id = $this->getID(); $class = $this->getTaskClass(); if (!class_exists($class)) { @@ -70,7 +69,7 @@ "Task class '{$class}' does not extend PhabricatorWorker."); } - return newv($class, array($this->getData())); + return newv($class, array($this, $this->getData())); } }