<div dir="ltr">Top posting, this is a very old thread.<div><br></div><div>Keeping in view the recent NetBSD problems and the number of bugs creeping in, I suggest we do these things right now:</div><div><br></div><div>a. Change the gerrit merge type to fast forward only.</div><div>As explained below in the thread, with our current setup even if both PatchA and PatchB pass regression separately when both are merged it is possible that a functional bug creeps in.</div><div>This is the only solution to prevent that from happening.</div><div>I will work with Kaushal to get this done.</div><div><br></div><div>b. In Jenkins, remove gerrit trigger and make it a manual operation</div><div><br></div><div>Too many developers use the upstream infra as a test cluster and it is *not*.<br></div><div>It is a verification mechanism for maintainers to ensure that the patch does not cause regression.</div><div>It is required that all developers run full regression on their machines before asking for reviews.<br></div><div>Reviewers should review the patch only when the developer has given a +1 verified on the patch.</div><div>Again, I will work with Kaushal to get this done.</div><div><br></div><div>P.S: Stop using the "universal" jenkins account to trigger jenkins build if you are not a maintainer.</div><div>If you are a maintainer and don't have your own jenkins account then get one soon!</div><div><br></div><div>Thanks,</div><div>Raghavendra Talur</div><div><br></div><div><br></div><div><br></div><div><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 10, 2015 at 11:33 AM, Atin Mukherjee <span dir="ltr"><<a href="mailto:atin.mukherjee83@gmail.com" target="_blank">atin.mukherjee83@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr"></p>
<p dir="ltr">-Atin<br>
Sent from one plus one</p><div><div class="h5"><br>
On Nov 10, 2015 11:24 AM, "Kaushal M" <<a href="mailto:kshlmster@gmail.com" target="_blank">kshlmster@gmail.com</a>> wrote:<br>
><br>
> On Tue, Nov 10, 2015 at 9:24 AM, Raghavendra Gowdappa<br>
> <<a href="mailto:rgowdapp@redhat.com" target="_blank">rgowdapp@redhat.com</a>> wrote:<br>
> ><br>
> ><br>
> > ----- Original Message -----<br>
> >> From: "Raghavendra Talur" <<a href="mailto:rtalur@redhat.com" target="_blank">rtalur@redhat.com</a>><br>
> >> To: "Gluster Devel" <<a href="mailto:gluster-devel@gluster.org" target="_blank">gluster-devel@gluster.org</a>><br>
> >> Sent: Tuesday, November 10, 2015 3:10:34 AM<br>
> >> Subject: [Gluster-devel] Gerrit review, submit type and Jenkins testing<br>
> >><br>
> >> Hi,<br>
> >><br>
> >> While trying to understand how our gerrit+jenkins setup works, I realized of<br>
> >> a possibility of allowing bugs to get in.<br>
> >><br>
> >> Currently, our gerrit is setup to have cherry-pick as the submit type. Now<br>
> >> consider a case where:<br>
> >><br>
> >> Dev1 sends a commit B with parent commit A(A is already merged).<br>
> >> Dev2 sends a commit C with parent commit A(A is already merged).<br>
> >><br>
> >> Both the patches get +2 from Jenkins.<br>
> >><br>
> >> Maintainer merges commit B from Dev1.<br>
> >> Another maintainer merges commit C from Dev2.<br>
> >><br>
> >> If the two commits B and C changed code which had no merge conflicts but were<br>
> >> conflicting in logic,<br>
> >> then we have a master which has bugs.<br>
> >><br>
> >> If Dev3 now sends a commit D with re-based master as parent, we have the<br>
> >> following cases:<br>
> >><br>
> >> 1. If bug introduced above is not racy, we have tests always failing for Dev3<br>
> >> on commit D. Tests that fail would be from components that commit B and C<br>
> >> changed. Dev3 has no idea on how to fix them and has to enlist help from<br>
> >> Dev1 and Dev2.<br>
> >><br>
> >> 2. If bug introduced above is racy, then there is a probability that Dev3<br>
> >> escapes from this trouble and someone else will bear it later. Even if the<br>
> >> racy code is hit and test fails, Dev3 will probably re-trigger the tests<br>
> >> given that they failed for a component which is not related to his/her code<br>
> >> and the bug stays in code longer.<br>
> >><br>
> >> The most obvious but not practical solution to the above problem is to change<br>
> >> the submit type in gerrit to "fast-forward only". It would then ensure that<br>
> >> once commit B is merged, Dev2 has to re-base and re-run the tests on commit<br>
> >> C with commit B as parent, before it could be merged. It is not practical<br>
> >> because it will cause all patches in review to get re-based and re-triggered<br>
> >> whenever a patch is merged.<br>
> >><br>
> >> A little modification to the above solution would be to<br>
> >><br>
> >><br>
> >> * change submit type to fast-forward only<br>
> >> * don't run any jenkins job on patches till they get +2 from reviewers<br>
> >> * once a +2 is given, run jenkins job on patch and automatically submit<br>
> >> it if test passes.<br>
> >> * automatically rebase all patches on review with new master and mark<br>
> >> conflict if merge conflict arises.<br>
> ><br>
> > Seems like a good suggestion. How about a slight variation to the above process? Can we run one initial set of regression immediately after submission, but before any reviews? That way reviewers can prioritize those patches that have passed regression over the ones that have failed? Flip side is that minimum two sets of regressions are needed to merge any patch. I am making this suggestion with the assumption that dev/reviewer time is more precious than machine time. Of course, this will have issues with patches that need to get in urgently (user/customer hot fix etc) where time is a constraint. But that can be worked around on a case-by-case basis.<br>
><br>
> We would still be running smoke, which would catch any very obvious<br>
> mistakes, isn't this enough?<br>
><br>
> Regarding the initial regression run, would it include the complete<br>
> regression suite or just a subset. If it is the complete set, then it<br>
> would be no different from what we are doing now. If it is a subset,<br>
> then we will need to come up with a subset of the regression suite<br>
> that catches most of obvious mistakes. We've had discussions several<br>
> times (don't remember if it was on the mailing lists, but I have had<br>
> conversations) about doing this. Every time we ended up on the<br>
> question of how we choose the subset, which is where we stopped.<br></div></div>
How about running tests/basic/*.t? I know coverage wise this isn't good enough, but still better than running everything or nothing.<span class=""><br>
><br>
> ><br>
> >><br>
> >> As a side effect of this, Dev would now be forced to run a complete<br>
> >> regression on dev machine before sending a patch for review.<br>
> >><br>
> >> Any thoughts on the above solutions or other suggestions?<br>
> >><br>
> >> Thanks,<br>
> >> Raghavendra Talur<br>
> >><br>
> >><br>
> >><br>
> >><br>
> >><br>
> >><br>
> >><br>
> >><br>
> >><br>
> >> _______________________________________________<br>
> >> Gluster-devel mailing list<br>
> >> <a href="mailto:Gluster-devel@gluster.org" target="_blank">Gluster-devel@gluster.org</a><br>
> >> <a href="http://www.gluster.org/mailman/listinfo/gluster-devel" target="_blank">http://www.gluster.org/mailman/listinfo/gluster-devel</a><br>
> > _______________________________________________<br>
> > Gluster-devel mailing list<br>
> > <a href="mailto:Gluster-devel@gluster.org" target="_blank">Gluster-devel@gluster.org</a><br>
> > <a href="http://www.gluster.org/mailman/listinfo/gluster-devel" target="_blank">http://www.gluster.org/mailman/listinfo/gluster-devel</a><br>
> _______________________________________________<br>
> Gluster-devel mailing list<br>
> <a href="mailto:Gluster-devel@gluster.org" target="_blank">Gluster-devel@gluster.org</a><br>
> <a href="http://www.gluster.org/mailman/listinfo/gluster-devel" target="_blank">http://www.gluster.org/mailman/listinfo/gluster-devel</a><br>
</span><p></p>
<br>_______________________________________________<br>
Gluster-devel mailing list<br>
<a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.org</a><br>
<a href="http://www.gluster.org/mailman/listinfo/gluster-devel" rel="noreferrer" target="_blank">http://www.gluster.org/mailman/listinfo/gluster-devel</a><br></blockquote></div><br></div></div></div>