[openstack-dev] [cinder] L3 low pri review queue starvation

Doug Hellmann doug at doughellmann.com
Wed Sep 2 14:12:25 UTC 2015


Excerpts from Gorka Eguileor's message of 2015-09-02 11:19:02 +0200:
> On Tue, Sep 01, 2015 at 09:30:26AM -0600, John Griffith wrote:
> > On Tue, Sep 1, 2015 at 5:57 AM, Tom Barron <tpb at dyncloud.net> wrote:
> > 
> > > [Yesterday while discussing the following issue on IRC, jgriffith
> > > suggested that I post to the dev list in preparation for a discussion in
> > > Wednesday's cinder meeting.]
> > >
> > > Please take a look at the 10 "Low" priority reviews in the cinder
> > > Liberty 3 etherpad that were punted to Mitaka yesterday. [1]
> > >
> > > Six of these *never* [2] received a vote from a core reviewer. With the
> > > exception of the first in the list, which has 35 patch sets, none of the
> > > others received a vote before Friday, August 28.  Of these, none had
> > > more than -1s on minor issues, and these have been remedied.
> > >
> > > Review https://review.openstack.org/#/c/213855 "Implement
> > > manage/unmanage snapshot in Pure drivers" is a great example:
> > >
> > >    * approved blueprint for a valuable feature
> > >    * pristine code
> > >    * passes CI and Jenkins (and by the deadline)
> > >    * never reviewed
> > >
> > > We have 11 core reviewers, all of whom were very busy doing reviews
> > > during L3, but evidently this set of reviews didn't really have much
> > > chance of making it.  This looks like a classic case where the
> > > individually rational priority decisions of each core reviewer
> > > collectively resulted in starving the Low Priority review queue.
> > >
> 
> I can't speak for other cores, but in my case reviewing was mostly not
> based on my own priorities, I reviewed patches based on the already set
> priority of each patch as well as patches that I was already
> reviewing.
> 
> Some of those medium priority patches took me a lot of time to review,
> since they were not trivial (some needed some serious rework).  As for
> patches I was already reviewing, as you can imagine it wouldn't be fair
> to just ignore a patch that I've been reviewing for some time just when
> it's almost ready and the deadline is closing in.
> 
> Having said that I have to agree that those patches didn't have much
> chances, and I apologize for my part on that.  While it is no excuse I
> have to agree with jgriffith when he says that those patches should have
> pushed cores for reviews (even if this is clearly not the "right" way to
> manage it).
> 
> > > One way to remedy would be for the 11 core reviewers to devote a day or
> > > two to cleaning up this backlog of 10 outstanding reviews rather than
> > > punting all of them out to Mitaka.
> > >
> > > Thanks for your time and consideration.
> > >
> > > Respectfully,
> > >
> > > -- Tom Barron
> > >
> > > [1] https://etherpad.openstack.org/p/cinder-liberty-3-reviews
> > > [2] At the risk of stating the obvious, in this count I ignore purely
> > > procedural votes such as the final -2.
> > >
> > > __________________________________________________________________________
> > > OpenStack Development Mailing List (not for usage questions)
> > > Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> > > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> > >
> > 
> > ​Thanks Tom, this is sadly an ongoing problem every release.  I think we
> > have a number of things we can talk about at the summit to try and
> > make some of this better.  I honestly think that if people were to
> > actually "use" launchpad instead of creating tracking etherpads
> > everywhere it would help.  What I mean is that there is a ranked
> > targeting of items in Launchpad and we should use it, core team
> > members should know that as the source of truth and things that must
> > get reviewed.
> > 
> 
> I agree, we should use Launchpad's functionality to track BPs and Bugs
> targeted for each milestone, and maybe we can discuss on a workflow that
> helps us reduce starvation at the same time that helps us keep track of
> code reviewers responsible for each item.
> 
> Just spitballing here, but we could add to BP's work items and Bug's
> comments what core members will be responsible for reviewing related
> patches.  Although this means cores will have to check this on every
> review they do that has a BP or Bug number, so if there are already 2
> cores responsible for that feature they should preferably just move on
> to other patches and if there are not 2 core reviewers they should add
> themselves to LP.  This way patch owners know who they should bug for
> reviews on their patches and if there are no core reviewers for them
> they should look for some (or wait for them to "claim" that
> bug/feature).

I know some teams are using an etherpad to track that sort of
information, because priorities change from week to week (as items are
closed out) and it's easy to add arbitrary comments to the etherpad.
Maybe one of the teams doing that can comment on its effectiveness?

Doug



More information about the OpenStack-dev mailing list