Re: [openstack-dev] [Neutron] Returning of HEAD files?

[Anna Kamyshnikova]

Ihar, Mark thank you for your comments!

Change [1] is ready, so we can land it in Neutron. For subprojects it will
be up to them to have HEAD files or not - tests won't fail, just warning
message will be shown.

[1] - https://review.openstack.org/#/c/232607

On Fri, Oct 9, 2015 at 8:16 PM, Mark McClain  wrote:

>
> 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.
>
>
> The HEAD file is a clear winner when gate load is high and there are
> competing migrations.  As you pointed out PEP8 will detect the problem, but
> not fast enough to cause a ripple in the changes behind it in the gate.
> The ripple causes resources to be wasted adding to the load, so the git
> merge conflict was the best compromise we developed.
>
> mark
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>


-- 
Regards,
Ann Kamyshnikova
Mirantis, Inc
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

[openstack-dev] [Neutron] Returning of HEAD files?

[Anna Kamyshnikova]

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/

-- 
Regards,
Ann Kamyshnikova
Mirantis, Inc
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [Neutron] Returning of HEAD files?

[Ihar Hrachyshka]

> On 09 Oct 2015, at 15:28, Anna Kamyshnikova  
> 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


signature.asc
Description: Message signed with OpenPGP using GPGMail
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe