Skip to content

Commit 65bc481

Browse files
author
epriestley
committedJun 24, 2019
Remove "phd.pid-directory" configuration and stop passing "piddir" to daemons
Summary: Ref T13321. The daemons no longer write PID files, so we no longer need to pass any of this stuff to them. Test Plan: Grepped for affected symbols. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13321 Differential Revision: https://secure.phabricator.com/D20608
1 parent 2498e37 commit 65bc481

File tree

4 files changed

+7
-151
lines changed

4 files changed

+7
-151
lines changed
 

‎src/applications/config/check/PhabricatorExtraConfigSetupCheck.php

+3
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,9 @@ public static function getAncientConfig() {
536536

537537
'differential.whitespace-matters' => pht(
538538
'Whitespace rendering is now handled automatically.'),
539+
540+
'phd.pid-directory' => pht(
541+
'Phabricator daemons no longer use PID files.'),
539542
);
540543

541544
return $ancient_config;

‎src/applications/config/option/PhabricatorPHDConfigOptions.php

-4
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,6 @@ public function getGroup() {
2121

2222
public function getOptions() {
2323
return array(
24-
$this->newOption('phd.pid-directory', 'string', '/var/tmp/phd/pid')
25-
->setLocked(true)
26-
->setDescription(
27-
pht('Directory that phd should use to track running daemons.')),
2824
$this->newOption('phd.log-directory', 'string', '/var/tmp/phd/log')
2925
->setLocked(true)
3026
->setDescription(

‎src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php

+1-15
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,6 @@ final protected function loadAvailableDaemonClasses() {
1212
->selectSymbolsWithoutLoading();
1313
}
1414

15-
final protected function getPIDDirectory() {
16-
$path = PhabricatorEnv::getEnvConfig('phd.pid-directory');
17-
return $this->getControlDirectory($path);
18-
}
19-
2015
final protected function getLogDirectory() {
2116
$path = PhabricatorEnv::getEnvConfig('phd.log-directory');
2217
return $this->getControlDirectory($path);
@@ -30,11 +25,10 @@ private function getControlDirectory($path) {
3025
pht(
3126
"%s requires the directory '%s' to exist, but it does not exist ".
3227
"and could not be created. Create this directory or update ".
33-
"'%s' / '%s' in your configuration to point to an existing ".
28+
"'%s' in your configuration to point to an existing ".
3429
"directory.",
3530
'phd',
3631
$path,
37-
'phd.pid-directory',
3832
'phd.log-directory'));
3933
}
4034
}
@@ -146,14 +140,6 @@ final protected function launchDaemons(
146140
$config['log'] = $this->getLogDirectory().'/daemons.log';
147141
}
148142

149-
$pid_dir = $this->getPIDDirectory();
150-
151-
// TODO: This should be a much better user experience.
152-
Filesystem::assertExists($pid_dir);
153-
Filesystem::assertIsDirectory($pid_dir);
154-
Filesystem::assertWritable($pid_dir);
155-
156-
$config['piddir'] = $pid_dir;
157143
$config['daemons'] = $daemons;
158144

159145
$command = csprintf('./phd-daemon %Ls', $flags);
Original file line numberDiff line numberDiff line change
@@ -1,127 +1,9 @@
11
<?php
22

3-
final class PhabricatorDaemonReference extends Phobject {
4-
5-
private $name;
6-
private $argv;
7-
private $pid;
8-
private $start;
9-
private $pidFile;
10-
11-
private $daemonLog;
12-
13-
public static function loadReferencesFromFile($path) {
14-
$pid_data = Filesystem::readFile($path);
15-
16-
try {
17-
$dict = phutil_json_decode($pid_data);
18-
} catch (PhutilJSONParserException $ex) {
19-
$dict = array();
20-
}
21-
22-
$refs = array();
23-
$daemons = idx($dict, 'daemons', array());
24-
25-
$logs = array();
26-
27-
$daemon_ids = ipull($daemons, 'id');
28-
if ($daemon_ids) {
29-
try {
30-
$logs = id(new PhabricatorDaemonLogQuery())
31-
->setViewer(PhabricatorUser::getOmnipotentUser())
32-
->withDaemonIDs($daemon_ids)
33-
->execute();
34-
} catch (AphrontQueryException $ex) {
35-
// Ignore any issues here; getting this information only allows us
36-
// to provide a more complete picture of daemon status, and we want
37-
// these commands to work if the database is inaccessible.
38-
}
39-
40-
$logs = mpull($logs, null, 'getDaemonID');
41-
}
42-
43-
// Support PID files that use the old daemon format, where each overseer
44-
// had exactly one daemon. We can eventually remove this; they will still
45-
// be stopped by `phd stop --force` even if we don't identify them here.
46-
if (!$daemons && idx($dict, 'name')) {
47-
$daemons = array(
48-
array(
49-
'config' => array(
50-
'class' => idx($dict, 'name'),
51-
'argv' => idx($dict, 'argv', array()),
52-
),
53-
),
54-
);
55-
}
56-
57-
foreach ($daemons as $daemon) {
58-
$ref = new PhabricatorDaemonReference();
59-
60-
// NOTE: This is the overseer PID, not the actual daemon process PID.
61-
// This is correct for checking status and sending signals (the only
62-
// things we do with it), but might be confusing. $daemon['pid'] has
63-
// the daemon PID, and we could expose that if we had some use for it.
64-
65-
$ref->pid = idx($dict, 'pid');
66-
$ref->start = idx($dict, 'start');
67-
68-
$config = idx($daemon, 'config', array());
69-
$ref->name = idx($config, 'class');
70-
$ref->argv = idx($config, 'argv', array());
71-
72-
$log = idx($logs, idx($daemon, 'id'));
73-
if ($log) {
74-
$ref->daemonLog = $log;
75-
}
76-
77-
$ref->pidFile = $path;
78-
$refs[] = $ref;
79-
}
80-
81-
return $refs;
82-
}
3+
// TODO: See T13321. After the removal of daemon PID files this class
4+
// no longer makes as much sense as it once did.
835

84-
public function updateStatus($new_status) {
85-
if (!$this->daemonLog) {
86-
return;
87-
}
88-
89-
try {
90-
$this->daemonLog
91-
->setStatus($new_status)
92-
->save();
93-
} catch (AphrontQueryException $ex) {
94-
// Ignore anything that goes wrong here.
95-
}
96-
}
97-
98-
public function getPID() {
99-
return $this->pid;
100-
}
101-
102-
public function getName() {
103-
return $this->name;
104-
}
105-
106-
public function getArgv() {
107-
return $this->argv;
108-
}
109-
110-
public function getEpochStarted() {
111-
return $this->start;
112-
}
113-
114-
public function getPIDFile() {
115-
return $this->pidFile;
116-
}
117-
118-
public function getDaemonLog() {
119-
return $this->daemonLog;
120-
}
121-
122-
public function isRunning() {
123-
return self::isProcessRunning($this->getPID());
124-
}
6+
final class PhabricatorDaemonReference extends Phobject {
1257

1268
public static function isProcessRunning($pid) {
1279
if (!$pid) {
@@ -148,15 +30,4 @@ public static function isProcessRunning($pid) {
14830
return $is_running;
14931
}
15032

151-
public function waitForExit($seconds) {
152-
$start = time();
153-
while (time() < $start + $seconds) {
154-
usleep(100000);
155-
if (!$this->isRunning()) {
156-
return true;
157-
}
158-
}
159-
return !$this->isRunning();
160-
}
161-
16233
}

0 commit comments

Comments
 (0)
Failed to load comments.