<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Aug 22, 2018 at 5:54 PM Shyam Ranganathan <<a href="mailto:srangana@redhat.com">srangana@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 08/18/2018 12:45 AM, Pranith Kumar Karampuri wrote:<br>
> <br>
> <br>
> On Tue, Aug 14, 2018 at 5:29 PM Shyam Ranganathan <<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a><br>
> <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a>>> wrote:<br>
> <br>
> On 08/09/2018 01:24 AM, Pranith Kumar Karampuri wrote:<br>
> ><br>
> ><br>
> > On Thu, Aug 9, 2018 at 1:25 AM Shyam Ranganathan<br>
> <<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a> <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a>><br>
> > <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a> <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a>>>> wrote:<br>
> ><br>
> > Maintainers,<br>
> ><br>
> > The following thread talks about a merge during a merge<br>
> lockdown, with<br>
> > differing view points (this mail is not to discuss the view<br>
> points).<br>
> ><br>
> > The root of the problem is that we leave the current process<br>
> to good<br>
> > faith. If we have a simple rule that we will not merge<br>
> anything during a<br>
> > lock down period, this confusion and any future repetitions of<br>
> the same<br>
> > would not occur.<br>
> ><br>
> > I propose that we follow the simpler rule, and would like to hear<br>
> > thoughts around this.<br>
> ><br>
> > This also means that in the future, we may not need to remove<br>
> commit<br>
> > access for other maintainers, as we do *not* follow a good<br>
> faith policy,<br>
> > and instead depend on being able to revert and announce on the<br>
> threads<br>
> > why we do so.<br>
> ><br>
> ><br>
> > I think it is a good opportunity to establish guidelines and<br>
> process so<br>
> > that we don't end up in this state in future where one needs to lock<br>
> > down the branch to make it stable. From that p.o.v. discussion on this<br>
> > thread about establishing a process for lock down probably doesn't add<br>
> > much value. My personal opinion for this instance at least is that<br>
> it is<br>
> > good that it was locked down. I tend to forget things and not<br>
> having the<br>
> > access to commit helped follow the process automatically :-).<br>
> <br>
> The intention is that master and release branches are always maintained<br>
> in good working order. This involves, tests and related checks passing<br>
> *always*.<br>
> <br>
> When this situation is breached, correcting it immediately is better<br>
> than letting it build up, as that would entail longer times and more<br>
> people to fix things up.<br>
> <br>
> In an ideal world, if nightly runs fail, the next thing done would be to<br>
> examine patches that were added between the 2 runs, and see if they are<br>
> the cause for failure, and back them out.<br>
> <br>
> Hence calling to backout patches is something that would happen more<br>
> regularly in the future if things are breaking.<br>
> <br>
> <br>
> I'm with you till here.<br>
> <br>
> <br>
> <br>
> Lock down may happen if 2 consecutive nightly builds fail, so as to<br>
> rectify the situation ASAP, and then move onto other work.<br>
> <br>
> In short, what I wanted to say is that preventing lock downs in the<br>
> future, is not a state we aspire for. <br>
> <br>
> <br>
> What are the problems you foresee in aspiring for preventing lock downs<br>
> for everyone?<br>
<br>
Any project will have test failures, when things are put together and<br>
exercised in different environments.<br>
<br>
For us, these environments, at present, are nightly regression on<br>
centOS7, Brick-mux enabled regression, lcov, clang and in the future we<br>
hope to increase this to ASAN, performance baselines, memory leak<br>
checks, etc.<br>
<br>
The whole intent of adding such tests and checks to the test pipeline,<br>
is to ensure that regressions to the good state, are caught early and<br>
regularly.<br>
<br>
When these tests and checks actually show up errors/issues, is when we<br>
need to pay attention to the same and focus first on getting us back on<br>
track.<br>
<br>
At the above juncture, is when we need the lockdown or commit blackout<br>
(or whatever we want to call it). The intent is to ensure that we do not<br>
add more patches and further destabilize the branch, but stabilize it<br>
first and then get other changes in later.<br>
<br>
There is a certain probability with which the above event will happen,<br>
and that can be reduced, if we are more stringent in our development<br>
practices, and ensuring code health is maintained (both by more checks<br>
in smoke and more tests per patch or even otherwise, and also by keener<br>
reviews and other such means).<br>
<br>
We also need to be proactive to, monitoring and fixing, failures in the<br>
tests, so that we can address them quickly, rather than someone calling<br>
out a lockdown.<br>
<br>
Now, is your question that we should avoid the above state altogether?<br>
Or, how to retain commit access for all, but still have such states as<br>
above, where only patches that help stabilization are merged?<br>
<br>
For the former, I do not have an answer, we can reduce the event<br>
probability as stated above, but it will never disappear (which means<br>
all those tests are pretty much of no value and needs to improve).<br></blockquote><div><br></div><div>Depends on how frequently it will happen. Let us agree on some tolerance level and get the maintainer/developer to address that particular test before any other patches in that component can be allowed to be merged. So per component lock-down instead of whole project lock-down. <br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
For the latter, it circles back to what I stated in earlier responses,<br>
on good faith versus process, and we would like to stick to a process<br>
and keep the commit list short, rather than use good faith to avoid any<br>
inadvertent "I missed the mail" cases.<br>
<br>
> <br>
> <br>
> Lock downs may/will happen, it is<br>
> done to get the branches stable quicker, than spend long times trying to<br>
> find what caused the instability in the first place.<br>
> <br>
> <br>
> <br>
> <br>
> <br>
> <br>
> > <br>
> ><br>
> ><br>
> > Please note, if there are extraneous situations (say reported<br>
> security<br>
> > vulnerabilities that need fixes ASAP) we may need to loosen up the<br>
> > stringency, as that would take precedence over the lock down.<br>
> These<br>
> > exceptions though, can be called out and hence treated as such.<br>
> ><br>
> > Thoughts?<br>
> ><br>
> ><br>
> > This is again my personal opinion. We don't need to merge patches<br>
> in any<br>
> > branch unless we need to make an immediate release with that<br>
> patch. For<br>
> > example if there is a security issue reported we *must* make a release<br>
> > with the fix immediately so it makes sense to merge it and do the<br>
> release.<br>
> <br>
> Agree, keeps the rule simple during lock down and not open to<br>
> interpretations.<br>
> <br>
> > <br>
> ><br>
> ><br>
> > Shyam<br>
> ><br>
> > PS: Added Yaniv to the CC as he reported the deviance<br>
> ><br>
> > -------- Forwarded Message --------<br>
> > Subject: Re: [Gluster-devel] Release 5: Master branch<br>
> health<br>
> > report<br>
> > (Week of 30th July)<br>
> > Date: Tue, 7 Aug 2018 23:22:09 +0300<br>
> > From: Yaniv Kaul <<a href="mailto:ykaul@redhat.com" target="_blank">ykaul@redhat.com</a> <mailto:<a href="mailto:ykaul@redhat.com" target="_blank">ykaul@redhat.com</a>><br>
> <mailto:<a href="mailto:ykaul@redhat.com" target="_blank">ykaul@redhat.com</a> <mailto:<a href="mailto:ykaul@redhat.com" target="_blank">ykaul@redhat.com</a>>>><br>
> > To: Shyam Ranganathan <<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a><br>
> <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a>><br>
> > <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a> <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a>>>><br>
> > CC: GlusterFS Maintainers <<a href="mailto:maintainers@gluster.org" target="_blank">maintainers@gluster.org</a><br>
> <mailto:<a href="mailto:maintainers@gluster.org" target="_blank">maintainers@gluster.org</a>><br>
> > <mailto:<a href="mailto:maintainers@gluster.org" target="_blank">maintainers@gluster.org</a><br>
> <mailto:<a href="mailto:maintainers@gluster.org" target="_blank">maintainers@gluster.org</a>>>>, Gluster Devel<br>
> > <<a href="mailto:gluster-devel@gluster.org" target="_blank">gluster-devel@gluster.org</a> <mailto:<a href="mailto:gluster-devel@gluster.org" target="_blank">gluster-devel@gluster.org</a>><br>
> <mailto:<a href="mailto:gluster-devel@gluster.org" target="_blank">gluster-devel@gluster.org</a> <mailto:<a href="mailto:gluster-devel@gluster.org" target="_blank">gluster-devel@gluster.org</a>>>>,<br>
> > Nigel Babu <<a href="mailto:nigelb@redhat.com" target="_blank">nigelb@redhat.com</a> <mailto:<a href="mailto:nigelb@redhat.com" target="_blank">nigelb@redhat.com</a>><br>
> <mailto:<a href="mailto:nigelb@redhat.com" target="_blank">nigelb@redhat.com</a> <mailto:<a href="mailto:nigelb@redhat.com" target="_blank">nigelb@redhat.com</a>>>><br>
> ><br>
> ><br>
> ><br>
> ><br>
> ><br>
> > On Tue, Aug 7, 2018, 10:46 PM Shyam Ranganathan<br>
> <<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a> <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a>><br>
> > <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a> <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a>>><br>
> > <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a> <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a>><br>
> <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a> <mailto:<a href="mailto:srangana@redhat.com" target="_blank">srangana@redhat.com</a>>>>> wrote:<br>
> ><br>
> > On 08/07/2018 02:58 PM, Yaniv Kaul wrote:<br>
> > > The intention is to stabilize master and not add<br>
> more patches<br>
> > that my<br>
> > > destabilize it.<br>
> > ><br>
> > ><br>
> > > <a href="https://review.gluster.org/#/c/20603/" rel="noreferrer" target="_blank">https://review.gluster.org/#/c/20603/</a> has been merged.<br>
> > > As far as I can see, it has nothing to do with<br>
> stabilization and<br>
> > should<br>
> > > be reverted.<br>
> ><br>
> > Posted this on the gerrit review as well:<br>
> ><br>
> > <snip><br>
> > 4.1 does not have nightly tests, those run on master only.<br>
> ><br>
> ><br>
> > That should change of course. We cannot strive for stability<br>
> otherwise,<br>
> > AFAIK. <br>
> ><br>
> ><br>
> > Stability of master does not (will not), in the near term<br>
> guarantee<br>
> > stability of release branches, unless patches that impact code<br>
> > already<br>
> > on release branches, get fixes on master and are back ported.<br>
> ><br>
> > Release branches get fixes back ported (as is normal),<br>
> this fix<br>
> > and its<br>
> > merge should not impact current master stability in any<br>
> way, and<br>
> > neither<br>
> > stability of 4.1 branch.<br>
> > </snip><br>
> ><br>
> > The current hold is on master, not on release branches. I<br>
> agree that<br>
> > merging further code changes on release branches (for example<br>
> > geo-rep<br>
> > issues that are backported (see [1]), as there are tests<br>
> that fail<br>
> > regularly on master), may further destabilize the release<br>
> > branch. This<br>
> > patch is not one of those.<br>
> ><br>
> ><br>
> > Two issues I have with the merge:<br>
> > 1. It just makes comparing master branch to release branch<br>
> harder. For<br>
> > example, to understand if there's a test that fails on master but<br>
> > succeeds on release branch, or vice versa. <br>
> > 2. It means we are not focused on stabilizing master branch.<br>
> > Y.<br>
> ><br>
> ><br>
> > Merging patches on release branches are allowed by release<br>
> > owners only,<br>
> > and usual practice is keeping the backlog low (merging weekly)<br>
> > in these<br>
> > cases as per the dashboard [1].<br>
> ><br>
> > Allowing for the above 2 reasons this patch was found,<br>
> > - Not on master<br>
> > - Not stabilizing or destabilizing the release branch<br>
> > and hence was merged.<br>
> ><br>
> > If maintainers disagree I can revert the same.<br>
> ><br>
> > Shyam<br>
> ><br>
> > [1] Release 4.1 dashboard:<br>
> ><br>
> > <br>
> <a href="https://review.gluster.org/#/projects/glusterfs,dashboards/dashboard:4-1-dashboard" rel="noreferrer" target="_blank">https://review.gluster.org/#/projects/glusterfs,dashboards/dashboard:4-1-dashboard</a><br>
> ><br>
> > _______________________________________________<br>
> > maintainers mailing list<br>
> > <a href="mailto:maintainers@gluster.org" target="_blank">maintainers@gluster.org</a> <mailto:<a href="mailto:maintainers@gluster.org" target="_blank">maintainers@gluster.org</a>><br>
> <mailto:<a href="mailto:maintainers@gluster.org" target="_blank">maintainers@gluster.org</a> <mailto:<a href="mailto:maintainers@gluster.org" target="_blank">maintainers@gluster.org</a>>><br>
> > <a href="https://lists.gluster.org/mailman/listinfo/maintainers" rel="noreferrer" target="_blank">https://lists.gluster.org/mailman/listinfo/maintainers</a><br>
> ><br>
> ><br>
> ><br>
> > --<br>
> > Pranith<br>
> <br>
> <br>
> <br>
> -- <br>
> Pranith<br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">Pranith<br></div></div></div>