Skip to content

Commit fb1e159

Browse files
author
epriestley
committed
Fix some issues with empty daemon autoscale pools
Summary: Ref T10811. I'm not sure this causes any real problems, but I caught a couple of errors in the production logs in this vein: ``` [13-Jun-2016 14:26:55 UTC] [2016-06-13 14:26:55] ERROR 8: Undefined index: at [/core/lib/libphutil/src/daemon/PhutilDaemonOverseer.php:302] [13-Jun-2016 14:26:55 UTC] arcanist(head=stable, ref.master=29839e8c72c5, ref.stable=7b0aac5c6f31), libcore(), phabricator(head=stable, ref.stable=b26ecb189e48), phutil(head=stable, ref.master=992abe4a420c, ref.stable=52748950bb36), services(head=stable, ref.master=5b51b63027b7, ref.stable=66cf59af4580) [13-Jun-2016 14:26:55 UTC] #0 PhutilDaemonOverseer::updateAutoscale() called at [<phutil>/src/daemon/PhutilDaemonOverseer.php:224] [13-Jun-2016 14:26:55 UTC] phacility#1 PhutilDaemonOverseer::run() called at [<phabricator>/scripts/daemon/launch_daemon.php:20] [13-Jun-2016 14:26:55 UTC] [2016-06-13 14:26:55] EXCEPTION: (InvalidArgumentException) Argument 1 passed to PhutilDaemonOverseer::getAutoscaleProperty() must be an instance of PhutilDaemonHandle, null given, called in /core/lib/libphutil/src/daemon/PhutilDaemonOverseer.php on line 303 and defined at [<phutil>/src/error/PhutilErrorHandler.php:200] [13-Jun-2016 14:26:55 UTC] arcanist(head=stable, ref.master=29839e8c72c5, ref.stable=7b0aac5c6f31), libcore(), phabricator(head=stable, ref.stable=b26ecb189e48), phutil(head=stable, ref.master=992abe4a420c, ref.stable=52748950bb36), services(head=stable, ref.master=5b51b63027b7, ref.stable=66cf59af4580) [13-Jun-2016 14:26:55 UTC] #0 PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phutil>/src/daemon/PhutilDaemonOverseer.php:278] [13-Jun-2016 14:26:55 UTC] phacility#1 PhutilDaemonOverseer::getAutoscaleProperty(NULL, string, integer) called at [<phutil>/src/daemon/PhutilDaemonOverseer.php:303] [13-Jun-2016 14:26:55 UTC] phacility#2 PhutilDaemonOverseer::updateAutoscale() called at [<phutil>/src/daemon/PhutilDaemonOverseer.php:224] [13-Jun-2016 14:26:55 UTC] phacility#3 PhutilDaemonOverseer::run() called at [<phabricator>/scripts/daemon/launch_daemon.php:20] ``` These are caused by attempting to update empty autoscale pools. Currently, autoscale pools read configuration from the first daemon in the pool, but obviously that doesn't work if the pool is empty. This could occur when shutting down or possibly while reloading daemons. Also, add a diagnostic message bout pool autoscaling. This might be a little too chatty, but maybe useful for some other issues. Test Plan: - A reliable way to hit this was `bin/phd debug task --autoscale`, then `^C` it. It no longer errors. - I used `bin/worker flood` to artificially create a lot of work, and saw the pool autoscale up and down properly. - I launched daemons normally (no autoscale). - I faked daemons as busy and saw the pool autoscale to full. - I changed config and saw daemons automatically reload properly. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10811 Differential Revision: https://secure.phabricator.com/D16110
1 parent a815d2d commit fb1e159

File tree

1 file changed

+34
-14
lines changed

1 file changed

+34
-14
lines changed

Diff for: src/daemon/PhutilDaemonOverseer.php

+34-14
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ final class PhutilDaemonOverseer extends Phobject {
2525
private $lastPidfile;
2626
private $startEpoch;
2727
private $autoscale = array();
28+
private $autoscaleConfig = array();
2829

2930
public function __construct(array $argv) {
3031
PhutilServiceProfiler::getInstance()->enableDiscardMode();
@@ -183,6 +184,18 @@ public function run() {
183184
$daemon->setTraceMemory($this->traceMemory);
184185

185186
$this->addDaemon($daemon, $config);
187+
188+
$group = idx($config['autoscale'], 'group');
189+
if (strlen($group)) {
190+
if (isset($this->autoscaleConfig[$group])) {
191+
throw new Exception(
192+
pht(
193+
'Two daemons are part of the same autoscale group ("%s"). '.
194+
'Each daemon autoscale group must be unique.',
195+
$group));
196+
}
197+
$this->autoscaleConfig[$group] = $config;
198+
}
186199
}
187200

188201
$should_reload = false;
@@ -271,17 +284,14 @@ private function removeDaemon(PhutilDaemonHandle $daemon) {
271284
}
272285

273286
private function getAutoscaleGroup(PhutilDaemonHandle $daemon) {
274-
return $this->getAutoscaleProperty($daemon, 'group');
275-
}
276-
277-
private function getAutoscaleProperty(
278-
PhutilDaemonHandle $daemon,
279-
$key,
280-
$default = null) {
281-
282287
$id = $daemon->getDaemonID();
283288
$autoscale = $this->daemons[$id]['config']['autoscale'];
284-
return idx($autoscale, $key, $default);
289+
return idx($autoscale, 'group');
290+
}
291+
292+
private function getAutoscaleProperty($group_key, $key, $default = null) {
293+
$config = $this->autoscaleConfig[$group_key]['autoscale'];
294+
return idx($config, $key, $default);
285295
}
286296

287297
public function didBeginWork(PhutilDaemonHandle $daemon) {
@@ -298,11 +308,14 @@ public function didBeginIdle(PhutilDaemonHandle $daemon) {
298308
}
299309

300310
public function updateAutoscale() {
311+
if ($this->inGracefulShutdown) {
312+
return;
313+
}
314+
301315
foreach ($this->autoscale as $group => $daemons) {
302-
$daemon = $this->daemons[head_key($daemons)]['handle'];
303-
$scaleup_duration = $this->getAutoscaleProperty($daemon, 'up', 2);
304-
$max_pool_size = $this->getAutoscaleProperty($daemon, 'pool', 8);
305-
$reserve = $this->getAutoscaleProperty($daemon, 'reserve', 0);
316+
$scaleup_duration = $this->getAutoscaleProperty($group, 'up', 2);
317+
$max_pool_size = $this->getAutoscaleProperty($group, 'pool', 8);
318+
$reserve = $this->getAutoscaleProperty($group, 'reserve', 0);
306319

307320
// Don't scale a group if it is already at the maximum pool size.
308321
if (count($daemons) >= $max_pool_size) {
@@ -344,7 +357,7 @@ public function updateAutoscale() {
344357
}
345358

346359
if ($should_scale) {
347-
$config = $this->daemons[$daemon_id]['config'];
360+
$config = $this->autoscaleConfig[$group];
348361

349362
$config['autoscale']['clone'] = true;
350363

@@ -359,6 +372,13 @@ public function updateAutoscale() {
359372
'autoscale' => $config['autoscale'],
360373
));
361374

375+
$this->logMessage(
376+
'AUTO',
377+
pht(
378+
'Scaling pool "%s" up to %s daemon(s).',
379+
$group,
380+
new PhutilNumber(count($daemons) + 1)));
381+
362382
$this->addDaemon($clone, $config);
363383

364384
// Don't scale more than one pool up per iteration. Otherwise, we could

0 commit comments

Comments
 (0)