Skip site navigation (1) Skip section navigation (2)

Re: Performance Patches Was: Lock Wait Statistics (next commitfest)

From: Greg Smith <greg(at)2ndquadrant(dot)com>
To: Mark Kirkwood <mark(dot)kirkwood(at)catalyst(dot)net(dot)nz>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Mark Kirkwood <markir(at)paradise(dot)net(dot)nz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, gokul007(at)gmail(dot)com
Subject: Re: Performance Patches Was: Lock Wait Statistics (next commitfest)
Date: 2010-02-28 00:06:24
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-hackers
Mark Kirkwood wrote:
> While I completely agree that the submitter should be required to 
> supply a test case and their results, so the rest of us can try to 
> reproduce said improvement - rejecting the patch out of hand is a bit 
> harsh I feel - Hey, they may just have forgotten to supply these things! 
I didn't put any strong wording in the Wiki, I was just mentioning my 
personal position is far less tolerant of this than the current project 
policy.  What I added was:

"If the patch is intended to improve performance, it's a good idea to 
include some reproducible tests to demonstrate the improvement. If a 
reviewer cannot duplicate your claimed performance improvement in a 
short period of time, it's very likely your patch will be bounced. Do 
not expect that a reviewer is going to find your performance feature so 
interesting that they will build an entire test suite to prove it works. 
You should have done that as part of patch validation, and included the 
necessary framework for testing with the submission."

Finding a reviewer for a performance patch and getting them up to speed 
to evaluate any submitted patch is time intensive, and it really sucks 
from the perspective of the CF manager and any reviewer who is handed a 
messy one.  The intention was not to cut people off without warning 
them.  The position I would advocate as being a fair one is that if you 
don't provide a test case for a performance improvement patch, you can't 
then expect that you'll be assigned a reviewer by the CF manager either 
until that's corrected.  And if time for the CF runs out before you do 
that, you're automatically moved to "returned with 
feedback"--specifically, "write us a test case".

Greg Smith  2ndQuadrant US  Baltimore, MD
PostgreSQL Training, Services and Support

In response to

pgsql-hackers by date

Next:From: Marc G. FournierDate: 2010-02-28 00:21:57
Subject: Re: Anyone know if Alvaro is OK?
Previous:From: Greg SmithDate: 2010-02-27 23:49:09
Subject: Re: Hot Standby query cancellation and Streaming Replication integration

Privacy Policy | About PostgreSQL
Copyright © 1996-2018 The PostgreSQL Global Development Group