From 26a3ad20f1c9445c6b6eee000da06bf93d54a14c Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 16 Jun 2022 12:31:03 +0200 Subject: [PATCH] [IMP] runbot: status management The status are currently sent by leader when build are created and by workers on post_commit. If the leader fails during the preparing of a batch (while creating builds) the transaction is rollbacked and the status are send again. The number of status to send makes it quite slow, making the transaction longuer, and the retry even more expensive. This leads to preparing time being quite important, sometimes ten minutes after many retries. This commit proposes to send status in another dedicated transaction. Since status are sent in batch, we can also try to use a unique session, and uniquify commit+context status. This allows to remove the postcommit logic A further improvement would be to wait before sending status in order to skip the pending status if the build is verry fast. An option is also added on the remote to skip the status: if the remote is a fork, sending the satus on the main remote should be enough. --- runbot/models/batch.py | 2 +- runbot/models/build.py | 6 +-- runbot/models/commit.py | 85 ++++++++++++++++++------------------- runbot/models/repo.py | 19 ++++++--- runbot/models/runbot.py | 4 ++ runbot/views/repo_views.xml | 3 ++ 6 files changed, 64 insertions(+), 55 deletions(-) diff --git a/runbot/models/batch.py b/runbot/models/batch.py index 2db6a142..04fc2b0a 100644 --- a/runbot/models/batch.py +++ b/runbot/models/batch.py @@ -138,7 +138,7 @@ class Batch(models.Model): build.host = self.bundle_id.host_id.name build.keep_host = True - build._github_status(post_commit=False) + build._github_status() return link_type, build def _prepare(self, auto_rebase=False): diff --git a/runbot/models/build.py b/runbot/models/build.py index 2910af47..7aa48911 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -1136,7 +1136,7 @@ class BuildResult(models.Model): if self.global_result in ('skipped', 'killed', 'manually_killed'): return 'killed' - def _github_status(self, post_commit=True): + def _github_status(self): """Notify github of failed/successful builds""" for build in self: # TODO maybe avoid to send status if build is killable (another new build exist and will send the status) @@ -1144,7 +1144,7 @@ class BuildResult(models.Model): if build.orphan_result: _logger.info('Skipping result for orphan build %s', self.id) else: - build.parent_id._github_status(post_commit) + build.parent_id._github_status() else: trigger = self.params_id.trigger_id if not trigger.ci_context: @@ -1170,4 +1170,4 @@ class BuildResult(models.Model): for build_commit in self.params_id.commit_link_ids: commit = build_commit.commit_id if 'base_' not in build_commit.match_type and commit.repo_id in trigger.repo_ids: - commit._github_status(build, trigger.ci_context, state, target_url, desc, post_commit) + commit._github_status(build, trigger.ci_context, state, target_url, desc) diff --git a/runbot/models/commit.py b/runbot/models/commit.py index 8dd56f8b..088e8058 100644 --- a/runbot/models/commit.py +++ b/runbot/models/commit.py @@ -136,7 +136,7 @@ class Commit(models.Model): for commit in self: commit.dname = '%s:%s' % (commit.repo_id.name, commit.name[:8]) - def _github_status(self, build, context, state, target_url, description=None, post_commit=True): + def _github_status(self, build, context, state, target_url, description=None): self.ensure_one() Status = self.env['runbot.commit.status'] last_status = Status.search([('commit_id', '=', self.id), ('context', '=', context)], order='id desc', limit=1) @@ -150,8 +150,8 @@ class Commit(models.Model): 'state': state, 'target_url': target_url, 'description': description or context, + 'to_process': True, }) - last_status._send(post_commit) class CommitLink(models.Model): @@ -184,51 +184,48 @@ class CommitStatus(models.Model): target_url = fields.Char('Url') description = fields.Char('Description') sent_date = fields.Datetime('Sent Date') + to_process = fields.Boolean('Status was not processed yet', index=True) - def _send(self, post_commit=True): - user_id = self.env.user.id - _dbname = self.env.cr.dbname - _context = self.env.context + def _send_to_process(self): + commits_status = self.search([('to_process', '=', True)], order='create_date DESC, id DESC') + if commits_status: + _logger.info('Sending %s commit status', len(commits_status)) + commits_status._send() - status_id = self.id - commit = self.commit_id - all_remote = commit.repo_id.remote_ids - remotes = all_remote.filtered(lambda remote: remote.token) - no_token_remote = all_remote-remotes - if no_token_remote: - _logger.warning('No token on remote %s, skipping status', no_token_remote.mapped("name")) - remote_ids = remotes.ids - commit_name = commit.name - - status = { - 'context': self.context, - 'state': self.state, - 'target_url': self.target_url, - 'description': self.description, - } - if remote_ids: - - def send_github_status(env): - for remote in env['runbot.remote'].browse(remote_ids): - _logger.info( - "github updating %s status %s to %s in repo %s", - status['context'], commit_name, status['state'], remote.name) - remote._github('/repos/:owner/:repo/statuses/%s' % commit_name, status, ignore_errors=True) - env['runbot.commit.status'].browse(status_id).sent_date = fields.Datetime.now() - - def send_github_status_async(): - try: - db_registry = registry(_dbname) - with api.Environment.manage(), db_registry.cursor() as cr: - env = api.Environment(cr, user_id, _context) - send_github_status(env) - except: - _logger.exception('Something went wrong sending notification for %s', commit_name) - - if post_commit: - self._cr.postcommit.add(send_github_status_async) + def _send(self): + session_cache = {} + processed = set() + for commit_status in self.sorted(lambda cs: (cs.create_date, cs.id), reverse=True): # ensure most recent are processed first + commit_status.to_process = False + # only send the last status for each commit+context + key = (commit_status.context, commit_status.commit_id.name) + if key not in processed: + processed.add(key) + status = { + 'context': commit_status.context, + 'state': commit_status.state, + 'target_url': commit_status.target_url, + 'description': commit_status.description, + } + for remote in commit_status.commit_id.repo_id.remote_ids.filtered('send_status'): + if not remote.token: + _logger.warning('No token on remote %s, skipping status', remote.mapped("name")) + else: + if remote.token not in session_cache: + session_cache[remote.token] = remote._make_github_session() + session = session_cache[remote.token] + _logger.info( + "github updating %s status %s to %s in repo %s", + status['context'], commit_status.commit_id.name, status['state'], remote.name) + remote._github('/repos/:owner/:repo/statuses/%s' % commit_status.commit_id.name, + status, + ignore_errors=True, + session=session + ) + commit_status.sent_date = fields.Datetime.now() else: - send_github_status(self.env) + _logger.info('Skipping outdated status for %s %s', commit_status.context, commit_status.commit_id.name) + class CommitExport(models.Model): diff --git a/runbot/models/repo.py b/runbot/models/repo.py index bf9ff48f..aff40c24 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -114,6 +114,7 @@ class Remote(models.Model): sequence = fields.Integer('Sequence', tracking=True) fetch_heads = fields.Boolean('Fetch branches', default=True, tracking=True) fetch_pull = fields.Boolean('Fetch PR', default=False, tracking=True) + send_status = fields.Boolean('Send status', default=True, tracking=True) token = fields.Char("Github token", groups="runbot.group_runbot_admin") @@ -155,24 +156,28 @@ class Remote(models.Model): self._cr.postcommit.add(self.repo_id._update_git_config) return res - def _github(self, url, payload=None, ignore_errors=False, nb_tries=2, recursive=False): - generator = self.sudo()._github_generator(url, payload=payload, ignore_errors=ignore_errors, nb_tries=nb_tries, recursive=recursive) + def _make_github_session(self): + session = requests.Session() + if self.token: + session.auth = (self.token, 'x-oauth-basic') + session.headers.update({'Accept': 'application/vnd.github.she-hulk-preview+json'}) + return session + + def _github(self, url, payload=None, ignore_errors=False, nb_tries=2, recursive=False, session=None): + generator = self.sudo()._github_generator(url, payload=payload, ignore_errors=ignore_errors, nb_tries=nb_tries, recursive=recursive, session=session) if recursive: return generator result = list(generator) return result[0] if result else False - def _github_generator(self, url, payload=None, ignore_errors=False, nb_tries=2, recursive=False): + def _github_generator(self, url, payload=None, ignore_errors=False, nb_tries=2, recursive=False, session=None): """Return a http request to be sent to github""" for remote in self: if remote.owner and remote.repo_name and remote.repo_domain: url = url.replace(':owner', remote.owner) url = url.replace(':repo', remote.repo_name) url = 'https://api.%s%s' % (remote.repo_domain, url) - session = requests.Session() - if remote.token: - session.auth = (remote.token, 'x-oauth-basic') - session.headers.update({'Accept': 'application/vnd.github.she-hulk-preview+json'}) + session = session or remote._make_github_session() while url: if recursive: _logger.info('Getting page %s', url) diff --git a/runbot/models/runbot.py b/runbot/models/runbot.py index f86f8e0a..82b9b929 100644 --- a/runbot/models/runbot.py +++ b/runbot/models/runbot.py @@ -260,12 +260,16 @@ class Runbot(models.AbstractModel): self._commit() self._commit() + self.env['runbot.commit.status']._send_to_process() + self._commit() + # cleanup old pull_info_failures for pr_number, t in pull_info_failures.items(): if t + 15*60 < time.time(): _logger.warning('Removing %s from pull_info_failures', pr_number) del pull_info_failures[pr_number] + return manager.get('sleep', default_sleep) def _scheduler_loop_turn(self, host, default_sleep=1): diff --git a/runbot/views/repo_views.xml b/runbot/views/repo_views.xml index 117f5c3f..a210d103 100644 --- a/runbot/views/repo_views.xml +++ b/runbot/views/repo_views.xml @@ -94,6 +94,7 @@ + @@ -118,6 +119,7 @@ + @@ -133,6 +135,7 @@ +