recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Don't fatal on daemon status updates from `phd`

Summary:
See D3126, T1667, T1658. Prior to D3126, `phd` did not use MySQL directly. Now that it does, there are at least two specific problems (see inline comment).

In the long term, we should probably break this dependency and use Conduit. However, we don't currently have access to the daemon log ID and getting it is a mess (the overseer generates it), and I think I want to rewrite how all this works at some point anyway (the daemon calls are currently completely unauthenticated, which is silly -- we should move them to an authenticated channel at some point, I think).

Test Plan: Ran `phd stop` with a bad MySQL config against a non-running daemon, didn't get a query error.

Reviewers: nh, vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1667, T1658

Differential Revision: https://secure.phabricator.com/D3314

epriestley 5342bb10 2a815e07

+28 -19
+2 -11
src/infrastructure/daemon/PhabricatorDaemonControl.php
··· 53 53 foreach ($daemons as $daemon) { 54 54 $name = $daemon->getName(); 55 55 if (!$daemon->isRunning()) { 56 - $daemon_log = $daemon->loadDaemonLog(); 57 - if ($daemon_log) { 58 - $daemon_log->setStatus(PhabricatorDaemonLog::STATUS_DEAD); 59 - $daemon_log->save(); 60 - } 61 - 56 + $daemon->updateStatus(PhabricatorDaemonLog::STATUS_DEAD); 62 57 $status = 2; 63 58 $name = '<DEAD> '.$name; 64 59 } ··· 116 111 if (!$daemon->isRunning()) { 117 112 echo "Daemon is not running.\n"; 118 113 unset($running[$key]); 119 - $daemon_log = $daemon->loadDaemonLog(); 120 - if ($daemon_log) { 121 - $daemon_log->setStatus(PhabricatorDaemonLog::STATUS_EXITED); 122 - $daemon_log->save(); 123 - } 114 + $daemon->updateStatus(PhabricatorDaemonLog::STATUS_EXITED); 124 115 } else { 125 116 posix_kill($pid, SIGINT); 126 117 }
+26 -8
src/infrastructure/daemon/control/PhabricatorDaemonReference.php
··· 35 35 return $ref; 36 36 } 37 37 38 - public function loadDaemonLog() { 39 - if (!$this->daemonLog) { 40 - $this->daemonLog = id(new PhabricatorDaemonLog())->loadOneWhere( 41 - 'daemon = %s AND pid = %d AND dateCreated = %d', 42 - $this->name, 43 - $this->pid, 44 - $this->start); 38 + public function updateStatus($new_status) { 39 + try { 40 + if (!$this->daemonLog) { 41 + $this->daemonLog = id(new PhabricatorDaemonLog())->loadOneWhere( 42 + 'daemon = %s AND pid = %d AND dateCreated = %d', 43 + $this->name, 44 + $this->pid, 45 + $this->start); 46 + } 47 + 48 + if ($this->daemonLog) { 49 + $this->daemonLog 50 + ->setStatus($new_status) 51 + ->save(); 52 + } 53 + } catch (AphrontQueryException $ex) { 54 + // Ignore anything that goes wrong here. We anticipate at least two 55 + // specific failure modes: 56 + // 57 + // - Upgrade scripts which run `git pull`, then `phd stop`, then 58 + // `bin/storage upgrade` will fail when trying to update the `status` 59 + // column, as it does not exist yet. 60 + // - Daemons running on machines which do not have access to MySQL 61 + // (like an IRC bot) will not be able to load or save the log. 62 + // 63 + // 45 64 } 46 - return $this->daemonLog; 47 65 } 48 66 49 67 public function getPID() {