mirror of
https://github.com/odoo/runbot.git
synced 2025-03-15 15:35:46 +07:00
[IMP] runbot_merge: avoid updating every PR and staging on status change
Because the only condition between PRs and statuses is sharing a repository (and kinda not even that for stagings), adding or removing a status on a repository would try to recompute the statuses/state of essentially every staging in the history of the project, and a significant fraction of the PRs, leading to tens of thousands of queries, minutes of computation, and even OOMs of the HTTP workers as we'd load the PRs, their batches, and the stagings into memory to update them, then load more things because of what depends on PR statuses, etc... But there is no reason to touch any of the closed or merged PRs, or the completed (deactivated) stagings. And in fact there is every reason *not* to. Implementing a search-only m2m on each object in order to restrict the set of PRs/stagings relevant to a change reduces the number of queries to a few hundreds, the run time to seconds, and the memory increase to unnoticeable. The change still takes some time because the RD project currently has about 7000 open PRs, 4500 of which target odoo/odoo (which is our test case here), but that is nothing compared to the 164000 total PRs to odoo/odoo out of some 250000 PRs for the RD project. And though they're likely less of an issue as they don't recurse quite as much, the >120000 stagings during the project's history are not to be ignored, when then number of *active* stagings at any one time is at most the number of branches, which is half a dozen to a dozen. For very basic numbers, as of committing this change, creating a status in odoo/odoo (RD), optional on PRs and ignored on statuses, on my current machine (7530U with 32GB RAM): - without this change, 4835 queries, 37s of sql, 65s of non-SQL, RSS climbs to 2258128 (2.15GiB) - with this change, 758 queries, 1.46s SQL, 2.25s non-SQL, RSS climbs to 187088 (182MiB) Fixes #1067
This commit is contained in:
parent
4fa93266c6
commit
391a323efd
@ -95,6 +95,7 @@ class StatusConfiguration(models.Model):
|
||||
def _default_pr_state(self) -> typing.Literal['pending', 'success']:
|
||||
return 'pending' if self.prs == 'required' else 'success'
|
||||
|
||||
|
||||
class Repository(models.Model):
|
||||
_name = _description = 'runbot_merge.repository'
|
||||
_order = 'sequence, id'
|
||||
@ -1260,11 +1261,22 @@ For your own safety I've ignored *everything in your entire comment*.
|
||||
self.env.add_to_compute(self._fields['state'], descendants_or_self)
|
||||
super().modified(fnames, create, before)
|
||||
|
||||
applicable_statuses = fields.Many2many(
|
||||
'runbot_merge.repository.status',
|
||||
store=False,
|
||||
search='_search_applicable_statuses',
|
||||
)
|
||||
def _search_applicable_statuses(self, operator, value):
|
||||
return [
|
||||
('merge_date', '=', False), ('closed', '=', False),
|
||||
('repository.status_ids', operator, value),
|
||||
]
|
||||
|
||||
@api.depends(
|
||||
'statuses', 'overrides', 'target', 'parent_id', 'skipchecks',
|
||||
'repository.status_ids.context',
|
||||
'repository.status_ids.branch_filter',
|
||||
'repository.status_ids.prs',
|
||||
'applicable_statuses.context',
|
||||
'applicable_statuses.branch_filter',
|
||||
'applicable_statuses.prs',
|
||||
)
|
||||
def _compute_statuses(self):
|
||||
for pr in self:
|
||||
@ -2250,12 +2262,24 @@ class Stagings(models.Model):
|
||||
|
||||
return True
|
||||
|
||||
applicable_statuses = fields.Many2many(
|
||||
'runbot_merge.repository.status',
|
||||
store=False,
|
||||
search='_search_applicable_statuses',
|
||||
)
|
||||
def _search_applicable_statuses(self, operator, value):
|
||||
return [
|
||||
('active', '=', True),
|
||||
('heads.repository_id.status_ids', operator, value),
|
||||
]
|
||||
|
||||
@api.depends(
|
||||
"statuses_cache",
|
||||
"target",
|
||||
"heads.commit_id.sha",
|
||||
"heads.repository_id.status_ids.branch_filter",
|
||||
"heads.repository_id.status_ids.context",
|
||||
"applicable_statuses.branch_filter",
|
||||
"applicable_statuses.context",
|
||||
"applicable_statuses.stagings",
|
||||
)
|
||||
def _compute_state(self):
|
||||
for staging in self:
|
||||
|
Loading…
Reference in New Issue
Block a user