Re: [XFS SUMMIT] Ugh, Rebasing Sucks!

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 27, 2020 at 07:44:10PM -0700, Darrick J. Wong wrote:
> On Thu, May 28, 2020 at 10:03:51AM +1000, Dave Chinner wrote:
> > On Wed, May 27, 2020 at 11:48:58AM -0700, Darrick J. Wong wrote:
> > > Hi everyone,
> > > 
> > > Many of you have complained (both publicly and privately) about the
> > > heavy cost of rebasing your development trees, particularly when you're
> > > getting close to sending a series out for review.  I get it, there have
> > > been a lot of large refactoring patchsets coming in the past few kernel
> > > cycles, and this has caused a lot of treewide churn.  I don't mind
> > > cleanups of things that have been weird and wonky about XFS for years,
> > > but, frankly, rebasing is soul-grinding.
> > > 
> > > To that end, I propose reducing the frequency of (my own) for-next
> > > pushes to reduce how often people feel compelled to rebase when they're
> > > trying to get a series ready for review.
> > > 
> > > Specifically, I would like to make an informal for-next push schedule as
> > > follows:
> > > 
> > >  1 Between -rc1 and -rc4, I'll collect critical bug fixes for the
> > >    merge window that just closed.  These should be small changes, so
> > >    I'll put them out incrementally with the goal of landing everything
> > >    in -rc4, and they shouldn't cause major disruptions for anyone else
> > >    working on a big patchset.  This is more or less what I've been doing
> > >    up till now -- if it's been on the list for > 24h and someone's
> > >    reviewed it, I'll put it in for-next for wider testing.
> > > 
> > >  2 A day or two after -rc4 drops.  This push is targeted for the next
> > >    merge window.  Coming three weeks after -rc1, I hope this will give
> > >    everyone enough time for a round of rebase, review, and debugging of
> > >    large changesets after -rc1.  IOWs, the majority of patchsets should
> > >    be ready to go in before we get halfway to the next merge window.
> > > 
> > >  3 Another push a day or two after -rc6 drops.  This will hopefully give
> > >    everyone a second chance to land patchsets that were nearly ready but
> > >    didn't quite make it for -rc4; or other cleanups that would have
> > >    interfered with the first round.  Once this is out, we're more or
> > >    less finished with the big patchsets.
> > 
> > This seems like a reasonable compromise - knowing when updates are
> > expected goes a long way to being able to plan development and
> > schedule dev tree updates to avoid repeated rebasing.
> > 
> > >  4 Perhaps another big push a day or two after -rc8 drops?  I'm not keen
> > >    on doing this.  It's not often that the kernel goes beyond -rc6 and I
> > >    find it really stressful when the -rc's drag on but people keep
> > >    sending large new patchsets.  Talk about stumbling around in the
> > >    dark...
> > 
> > IMO it's too late at -rc8 to be including big new changes for the
> > merge window. Bug fixes are fine, but not cleanups or features at
> > this point because there's too little test and soak time to catch
> > brown paper bag bugs before it's in the mainline tree and in much
> > more widespread use.
> 
> Fair enough.  I didn't really like this #4 anyway.  Withdrawn. :)
> 
> > Same goes for merging new stuff during the merge window - last time
> > around we had updates right up to the merge window, then an update
> > during the merge window for a second pull request. There just wasn't
> > any time when the tree wasn't actively moving forward.
> 
> Urk, sorry about that... I was hoping to land a fix for $largeclient
> but then the crazy just kept coming.  Never gonna do /that/ again. :/
> 
> > From my perspective, an update from for-next after the -rc6 update
> > gets me all the stuff that will be in the next release. That's the
> > major rebase for my work, and everything pulled in from for-next
> > starts getting test coverage a couple of weeks out from the merge
> > window.  Once the merge window closes, another local update to the
> > -rc1 kernel (which should be a no-op for all XFS work) then gets
> > test coverage for the next release. -rc1 to -rc4 is when
> > review/rework for whatever I want merged in -rc4/-rc6 would get
> > posted to the list....
> 
> <nod>
> 
> My workflow is rather different -- I rebase my dev tree off the latest
> rc every week, and when a series is ready I port it to a branch off of
> for-next.  Occasionally I'll port a refactoring from for-next into my
> dev tree to keep the code bases similar.  Both trees get run through
> fstests and $whatnot whenever they change, which mean that most mornings
> I'm looking at nightlies.
> 
> > This means there's a single rebase event a cycle at -rc6, and the
> > rest of the time the tree is pretty stable and the base tree I'm
> > testing is almost always the tree that we need to focus dev testing
> > on. That is, just before the merge window everyone should be testing
> > for-next on a -rc6/-rc7 base, and once -rc1 is out, everyone should
> > be testing that kernel through to ~-rc4 at which point it has
> > largely stabilised and the cycle starts again....
> > 
> > >  5 Obviously, I wouldn't hold back on critical bug fixes to things that
> > >    are broken in for-next, since the goal is to promote testing, not
> > >    hinder it.
> > 
> > *nod*
> > 
> > > Hopefully this will cut down on the "arrrgh I was almost ready to send
> > > this but then for-next jumped and nggghghghg" feelings. :/
> > > 
> > > Thoughts?  Flames?
> > 
> > Perhaps:
> > 
> > - each patch set that is posted should start with "this is aimed at
> >   a 5.x.y-rc4/-rc6 merge" or "still work in progress" so that
> >   everyone has some expectation of when changes are likely to land.
> 
> <nod> This would probably help with peoples' ability to distinguish
> djwong patchbombs for submission vs. making backups on NYE. ;)
> 

ISTM that anything posted that isn't intended to go through the typical
review -> merge cycle should simply be tagged RFC. That includes things
with obvious implementation gaps, prototypes looking for design
discussion, patchbomb backups, etc. The details can always be described
in the cover letter, but RFC helps reviewers prioritize based on that
information and immediately indicates to the maintainer that this isn't
on the merge track.

That's a bit different than an explicit release target, which I
personally find a bit dubious given that I think it could discourage
wider review on patches. IOW, I'd be a little concerned that patches
would start being merged ASAP after meeting some minimum criteria for
the current release (i.e. 24 hours + a review, noted above) as opposed
to more common sense behavior where more reviews might be ideal (and/or
likely based on time on list) based on the complexity of a particular
feature, etc. Just my .02, though.

Do note that I personally rarely ever know or care at what point in a
release we're at when posting patches to the list. I post patches when
ready for review, acknowledge that merge is dependent on review and
expect further review feedback can come at any time (not immediately)
and lead to any number of releases for a particular series. Therefore, I
ultimately care more about getting something merged into the pipeline
such that release is imminent vs. whether it lands in this release or
the next. IMO, the purpose of the rolling release cycle is to separate
the development process from the release process as such. That aside, my
main point here is to indicate that if I do happen to post something
"last minute" as such, I most likely have no idea, have no expectation
beyond hitting the next "reasonable deadline" and thus do not intend to
put implicit pressure on the maintainer. :)

Brian

> > or:
> > 
> > - aim to land features and complex bug fixes in -rc4 and cleanups in
> >   -rc6, that way we naturally minimise the rebase work for the
> >   features/bug fixes that are being landed. This may mean that -rc4
> >   is a small merge if there are no features/bug fixes that meet the
> >   -rc4 merge criteria...
> 
> I like that idea.
> 
> --D
> 
> > Cheers,
> > 
> > Dave.
> > -- 
> > Dave Chinner
> > david@xxxxxxxxxxxxx
> 




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux