[FIX] forwardport: race condition on PR creation (#357)

e9e08fec3c attempted to fix the issue
but obviously failed as it still occurs: when creating a PR through
the API, it's possible that the webhook gets triggered fast enough the
transaction creating the PR from the webhook commits before we get
around to creating our own PR from the API call. In which case the
forward port process aborts.

The process is re-run later on and generally succeeds fully, but we're
left with a dangling PR we created but couldn't do anything with as
its use broke.

This issue seems to be getting more frequent so it's becoming quite
important to fix it. Therefore we give our Raging Bull a Big Gun and
now he has 20 attack *cough cough* we lock the bloody table down
tight (only allow concurrent `SELECT`) until we've got the PR back and
we've done the updates we need to it and nobody can mess with it...
probably.

This is not ideal as it's going to block updates to completely
unrelated PRs but it doesn't seem like postgres really allows for
locking out creations without locking out the rest, short of using
advisory locks maybe? E.g. in the `create` override get a
`pg_advisory_xact_lock_shared`, then get a `pg_advisory_xact_lock` in
the forward-port process that way we're just blocking the concurrent
creation of PRs during forward port, but creations don't block one
another and we don't block updates.

Application-level locks wouldn't really work as the 'bot could be
deployed using multi-worker scenarios so we'd need cross-process locks
or something.

Hopefully fixes #352
This commit is contained in:
xmo-odoo 2020-04-07 15:02:39 +02:00 committed by GitHub
parent 4c4b7213bb
commit 49c1d960fa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -594,6 +594,7 @@ class PullRequests(models.Model):
if not body:
body = None
self.env.cr.execute('LOCK runbot_merge_pull_requests IN SHARE MODE')
r = gh.post('https://api.github.com/repos/{}/pulls'.format(pr.repository.name), json={
'base': target.name, 'head': '%s:%s' % (owner, new_branch),
'title': title, 'body': body,
@ -613,17 +614,9 @@ class PullRequests(models.Model):
raise RuntimeError("Forwardport failure: %s (%s)" % (pr.display_name, results))
r = results
self.env.cr.commit()
new_pr = self.search([
('number', '=', r['number']),
('repository.name', '=', r['base']['repo']['full_name']),
], limit=1)
if new_pr:
_logger.info("Received forward-port PR %s", new_pr)
else:
new_pr = self._from_gh(r)
_logger.info("Created forward-port PR %s", new_pr)
new_pr = self._from_gh(r)
_logger.info("Created forward-port PR %s", new_pr)
new_batch |= new_pr
# delegate original author on merged original PR & on new PR so