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.

Do not perform write in PhabricatorDaemonLogQuery by default

Summary:
See <http://github.com/facebook/phabricator/issues/487>. By default, we perform a write in this query to moved daemons to "dead" status after a timeout. This is normally reasonable, but after D7964 we do a setup check against the daemons, which means this query is invoked very early in the stack, before we have a write guard.

Since doing this write unconditionally is unnecessarily, surprising, and overly ambitious, make the write conditional and do not attempt to perform it from the setup check.

(We could also move this to a GC/cron sort of thing eventually, maybe -- it's a bit awkward here, but we don't have other infrastructure which is a great fit right now.)

Test Plan: Hit setup issues and daemon pages. Will confirm with user that this fixes things.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

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

epriestley 136af8d2 56bcb33a

+19 -4
+1
src/applications/daemon/controller/PhabricatorDaemonConsoleController.php
··· 73 73 $logs = id(new PhabricatorDaemonLogQuery()) 74 74 ->setViewer($user) 75 75 ->withStatus(PhabricatorDaemonLogQuery::STATUS_ALIVE) 76 + ->setAllowStatusWrites(true) 76 77 ->execute(); 77 78 78 79 $taskmasters = 0;
+1
src/applications/daemon/controller/PhabricatorDaemonLogListController.php
··· 12 12 13 13 $logs = id(new PhabricatorDaemonLogQuery()) 14 14 ->setViewer($viewer) 15 + ->setAllowStatusWrites(true) 15 16 ->executeWithCursorPager($pager); 16 17 17 18 $daemon_table = new PhabricatorDaemonLogListView();
+1
src/applications/daemon/controller/PhabricatorDaemonLogViewController.php
··· 16 16 $log = id(new PhabricatorDaemonLogQuery()) 17 17 ->setViewer($user) 18 18 ->withIDs(array($this->id)) 19 + ->setAllowStatusWrites(true) 19 20 ->executeOne(); 20 21 if (!$log) { 21 22 return new Aphront404Response();
+1
src/applications/daemon/management/PhabricatorDaemonManagementLogWorkflow.php
··· 34 34 $daemon = id(new PhabricatorDaemonLogQuery()) 35 35 ->setViewer($this->getViewer()) 36 36 ->withIDs(array($id)) 37 + ->setAllowStatusWrites(true) 37 38 ->executeOne(); 38 39 39 40 if (!$daemon) {
+15 -4
src/applications/daemon/query/PhabricatorDaemonLogQuery.php
··· 9 9 private $ids; 10 10 private $status = self::STATUS_ALL; 11 11 private $daemonClasses; 12 + private $allowStatusWrites; 12 13 13 14 public static function getTimeUntilUnknown() { 14 15 return 3 * PhutilDaemonOverseer::HEARTBEAT_WAIT; ··· 30 31 31 32 public function withDaemonClasses(array $classes) { 32 33 $this->daemonClasses = $classes; 34 + return $this; 35 + } 36 + 37 + public function setAllowStatusWrites($allow) { 38 + $this->allowStatusWrites = $allow; 33 39 return $this; 34 40 } 35 41 ··· 80 86 $status = $status_dead; 81 87 } 82 88 83 - // If we changed the daemon's status, update it. 89 + // If we changed the daemon's status, adjust it. 84 90 if ($status != $daemon->getStatus()) { 85 - $guard = AphrontWriteGuard::beginScopedUnguardedWrites(); 86 - $daemon->setStatus($status)->save(); 87 - unset($guard); 91 + $daemon->setStatus($status); 92 + 93 + // ...and write it, if we're in a context where that's reasonable. 94 + if ($this->allowStatusWrites) { 95 + $guard = AphrontWriteGuard::beginScopedUnguardedWrites(); 96 + $daemon->save(); 97 + unset($guard); 98 + } 88 99 } 89 100 90 101 // If the daemon no longer matches the filter, get rid of it.