Open Stack

2015-09-02 17:19 GMT+08:00 Gorka Eguileor <geguileo at redhat.com>:
> 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).

Like Gorka's idea here. This cloud be a better way to relieve this issue.
I hope patch owners could require cores' help via IRC or something else if there
are no core reviewers.
>
>> As far as Liberty and your patches; Yesterday was the freeze point, the
>> entire Cinder team agreed on that (yourself included both at the mid-cycle
>> meet up and at the team meeting two weeks ago when Thingee reiterated the
>> deadlines).  If you noticed last week that your patches weren't going
>> anywhere YOU should've wrangled up some reviews.
>>
>> Furthermore, I've explained every release for the last 3 or 4 years that
>> there's no silver bullet, no magic process when it come to review
>> throughput.  ESPECIALLY when it comes to the 3'rd milestone.  You can try
>> landing strips, priority listed etherpads, sponsors etc etc but the fact is
>> that things happen, the gate slows down (or we completely break on the
>> Cinder side like we did yesterday).  This doesn't mean "oh, well then you
>> get another day or two", it means stuff happens and it sucks but first
>> course of action is drop low priority items.  It just means if you really
>> wanted it you probably should've made it happen earlier.  Just so you know,
>> I run into this every release as well.  I had a number of things in
>> progress that I had hoped to finish last week and yesterday, BUT my
>> priority shifted to trying to help get the cinder patches back on track and
>> get the items in Launchpad updated to actually reflect something that was
>> somewhat possible.
>>
>> The only thing that works is "submit early and review often" it's simple.
>>
>
> While this is true, sometimes it's not a matter of submitting early,
> because some patches just keep getting ignored and when deadline closes
> in low priority patches will always have higher priority patches that
> will go before them, unless we set a workflow that will give them a
> better chance of getting reviews.
>
>> Finally, I pointed out to you yesterday that we could certainly discuss as
>> a team what to do with your patches.  BUT that given how terribly far
>> behind we were in the process that I wanted reviewers to focus on medium,
>> high and critical prioritized items.  That's what prioritization's are for,
>> it means when crunch time hits and things hit the fan it's usually the
>> "low" priority things that get bumped.
>>
>> Thanks,
>
> I am not against discussing this, but in all fairness we should discuss
> *everybody's* patches that were targeted for L3, not just tbarron's.
>
> Cheers,
> Gorka.
>
> __________________________________________________________________________
> 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



-- 
Best Wishes For You!