Merge lp://staging/~jelmer/launchpad/681974-building-commit into lp://staging/launchpad
Proposed by
Jelmer Vernooij
| Status: | Merged |
|---|---|
| Approved by: | Jelmer Vernooij |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 12024 |
| Proposed branch: | lp://staging/~jelmer/launchpad/681974-building-commit |
| Merge into: | lp://staging/launchpad |
| Diff against target: |
39 lines (+5/-6) 2 files modified
lib/lp/buildmaster/model/builder.py (+0/-1) lib/lp/buildmaster/model/packagebuild.py (+5/-5) |
| To merge this branch: | bzr merge lp://staging/~jelmer/launchpad/681974-building-commit |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Jeroen T. Vermeulen (community) | code | Approve | |
|
Review via email:
|
|||
Commit message
[r=jtv][ui=none][bug=681974] Commit rather than flush to prevent races between buildd manager and archive uploader.
Description of the change
There is a race condition between the buildd manager and the upload processor, which we seem to've hit a couple of times. The buildd manager in some cases moves the file before it commits the change to the job status, causing the upload processor to blow up.
We did originally consider this but did a database flush rather than a commit. This fixes that.
I haven't added any new tests since I couldn't think of a good way to test this. I'm interested in any ideas on how to test this.
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
(18:00:31) jtv: good cover letter. I'm surprised it's the buildd manager confusing the upload processor by moving the file before committing, and not another way around.
(18:03:44) jelmer: This is happening with binary builds that the buildd manager has fetched from the builders.
(18:04:25) jelmer: There is another archive uploader instance that processes the source uploads from users, which the buildd manager then sends off to the builders.
(18:08:17) jtv: ah so the data flows in the opposite direction from what I thought.
(18:09:45) jtv: Depending on what the upload processor's transactions look like and even which isolation level it uses, there may _conceivably_ still be race conditions but even then this should narrow it own. And I'm not assuming that you got it wrong anyway. :)
(18:10:06) jtv: I'm also assuming that you're not committing in some untenable state, and therefore r=jtv.