From af303f458b9c2924b2f7d4c62f8fc55b7e58492a Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 23 Feb 2015 17:58:01 -0800 Subject: [PATCH] Convert taskmasters to use an autoscale pool Summary: Ref T7352. This is pretty straightforward. I renamed `phd.start-taskmasters` to `phd.taskmasters` for clarity. Test Plan: - Ran `phd start`, `phd start --autoscale-reserve 0.25`, `phd restart --autoscale-reserve 0.25`, etc. - Examined PID file to see options were passed. - I'm defaulting this off (0 reserve) and making it a flag rather than an option because it's a very advanced feature which is probably not useful outside of instancing. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T7352 Differential Revision: https://secure.phabricator.com/D11871 --- .../PhabricatorExtraConfigSetupCheck.php | 3 ++ .../option/PhabricatorPHDConfigOptions.php | 10 ++-- ...ricatorDaemonManagementRestartWorkflow.php | 6 ++- ...abricatorDaemonManagementStartWorkflow.php | 8 ++- .../PhabricatorDaemonManagementWorkflow.php | 49 ++++++++++++++----- .../configuration/managing_daemons.diviner | 5 +- .../workers/PhabricatorTaskmasterDaemon.php | 22 ++------- 7 files changed, 64 insertions(+), 39 deletions(-) diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index 29e6752bc1..78805c218d 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -208,6 +208,9 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'longer used or supported.'), 'config.mask' => pht( 'Use `config.hide` instead of this option.'), + 'phd.start-taskmasters' => pht( + 'Taskmasters now use an autoscaling pool. You can configure the '. + 'pool size with `phd.taskmasters`.'), ); return $ancient_config; diff --git a/src/applications/config/option/PhabricatorPHDConfigOptions.php b/src/applications/config/option/PhabricatorPHDConfigOptions.php index ee240d3916..3fb0afdc57 100644 --- a/src/applications/config/option/PhabricatorPHDConfigOptions.php +++ b/src/applications/config/option/PhabricatorPHDConfigOptions.php @@ -29,13 +29,13 @@ final class PhabricatorPHDConfigOptions ->setDescription( pht( 'Directory that the daemons should use to store log files.')), - $this->newOption('phd.start-taskmasters', 'int', 4) - ->setSummary(pht('Number of TaskMaster daemons to start by default.')) + $this->newOption('phd.taskmasters', 'int', 4) + ->setSummary(pht('Maximum taskmaster daemon pool size.')) ->setDescription( pht( - "Number of 'TaskMaster' daemons that 'phd start' should start. ". - "You can raise this if you have a task backlog, or explicitly ". - "launch more with 'phd launch taskmaster'.")), + 'Maximum number of taskmaster daemons to run at once. Raising '. + 'this can increase the maximum throughput of the task queue. The '. + 'pool will automatically scale down when unutilized.')), $this->newOption('phd.verbose', 'bool', false) ->setBoolOptions( array( diff --git a/src/applications/daemon/management/PhabricatorDaemonManagementRestartWorkflow.php b/src/applications/daemon/management/PhabricatorDaemonManagementRestartWorkflow.php index cbf02f161c..5d8d6df89e 100644 --- a/src/applications/daemon/management/PhabricatorDaemonManagementRestartWorkflow.php +++ b/src/applications/daemon/management/PhabricatorDaemonManagementRestartWorkflow.php @@ -31,6 +31,7 @@ final class PhabricatorDaemonManagementRestartWorkflow 'Also stop running processes that look like daemons but do '. 'not have corresponding PID files.'), ), + $this->getAutoscaleReserveArgument(), )); } @@ -46,7 +47,10 @@ final class PhabricatorDaemonManagementRestartWorkflow return $err; } - return $this->executeStartCommand(); + return $this->executeStartCommand( + array( + 'reserve' => (float)$args->getArg('autoscale-reserve', 0.0), + )); } } diff --git a/src/applications/daemon/management/PhabricatorDaemonManagementStartWorkflow.php b/src/applications/daemon/management/PhabricatorDaemonManagementStartWorkflow.php index 7826dde662..d489dca88b 100644 --- a/src/applications/daemon/management/PhabricatorDaemonManagementStartWorkflow.php +++ b/src/applications/daemon/management/PhabricatorDaemonManagementStartWorkflow.php @@ -24,13 +24,17 @@ final class PhabricatorDaemonManagementStartWorkflow 'help' => pht( 'Start daemons even if daemons are already running.'), ), + $this->getAutoscaleReserveArgument(), )); } public function execute(PhutilArgumentParser $args) { return $this->executeStartCommand( - $args->getArg('keep-leases'), - $args->getArg('force')); + array( + 'keep-leases' => $args->getArg('keep-leases'), + 'force' => $args->getArg('force'), + 'reserve' => (float)$args->getArg('autoscale-reserve', 0.0), + )); } } diff --git a/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php b/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php index 5e719532c1..6d523c76fd 100644 --- a/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php +++ b/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php @@ -290,10 +290,18 @@ abstract class PhabricatorDaemonManagementWorkflow /* -( Commands )----------------------------------------------------------- */ - protected final function executeStartCommand($keep_leases, $force) { + protected final function executeStartCommand(array $options) { + PhutilTypeSpec::checkMap( + $options, + array( + 'keep-leases' => 'optional bool', + 'force' => 'optional bool', + 'reserve' => 'optional float', + )); + $console = PhutilConsole::getConsole(); - if (!$force) { + if (!idx($options, 'force')) { $running = $this->loadRunningDaemons(); // This may include daemons which were launched but which are no longer @@ -315,7 +323,7 @@ abstract class PhabricatorDaemonManagementWorkflow } } - if ($keep_leases) { + if (idx($options, 'keep-leases')) { $console->writeErr("%s\n", pht('Not touching active task queue leases.')); } else { $console->writeErr("%s\n", pht('Freeing active task leases...')); @@ -335,14 +343,15 @@ abstract class PhabricatorDaemonManagementWorkflow array( 'class' => 'PhabricatorTriggerDaemon', ), - ); - - $taskmasters = PhabricatorEnv::getEnvConfig('phd.start-taskmasters'); - for ($ii = 0; $ii < $taskmasters; $ii++) { - $daemons[] = array( + array( 'class' => 'PhabricatorTaskmasterDaemon', - ); - } + 'autoscale' => array( + 'group' => 'task', + 'pool' => PhabricatorEnv::getEnvConfig('phd.taskmasters'), + 'reserve' => idx($options, 'reserve', 0), + ), + ), + ); $this->launchDaemons($daemons, $is_debug = false); @@ -577,7 +586,13 @@ abstract class PhabricatorDaemonManagementWorkflow foreach ($daemons as $daemon) { $is_autoscale = isset($daemon['autoscale']['group']); if ($is_autoscale) { - $autoscale = pht('(Autoscaling)'); + $autoscale = $daemon['autoscale']; + foreach ($autoscale as $key => $value) { + $autoscale[$key] = $key.'='.$value; + } + $autoscale = implode(', ', $autoscale); + + $autoscale = pht('(Autoscaling: %s)', $autoscale); } else { $autoscale = pht('(Static)'); } @@ -591,4 +606,16 @@ abstract class PhabricatorDaemonManagementWorkflow $console->writeOut("\n"); } + protected function getAutoscaleReserveArgument() { + return array( + 'name' => 'autoscale-reserve', + 'param' => 'ratio', + 'help' => pht( + 'Specify a proportion of machine memory which must be free '. + 'before autoscale pools will grow. For example, a value of 0.25 '. + 'means that pools will not grow unless the machine has at least '. + '25%% of its RAM free.'), + ); + } + } diff --git a/src/docs/user/configuration/managing_daemons.diviner b/src/docs/user/configuration/managing_daemons.diviner index 988051ecc8..e98af36a24 100644 --- a/src/docs/user/configuration/managing_daemons.diviner +++ b/src/docs/user/configuration/managing_daemons.diviner @@ -99,8 +99,9 @@ This daemon will daemonize and run normally. == General Tips == - - You can set the number of taskmasters that `phd start` starts by the config - key `phd.start-taskmasters`. If you have a task backlog, try increasing it. + - You can set the maximum number of taskmasters that will run at once + by adjusting `phd.taskmasters`. If you have a task backlog, try increasing + it. - When you `phd launch` or `phd debug` a daemon, you can type any unique substring of its name, so `phd launch pull` will work correctly. - `phd stop` and `phd restart` stop **all** of the daemons on the machine, not diff --git a/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php b/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php index f48194bb36..40606c3d61 100644 --- a/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php +++ b/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php @@ -3,9 +3,6 @@ final class PhabricatorTaskmasterDaemon extends PhabricatorDaemon { protected function run() { - $taskmaster_count = PhabricatorEnv::getEnvConfig('phd.start-taskmasters'); - $offset = mt_rand(0, $taskmaster_count - 1); - do { $tasks = id(new PhabricatorWorkerLeaseQuery()) ->setLimit(1) @@ -44,22 +41,11 @@ final class PhabricatorTaskmasterDaemon extends PhabricatorDaemon { $sleep = 0; } else { - // When there's no work, sleep for as many seconds as there are - // active taskmasters. - - // On average, this starts tasks added to an empty queue after one - // second. This keeps responsiveness high even on small instances - // without much work to do. - - // It also means an empty queue has an average load of one query - // per second even if there are a very large number of taskmasters - // launched. - - // The first time we sleep, we add a random offset to try to spread - // the sleep times out somewhat evenly. + // When there's no work, sleep for one second. The pool will + // autoscale down if we're continuously idle for an extended period + // of time. $this->willBeginIdle(); - $sleep = $taskmaster_count + $offset; - $offset = 0; + $sleep = 1; } $this->sleep($sleep);