Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T7053: Restart daemons automagically
- Commits
- Restricted Diffusion Commit
rPa07a8aca2462: Add a daemon overseer module to restart daemons when config changes
Created a custom daemon which dumps out the config hash (by querying PhabricatorEnv::calculateEnvironmentHash()). Ran this daemon with ./bin/phd debug PhabricatorDebugDaemon and saw the config hash update within 30 seconds.
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
I think we should a timer in this so it only queries if the last query was more than, say, 10 seconds ago. Looks good otherwise.
| src/infrastructure/daemon/overseer/PhabricatorDaemonOverseerModule.php | ||
|---|---|---|
| 39 | Prefer new PhabricatorConfigEntryTransaction()->getTableName() | |
| src/infrastructure/daemon/overseer/PhabricatorDaemonOverseerModule.php | ||
|---|---|---|
| 16–19 | Oh, we also shouldn't issue a restart immediately upon startup. If the current version hasn't been set yet, just load it without issuing a restart. | |
Although maybe it would be better not to do it as a side effect of construction, given that we're PhutilClassMapQuery'ing this thing. Running queries inside PhutilClassMapQuery feels a little iffy.
Yeah agreed, I'll change it.
| src/infrastructure/daemon/overseer/PhabricatorDaemonOverseerModule.php | ||
|---|---|---|
| 16–19 | We won't, that's why I query the version in the constructor. | |
| src/infrastructure/daemon/overseer/PhabricatorDaemonOverseerModule.php | ||
|---|---|---|
| 11–15 | Prefer PhabricatorTime::getNow() so we can muck with it in unit tests if we ever need to. | |
| 32 | Consider load over get to hint at the query. | |
| 43 | I think this actually has an initialization problem? First time through we'll issue a reload? | |
| 45 | We should always update the timestamp -- otherwise we'll query after 10s, then after 1s, 1s, 1s, 1s, ... | |