Open Stack

> On 09 Oct 2015, at 15:28, Anna Kamyshnikova <akamyshnikova at mirantis.com> wrote:
> 
> Some time ago we merged change [1] that removes HEADS file. Validation of migration revisions using HEADS file was replaced with pep8. This allows us to avoid merge conflicts that appeared every time a new migration was merged.
> 
> The problem was pointed by Kevin Benton as the original idea of HEAD file [2] was not only to validate revisions, so as not to allow outdated changes go into merge queue, that could be very important for the end of the cycle when a lot of patches get approved.
> 
> I introduced change [3] that returns HEAD files, but this time they are created per branch, so that will reduce merge conflicts a bit.
> 
> I understand that it was better to ask at the first time when [1] was on review, should we have HEAD files and merge conflicts or not, but I want to ask it now: Should I continue work on [3] or we are not expecting to have problems with big merge queues?
> 
> 
> [1] - https://review.openstack.org/#/c/227319/
> [2] - https://github.com/openstack/neutron/commit/36d85f831ae8eb21383806261bfc4c3d53dd1929
> [3] - https://review.openstack.org/#/c/232607/


I think it’s worth describing merge scenarios with and without HEAD files.

1. both patches in merge queue

Currently (no HEAD files), if two patches that touch the same alembic branch head are pushed into the gate:

- first patch passes the gate;
- second patch fails on pep8, and moved out of the merge queue; other jobs continue to run to report the failure back to Gerrit;
- if a patch above the first patch reset the queue, then both patches are re-added into merge queue; again, the second patch fails on pep8 quickly and is moved out of the queue;
- the second patch author is notified about the failure once the first patch is merged and all jobs for the second patch are complete (at least pep8 is failed).

With HEAD files,
- first patch passes the gate;
- second patch does not get into the queue until first patch merges, or fails (because there are now git conflicts);
- if a patch above the first patch reset the queue, only first patch is re-added into the merge queue; the second patch waits until the first patch merges or fails to merge; in the former case, zuul reports git conflict back to the author of the second patch; in the latter case, the second patch is added into the merge queue and merges if all goes well;
- meaning, the second patch author is notified about the failure once the first patch is merged.

I see the following speed-ups with HEAD files:
- there is no need to wait for jobs of the second patch to complete before we notify the author about the problem;
- queue is not reset by second patch failures after each gate reset; since pep8 job fails quick, it’s ~5 mins per reset (which should not occur frequently);

I see the following speed-up without HEAD files:
- if first patch fails to merge, the second patch gets into the merge queue at the point were it was pushed into the gate, not at the end of the list at the moment the first patch failed.

2. one patch in merge queue

Currently, when a patch is merged in the gate, all other patches are tested on git conflicts, but since there are no conflicts due to HEAD files (there are no such things now), authors are not notified about the issue. Still, the patch can proceed with review (there is no -1 vote from CI which scares a lot of people), and if pushed into gate, will immediately fail. Then author will need to rebase, and reviewers repeat the push into the gate.

With HEAD files, we would immediately detect git conflict and report to the author about the issue, setting -1 vote for CI. Then the author updates the patch, gets fresh vote and hopes that other reviewers get back to his patch.

In that scenario, it’s not clear what’s better for review velocity. My experience shows that git conflicts and -1 CI votes slow down reviews, and if a patch was in the gate before, it should be easy to respin it for a small change in head and push. On the contrary, git conflicts are very reviewer time consuming, and scare reviewers away.

3. Another tiny benefit not to have git conflicts on HEAD files is that reviewers can distinguish legitimate git conflicts from branching failures, and apply appropriate review attention based on the nature of the failure. We also get fresh CI run for the patch (except for pep8 job that is doomed to fail until rebase).

There are pros and cons for both approaches, but overall, I don’t see how the former justify having HEAD files and the complexity to handle them in code and in file system.

Ihar
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20151009/9fa6301b/attachment.pgp>