Open Stack

I wonder what the turnaround of trivial patches actually is, I bet you it's
very very small, and as Daniel said, the human burden is rather minimal (I
would be more concerned about slowing them down in the gate, but I digress).

I think that introducing a two-tier level for patch approval can only
mitigate the problem, but I wonder if we'd need to go a lot further, and
rather figure out a way to borrow concepts from queueing theory so that
they can be applied in the context of Gerrit. For instance Little's law [1]
says:

"The long-term average number of customers (in this context *reviews*) in a
stable system L is equal to the long-term average effective arrival rate,
λ, multiplied by the average time a customer spends in the system, W; or
expressed algebraically: L = λW."

L can be used to determine the number of core reviewers that a project will
need at any given time, in order to meet a certain arrival rate and average
time spent in the queue. If the number of core reviewers is a lot less than
L then that core team is understaffed and will need to increase.

If we figured out how to model and measure Gerrit as a queuing system, then
we could improve its performance a lot more effectively; for instance, this
idea of privileging trivial patches over longer patches has roots in a
popular scheduling policy [3] for  M/G/1 queues, but that does not really
help aging of 'longer service time' patches and does not have a preemption
mechanism built-in to avoid starvation.

Just a crazy opinion...
Armando

[1] - http://en.wikipedia.org/wiki/Little's_law
[2] - http://en.wikipedia.org/wiki/Shortest_job_first
[3] - http://en.wikipedia.org/wiki/M/G/1_queue


On 17 June 2014 14:12, Matthew Booth <mbooth at redhat.com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 17/06/14 12:36, Sean Dague wrote:
> > On 06/17/2014 07:23 AM, Daniel P. Berrange wrote:
> >> On Tue, Jun 17, 2014 at 11:04:17AM +0100, Matthew Booth wrote:
> >>> We all know that review can be a bottleneck for Nova
> >>> patches.Not only that, but a patch lingering in review, no
> >>> matter how trivial, will eventually accrue rebases which sap
> >>> gate resources, developer time, and will to live.
> >>>
> >>> It occurs to me that there are a significant class of patches
> >>> which simply don't require the attention of a core reviewer.
> >>> Some examples:
> >>>
> >>> * Indentation cleanup/comment fixes * Simple code motion * File
> >>> permission changes * Trivial fixes which are obviously correct
> >>>
> >>> The advantage of a core reviewer is that they have experience
> >>> of the whole code base, and have proven their ability to make
> >>> and judge core changes. However, some fixes don't require this
> >>> level of attention, as they are self-contained and obvious to
> >>> any reasonable programmer.
> >>>
> >>> Without knowing anything of the architecture of gerrit, I
> >>> propose something along the lines of a '+1 (trivial)' review
> >>> flag. If a review gained some small number of these, I suggest
> >>> 2 would be reasonable, it would be equivalent to a +2 from a
> >>> core reviewer. The ability to set this flag would be a
> >>> privilege. However, the bar to gaining this privilege would be
> >>> low, and preferably automatically set, e.g. 5 accepted patches.
> >>> It would be removed for abuse.
> >>>
> >>> Is this practical? Would it help?
> >>
> >> You are right that some types of fix are so straightforward that
> >> most reasonable programmers can validate them. At the same time
> >> though, this means that they also don't really consume
> >> significant review time from core reviewers.  So having
> >> non-cores' approve trivial fixes wouldn't really reduce the
> >> burden on core devs.
> >>
> >> The main positive impact would probably be a faster turn around
> >> time on getting the patches approved because it is easy for the
> >> trivial fixes to drown in the noise.
> >>
> >> IME any non-trivial change to gerrit is just not going to happen
> >> in any reasonably useful timeframe though. Perhaps an
> >> alternative strategy would be to focus on identifying which the
> >> trivial fixes are. If there was an good way to get a list of all
> >> pending trivial fixes, then it would make it straightforward for
> >> cores to jump in and approve those simple patches as a priority,
> >> to avoid them languishing too long.
> >>
> >> If would be nice if gerrit had simple keyword tagging so any
> >> reviewer can tag an existing commit as "trivial", but that
> >> doesn't seem to exist as a concept yet.
> >>
> >> So an alternative perhaps submit trivial stuff using a well
> >> known topic eg
> >>
> >> # git review --topic trivial
> >>
> >> Then you can just query all changes in that topic to find easy
> >> stuff to approve.
> >
> > It could go in the commit message:
> >
> > TrivialFix
> >
> > Then could be queried with -
> > https://review.openstack.org/#/q/message:TrivialFix,n,z
> >
> > If a reviewer felt it wasn't a trivial fix, they could just edit
> > the commit message inline to drop it out.
>
> +1. If possible I'd update the query to filter out anything with a -1.
>
> Where do we document these things? I'd be happy to propose a docs update.
>
> Matt
> - --
> Matthew Booth
> Red Hat Engineering, Virtualisation Team
>
> Phone: +442070094448 (UK)
> GPG ID:  D33C3490
> GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlOgML0ACgkQNEHqGdM8NJCmpgCfbSy9bljyEqksjrJ7oRjE8LNH
> 8nUAoJBE5L+uAcQbew5ff/98eeqoRvW2
> =+SOM
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140617/f4e4e356/attachment.html>