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