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 <[email protected]> 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 > [email protected] > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >
_______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
