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

Do initial repository imports at a lower priority and finish importing commits before starting new ones

Summary:
Fixes T11677. This makes two minor adjustments to the repository import daemons:

  - The first step ("Message") now queues at a slightly-lower-than-default (for already-imported repositories) or very-low (for newly importing repositories) priority level.
  - The other steps now queue at "default" priority level. This is actually what they already did, but without this change their behavior would be to inherit the priority level of their parents.

This has two effects:

  - When adding new repositories to an existing install, they shouldn't block other things from happening anymore.
  - The daemons will tend to start one commit and run through all of its steps before starting another commit. This makes progress through the queue more even and predictable.
    - Before, they did ALL the message tasks, then ALL the change tasks, etc. This works fine but is confusing/uneven/less-predictable because each type of task takes a different amount of time.

Test Plan:
  - Added a new repository.
  - Saw all of its "message" steps queue at priority 4000.
  - Saw followups queue at priority 2000.
  - Saw progress generally "finish what you started" -- go through the queue one commit at a time, instead of one type of task at a time.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11677

Differential Revision: https://secure.phabricator.com/D16585
This commit is contained in:
epriestley 2016-09-21 15:16:14 -07:00
parent 8941bbfcea
commit db2425b300
3 changed files with 35 additions and 1 deletions

View file

@ -682,7 +682,32 @@ final class PhabricatorRepositoryDiscoveryEngine
$data['commitID'] = $commit->getID();
PhabricatorWorker::scheduleTask($class, $data);
// If the repository is importing for the first time, we schedule tasks
// at IMPORT priority, which is very low. Making progress on importing a
// new repository for the first time is less important than any other
// daemon task.
// If the repostitory has finished importing and we're just catching up
// on recent commits, we schedule discovery at COMMIT priority, which is
// slightly below the default priority.
// Note that followup tasks and triggered tasks (like those generated by
// Herald or Harbormaster) will queue at DEFAULT priority, so that each
// commit tends to fully import before we start the next one. This tends
// to give imports fairly predictable progress. See T11677 for some
// discussion.
if ($repository->isImporting()) {
$task_priority = PhabricatorWorker::PRIORITY_IMPORT;
} else {
$task_priority = PhabricatorWorker::PRIORITY_COMMIT;
}
$options = array(
'priority' => $task_priority,
);
PhabricatorWorker::scheduleTask($class, $data, $options);
}
private function isInitialImport(array $refs) {

View file

@ -47,6 +47,14 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
$this->getFollowupTaskClass(),
array(
'commitID' => $commit->getID(),
),
array(
// We queue followup tasks at default priority so that the queue
// finishes work it has started before starting more work. If
// followups are queued at the same priority level, we do all
// message parses first, then all change parses, etc. This makes
// progress uneven. See T11677 for discussion.
'priority' => PhabricatorWorker::PRIORITY_DEFAULT,
));
}
}

View file

@ -16,6 +16,7 @@ abstract class PhabricatorWorker extends Phobject {
const PRIORITY_ALERTS = 1000;
const PRIORITY_DEFAULT = 2000;
const PRIORITY_COMMIT = 2500;
const PRIORITY_BULK = 3000;
const PRIORITY_IMPORT = 4000;