recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Improve use of keys when iterating over commits in "bin/audit delete" and "bin/repository rebuild-identities"

Summary:
Fixes T13457. Ref T13444. When we iterate over commits in a particular repository, the default iteration strategy can't effectively use the keys on the table.

Tweak the ordering so the "<repositoryID, epoch, [id]>" key can be used.

Test Plan:
- Ran `bin/audit delete --repository X` and `bin/repository rebuild-identities --repository X` before and after changes.
- With just the key changes, performance was slightly better. My local data isn't large enough to really emphasize the key changes.
- With the page size changes, performance was a bit better (~30%, but on 1-3 second run durations).
- Used `--trace` and ran `EXPLAIN ...` on the new queries, saw them select the "<repositoryID, epoch, [id]>" key and report a bare "Using index condition" in the "Extra" column.

Maniphest Tasks: T13457, T13444

Differential Revision: https://secure.phabricator.com/D20921

epriestley 63d84e0b a7aca500

+23 -3
+11 -1
src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php
··· 93 93 94 94 if ($repos) { 95 95 $query->withRepositoryIDs(mpull($repos, 'getID')); 96 + 97 + // See T13457. If we're iterating over commits in a single large 98 + // repository, the lack of a "<repositoryID, [id]>" key can slow things 99 + // down. Iterate in a specific order to use a key which is present 100 + // on the table ("<repositoryID, epoch, [id]>"). 101 + $query->setOrderVector(array('-epoch', '-id')); 96 102 } 97 103 98 104 $auditor_map = array(); ··· 105 111 $query->withPHIDs(mpull($commits, 'getPHID')); 106 112 } 107 113 108 - $commit_iterator = new PhabricatorQueryIterator($query); 114 + $commit_iterator = id(new PhabricatorQueryIterator($query)); 115 + 116 + // See T13457. We may be examining many commits; each commit is small so 117 + // we can safely increase the page size to improve performance a bit. 118 + $commit_iterator->setPageSize(1000); 109 119 110 120 $audits = array(); 111 121 foreach ($commit_iterator as $commit) {
+6 -1
src/applications/repository/management/PhabricatorRepositoryManagementRebuildIdentitiesWorkflow.php
··· 98 98 ->needCommitData(true) 99 99 ->withRepositoryIDs(array($repository->getID())); 100 100 101 - $commit_iterator = new PhabricatorQueryIterator($commit_query); 101 + // See T13457. Adjust ordering to hit keys better and tweak page size 102 + // to improve performance slightly, since these records are small. 103 + $commit_query->setOrderVector(array('-epoch', '-id')); 104 + 105 + $commit_iterator = id(new PhabricatorQueryIterator($commit_query)) 106 + ->setPageSize(1000); 102 107 103 108 $this->rebuildCommits($commit_iterator); 104 109 }
+6 -1
src/infrastructure/storage/lisk/PhabricatorQueryIterator.php
··· 10 10 } 11 11 12 12 protected function didRewind() { 13 - $this->pager = new AphrontCursorPagerView(); 13 + $pager = new AphrontCursorPagerView(); 14 + 15 + $page_size = $this->getPageSize(); 16 + $pager->setPageSize($page_size); 17 + 18 + $this->pager = $pager; 14 19 } 15 20 16 21 public function key() {