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