Page MenuHomePhabricator
Files F13184857

D21516.diff
No OneTemporary
Actions

D21516.diff
View Options

diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
--- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
@@ -655,7 +655,12 @@
$epoch,
$task_priority) {
- $this->insertTask($repository, $commit, $task_priority);
+ $this->queueCommitImportTask(
+ $repository,
+ $commit->getID(),
+ $commit->getPHID(),
+ $task_priority,
+ $via = 'discovery');
// Update the repository summary table.
queryfx(
@@ -679,36 +684,6 @@
}
}
- private function insertTask(
- PhabricatorRepository $repository,
- PhabricatorRepositoryCommit $commit,
- $task_priority,
- $data = array()) {
-
- $vcs = $repository->getVersionControlSystem();
- switch ($vcs) {
- case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
- $class = 'PhabricatorRepositoryGitCommitMessageParserWorker';
- break;
- case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
- $class = 'PhabricatorRepositorySvnCommitMessageParserWorker';
- break;
- case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
- $class = 'PhabricatorRepositoryMercurialCommitMessageParserWorker';
- break;
- default:
- throw new Exception(pht("Unknown repository type '%s'!", $vcs));
- }
-
- $data['commitID'] = $commit->getID();
-
- $options = array(
- 'priority' => $task_priority,
- );
-
- PhabricatorWorker::scheduleTask($class, $data, $options);
- }
-
private function isInitialImport(array $refs) {
$commit_count = count($refs);
@@ -926,71 +901,4 @@
$data['epoch']);
}
- private function getImportTaskPriority(
- PhabricatorRepository $repository,
- array $refs) {
-
- // 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 repository has finished importing and we're just catching up
- // on recent commits, we usually 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()) {
- $this->log(
- pht(
- 'Importing %s commit(s) at low priority ("PRIORITY_IMPORT") '.
- 'because this repository is still importing.',
- phutil_count($refs)));
-
- return PhabricatorWorker::PRIORITY_IMPORT;
- }
-
- // See T13369. If we've discovered a lot of commits at once, import them
- // at lower priority.
-
- // This is mostly aimed at reducing the impact that synchronizing thousands
- // of commits from a remote upstream has on other repositories. The queue
- // is "mostly FIFO", so queueing a thousand commit imports can stall other
- // repositories.
-
- // In a perfect world we'd probably give repositories round-robin queue
- // priority, but we don't currently have the primitives for this and there
- // isn't a strong case for building them.
-
- // Use "a whole lot of commits showed up at once" as a heuristic for
- // detecting "someone synchronized an upstream", and import them at a lower
- // priority to more closely approximate fair scheduling.
-
- if (count($refs) >= PhabricatorRepository::LOWPRI_THRESHOLD) {
- $this->log(
- pht(
- 'Importing %s commit(s) at low priority ("PRIORITY_IMPORT") '.
- 'because many commits were discovered at once.',
- phutil_count($refs)));
-
- return PhabricatorWorker::PRIORITY_IMPORT;
- }
-
- // Otherwise, import at normal priority.
-
- if ($refs) {
- $this->log(
- pht(
- 'Importing %s commit(s) at normal priority ("PRIORITY_COMMIT").',
- phutil_count($refs)));
- }
-
- return PhabricatorWorker::PRIORITY_COMMIT;
- }
-
}
diff --git a/src/applications/repository/engine/PhabricatorRepositoryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryEngine.php
--- a/src/applications/repository/engine/PhabricatorRepositoryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryEngine.php
@@ -83,4 +83,114 @@
return $this;
}
+ final protected function queueCommitImportTask(
+ PhabricatorRepository $repository,
+ $commit_id,
+ $commit_phid,
+ $task_priority,
+ $via = null) {
+
+ $vcs = $repository->getVersionControlSystem();
+ switch ($vcs) {
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+ $class = 'PhabricatorRepositoryGitCommitMessageParserWorker';
+ break;
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+ $class = 'PhabricatorRepositorySvnCommitMessageParserWorker';
+ break;
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+ $class = 'PhabricatorRepositoryMercurialCommitMessageParserWorker';
+ break;
+ default:
+ throw new Exception(
+ pht(
+ 'Unknown repository type "%s"!',
+ $vcs));
+ }
+
+ $data = array(
+ 'commitID' => $commit_id,
+ );
+
+ if ($via !== null) {
+ $data['via'] = $via;
+ }
+
+ $options = array(
+ 'priority' => $task_priority,
+ 'objectPHID' => $commit_phid,
+ );
+
+ PhabricatorWorker::scheduleTask($class, $data, $options);
+ }
+
+ final protected function getImportTaskPriority(
+ PhabricatorRepository $repository,
+ array $refs) {
+ assert_instances_of($refs, 'PhabricatorRepositoryCommitRef');
+
+ // 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 repository has finished importing and we're just catching up
+ // on recent commits, we usually 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()) {
+ $this->log(
+ pht(
+ 'Importing %s commit(s) at low priority ("PRIORITY_IMPORT") '.
+ 'because this repository is still importing.',
+ phutil_count($refs)));
+
+ return PhabricatorWorker::PRIORITY_IMPORT;
+ }
+
+ // See T13369. If we've discovered a lot of commits at once, import them
+ // at lower priority.
+
+ // This is mostly aimed at reducing the impact that synchronizing thousands
+ // of commits from a remote upstream has on other repositories. The queue
+ // is "mostly FIFO", so queueing a thousand commit imports can stall other
+ // repositories.
+
+ // In a perfect world we'd probably give repositories round-robin queue
+ // priority, but we don't currently have the primitives for this and there
+ // isn't a strong case for building them.
+
+ // Use "a whole lot of commits showed up at once" as a heuristic for
+ // detecting "someone synchronized an upstream", and import them at a lower
+ // priority to more closely approximate fair scheduling.
+
+ if (count($refs) >= PhabricatorRepository::LOWPRI_THRESHOLD) {
+ $this->log(
+ pht(
+ 'Importing %s commit(s) at low priority ("PRIORITY_IMPORT") '.
+ 'because many commits were discovered at once.',
+ phutil_count($refs)));
+
+ return PhabricatorWorker::PRIORITY_IMPORT;
+ }
+
+ // Otherwise, import at normal priority.
+
+ if ($refs) {
+ $this->log(
+ pht(
+ 'Importing %s commit(s) at normal priority ("PRIORITY_COMMIT").',
+ phutil_count($refs)));
+ }
+
+ return PhabricatorWorker::PRIORITY_COMMIT;
+ }
+
+
}
diff --git a/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php b/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php
--- a/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php
@@ -548,6 +548,22 @@
}
}
+ $commit_refs = array();
+ foreach ($identifiers as $identifier) {
+
+ // See T13591. This construction is a bit ad-hoc, but the priority
+ // function currently only cares about the number of refs we have
+ // discovered, so we'll get the right result even without filling
+ // these records out in detail.
+
+ $commit_refs[] = id(new PhabricatorRepositoryCommitRef())
+ ->setIdentifier($identifier);
+ }
+
+ $task_priority = $this->getImportTaskPriority(
+ $repository,
+ $commit_refs);
+
$permanent_flag = PhabricatorRepositoryCommit::IMPORTED_PERMANENT;
$published_flag = PhabricatorRepositoryCommit::IMPORTED_PUBLISH;
@@ -580,17 +596,12 @@
$import_status,
$row['id']);
- $data = array(
- 'commitID' => $row['id'],
- );
-
- PhabricatorWorker::scheduleTask(
- $class,
- $data,
- array(
- 'priority' => PhabricatorWorker::PRIORITY_COMMIT,
- 'objectPHID' => $row['phid'],
- ));
+ $this->queueCommitImportTask(
+ $repository,
+ $row['id'],
+ $row['phid'],
+ $task_priority,
+ $via = 'ref');
}
}

File Metadata

Mime Type
text/plain
Expires
Sun, May 12, 12:58 AM (3 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6287036
Default Alt Text
D21516.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility