From a0f0ba6acd47dbe64f43f8474f4b6ef5d0e9d48c Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 12 Aug 2013 11:20:22 -0700 Subject: [PATCH] Stop using process/filesystem-based checks to determine if daemons are running Summary: We currently check if daemons are running using the filesystem and process list. These checks reach the wrong result for a lot of users because their webservers can't read the filesystem or process list. They also reach the wrong result for daemons running on other machines. Instead, query the active daemon list to see if daemons are running. This should be significantly more reliable. (We didn't do this before because the running daemon list mechanism didn't exist when the check was written, and at the time it was more complex than doing a simple filesystem/process list thing.) Test Plan: Viewed `/repositories/` with and without daemons running, saw appropriate warning or lack of warning. Reviewers: btrahan Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D6722 --- .../query/PhabricatorDaemonLogQuery.php | 13 ++++++ .../PhabricatorRepositoryController.php | 41 +++++++------------ 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/applications/daemon/query/PhabricatorDaemonLogQuery.php b/src/applications/daemon/query/PhabricatorDaemonLogQuery.php index bb00f9ff85..448c6f8927 100644 --- a/src/applications/daemon/query/PhabricatorDaemonLogQuery.php +++ b/src/applications/daemon/query/PhabricatorDaemonLogQuery.php @@ -8,6 +8,7 @@ final class PhabricatorDaemonLogQuery private $ids; private $status = self::STATUS_ALL; + private $daemonClasses; public static function getTimeUntilUnknown() { return 3 * PhutilDaemonOverseer::HEARTBEAT_WAIT; @@ -27,6 +28,11 @@ final class PhabricatorDaemonLogQuery return $this; } + public function withDaemonClasses(array $classes) { + $this->daemonClasses = $classes; + return $this; + } + public function loadPage() { $table = new PhabricatorDaemonLog(); $conn_r = $table->establishConnection('r'); @@ -109,6 +115,13 @@ final class PhabricatorDaemonLogQuery $this->getStatusConstants()); } + if ($this->daemonClasses) { + $where[] = qsprintf( + $conn_r, + 'daemon IN (%Ls)', + $this->daemonClasses); + } + $where[] = $this->buildPagingClause($conn_r); return $this->formatWhereClause($where); } diff --git a/src/applications/repository/controller/PhabricatorRepositoryController.php b/src/applications/repository/controller/PhabricatorRepositoryController.php index 20b33dc924..ba5bfacc23 100644 --- a/src/applications/repository/controller/PhabricatorRepositoryController.php +++ b/src/applications/repository/controller/PhabricatorRepositoryController.php @@ -22,15 +22,14 @@ abstract class PhabricatorRepositoryController extends PhabricatorController { } private function isPullDaemonRunning() { - // TODO: This is yuck, fix it. - $control = new PhabricatorDaemonManagementListWorkflow(); - $daemons = $control->loadRunningDaemons(); - foreach ($daemons as $daemon) { - if ($daemon->isRunning() && - $daemon->getName() == 'PhabricatorRepositoryPullLocalDaemon') - return true; - } - return false; + $daemons = id(new PhabricatorDaemonLogQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withStatus(PhabricatorDaemonLogQuery::STATUS_ALIVE) + ->withDaemonClasses(array('PhabricatorRepositoryPullLocalDaemon')) + ->setLimit(1) + ->execute(); + + return (bool)$daemons; } protected function renderDaemonNotice() { @@ -47,24 +46,14 @@ abstract class PhabricatorRepositoryController extends PhabricatorController { "repositories. For instructions on starting the daemon, see %s.", phutil_tag('strong', array(), $documentation)); - try { - $daemon_running = $this->isPullDaemonRunning(); - if ($daemon_running) { - return null; - } - $title = "Repository Daemon Not Running"; - $message = hsprintf( - "

The repository daemon is not running on this machine. %s

", - $common); - } catch (Exception $ex) { - $title = "Unable To Verify Repository Daemon"; - $message = hsprintf( - "

Unable to determine if the repository daemon is running on this ". - "machine. %s

". - "

Exception: %s

", - $common, - $ex->getMessage()); + $daemon_running = $this->isPullDaemonRunning(); + if ($daemon_running) { + return null; } + $title = "Repository Daemon Not Running"; + $message = hsprintf( + "

The repository daemon is not running. %s

", + $common); $view = new AphrontErrorView(); $view->setSeverity(AphrontErrorView::SEVERITY_WARNING);