1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

Free task leases on "phd start"

Summary:
Fixes T5154. Currently, "phd stop" terminates daemons relatively abruptly (and other things do too, like killing them). This can leave them with long leases that won't expire any time soon. Normally this isn't a big deal, since it just means an email or an import takes a bit longer (often 2 hours, but up to 24 hours) to run. However:

  - We've increased default lease durations a lot fairly recently -- the 2 hours used to be 15 minutes.
  - Harbormaster and Drydock add new types of tasks which are more dependent on other tasks, so waiting 2 hours for something to free up can hold up more stuff in queue.

When `phd start` is run, we can be confident (at least, in normal circumstances) that leases are safe to free, since we do a check. This undoes any damage done by abrupt stops in "phd stop" or by users or systems killing stuff.

(It would be nice to make "phd stop" more graceful at some point, but we always have to deal with abrupt termination in some cases no matter how gentle "phd stop" is.)

One sort-of-questionable thing here is that we don't distinguish between tasks which had an active lease and tasks which had been released, since the system itself does not make a distiction. So, for example, if you have a task that retries 5 times and waits an hour between retries, you'll get a retry on every `phd start` now, and could exhaust them all in a few minutes if you cycle `phd start` aggressively. I think this is OK. In the future, we could try to distinguish between these types of tasks, and only free the ones with active leases.

Test Plan:
  - Used `phd start` normally, saw it free leases.
  - Used `phd start`, killed it real quick so no taskmasters spawned, ran it again an saw no leases freed.
  - Used `phd start --keep-leases`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5154

Differential Revision: https://secure.phabricator.com/D9256
This commit is contained in:
epriestley 2014-05-22 10:40:45 -07:00
parent 890ae77a9a
commit 5e7b316fbe
2 changed files with 32 additions and 3 deletions

View file

@ -11,11 +11,19 @@ final class PhabricatorDaemonManagementStartWorkflow
'Start the standard configured collection of Phabricator daemons. '.
'This is appropriate for most installs. Use **phd launch** to '.
'customize which daemons are launched.'))
->setArguments(array());
->setArguments(
array(
array(
'name' => 'keep-leases',
'help' => pht(
'By default, **phd start** will free all task leases held by '.
'the daemons. With this flag, this step will be skipped.'),
),
));
}
public function execute(PhutilArgumentParser $args) {
return $this->executeStartCommand();
return $this->executeStartCommand($args->getArg('keep-leases'));
}
}

View file

@ -224,7 +224,7 @@ abstract class PhabricatorDaemonManagementWorkflow
/* -( Commands )----------------------------------------------------------- */
protected function executeStartCommand() {
protected function executeStartCommand($keep_leases = false) {
$console = PhutilConsole::getConsole();
$running = $this->loadRunningDaemons();
@ -246,6 +246,16 @@ abstract class PhabricatorDaemonManagementWorkflow
}
}
if ($keep_leases) {
$console->writeErr("%s\n", pht('Not touching active task queue leases.'));
} else {
$console->writeErr("%s\n", pht('Freeing active task leases...'));
$count = $this->freeActiveLeases();
$console->writeErr(
"%s\n",
pht('Freed %s task lease(s).', new PhutilNumber($count)));
}
$daemons = array(
array('PhabricatorRepositoryPullLocalDaemon', array()),
array('PhabricatorGarbageCollectorDaemon', array()),
@ -352,4 +362,15 @@ abstract class PhabricatorDaemonManagementWorkflow
return 0;
}
private function freeActiveLeases() {
$task_table = id(new PhabricatorWorkerActiveTask());
$conn_w = $task_table->establishConnection('w');
queryfx(
$conn_w,
'UPDATE %T SET leaseExpires = UNIX_TIMESTAMP()
WHERE leaseExpires > UNIX_TIMESTAMP()',
$task_table->getTableName());
return $conn_w->getAffectedRows();
}
}