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

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
greg(at)2ndQuadrant(dot)com www.2ndQuadrant.us