mirror of
https://github.com/odoo/runbot.git
synced 2025-03-15 23:45:44 +07:00
[FIX] forwardport: possible race in forwardport followup update
Normally when a forwardport is updated the forward-bot cascades the update onto its followups (if it has any), but takes care to keep the followups attached as they were not updated "by hand". In the case of odoo/odoo#77677 however that did not work and the followup PRs got detached. Looking at the logs, it becomes flagrant that there was a race condition: either Git took a long time to respond to the push, or there was an IO slowdown which led to the "local update" taking a very long time. Either way this allowed the "synchronize" webhook from github to arrive before the current transaction was committed, rolling back said transaction and making the forwardbot assume this was a "real" sync and detach the followup from its parent. Locking the PR row up-front ought fix the issue, and also move the local update to before having pushed: the "extra" commits in the local cache don't matter too much even if pushing to github fails, they'll be cleaned up by a GC eventually. Also migrate the `-f` on push to `--force-if-includes` in order to avoid possible race conditions on the push (since we're not fetching the current branch, use the full-syntax explicit CAS form, that's exactly what we're looking for anyway). Fixes #541 (hopefully)
This commit is contained in:
parent
1dcbb4a5ac
commit
c6755a045a
@ -102,6 +102,12 @@ class UpdateQueue(models.Model, Queue):
|
||||
previous = self.new_root
|
||||
with ExitStack() as s:
|
||||
for child in self.new_root._iter_descendants():
|
||||
self.env.cr.execute("""
|
||||
SELECT id
|
||||
FROM runbot_merge_pull_requests
|
||||
WHERE id = %s
|
||||
FOR UPDATE NOWAIT
|
||||
""", [child.id])
|
||||
_logger.info(
|
||||
"Re-port %s from %s (changed root %s -> %s)",
|
||||
child.display_name,
|
||||
@ -149,22 +155,23 @@ class UpdateQueue(models.Model, Queue):
|
||||
f'{child.target.name}..{child.refname}',
|
||||
count=True
|
||||
).stdout.decode().strip())
|
||||
old_head = child.head
|
||||
# update child's head to the head we're going to push
|
||||
child.with_context(ignore_head_update=True).write({
|
||||
'head': new_head,
|
||||
# 'state': 'opened',
|
||||
'squash': commits_count == 1,
|
||||
})
|
||||
working_copy.push('-f', 'target', child.refname)
|
||||
|
||||
# also push to local cache: looks like in some cases github
|
||||
# doesn't propagate revisions (?) or at least does so too slowly
|
||||
# so on the next loop we try to fetch the revision we just
|
||||
# pushed through PR and... we can't find it
|
||||
# push the new head to the local cache: in some cases github
|
||||
# doesn't propagate revisions fast enough so on the next loop we
|
||||
# can't find the revision we just pushed
|
||||
dummy_branch = str(uuid.uuid4())
|
||||
ref = previous._get_local_directory()
|
||||
working_copy.push(ref._directory, f'{new_head}:refs/heads/{dummy_branch}')
|
||||
ref.branch('--delete', '--force', dummy_branch)
|
||||
# then update the child's branch to the new head
|
||||
working_copy.push(f'--force-with-lease={child.refname}:{old_head}',
|
||||
'target', child.refname)
|
||||
|
||||
# committing here means github could technically trigger its
|
||||
# webhook before sending a response, but committing before
|
||||
|
Loading…
Reference in New Issue
Block a user