Re: Last gasp - Mailing list pgsql-hackers

On Sat, Apr 07, 2012 at 04:51:03PM -0400, Robert Haas wrote:
> I think this basically just boils down to too many patches and not
> enough people.  I was interested in Command Triggers from the
> beginning of this CommitFest, and I would have liked to pick it up
> sooner, but there were a LOT of patches to work on for this
> CommitFest.  The first three CommitFests of this cycle each had
> between 52 and 60 patches, while this one had 106 which included
> several very complex and invasive patches, command triggers among
> them.  So there was just a lot more to do, and a number of the people
> who submitted all of those patches didn't do a whole lot to help
> review them, sometimes because they were still furiously rewriting
> their submissions.  It's not surprising that more patches + fewer
> reviewers = each patch getting less attention, or getting it later.
> 
> Even before this CommitFest, it's felt to me like this hasn't been a
> great cycle for reviewing.  I think we have generally had fewer people
> doing reviews than we did during the 9.0 and 9.1 cycles.  I think we
> had a lot of momentum with the CommitFest process when it was new, but
> three years on I think there's been some ebbing of the relative
> enthusiastic volunteerism that got off the ground.  I don't have a
> very good idea what to do about that, but I think it bears some
> thought.

http://wiki.postgresql.org/wiki/Running_a_CommitFest suggests marking a patch
Returned with Feedback after five consecutive days of Waiting on Author.  That
was a great tool for keeping things moving, and I think we should return to it
or a similar timer.  It's also an objective test approximating the subjective
"large patch needs too much rework" test.  One cure for insufficient review
help is to then ratchet down the permitted Waiting on Author days.  The queue
will clear faster, and patch authors will have clearer schedules to consider
assisting with review of the remaining patches.  Incidentally, I can't feel
too sorry for any patch bounced after one solid review when some other patch
gets an implicit bounce after incomplete review or no review.  No patch in the
latest CF has or will have that fate.  Several patches in CF 2011-11 did end
that way, and it wasn't the first incident thereof.

I liked Simon's idea[1] for increasing the review supply: make a community
policy that patch submitters shall furnish commensurate review effort.  If
review is available-freely-but-we-hope-you'll-help, then the supply relative
to patch submissions is unpredictable.  Feature sponsors should see patch
review as efficient collaborative development.  When patch authorship teams
spend part of their time reviewing other submissions with the expectation of
receiving comparable reviews of their own work, we get a superior final
product compared to allocating all that time to initial patch writing.  (The
details might need work.  For example, do we give breaks for new contributors
or self-sponsored authors?)

[1] http://archives.postgresql.org/pgsql-hackers/2012-03/msg01612.php

> When it's discovered
> and agreed that a patch has serious problems that can't be corrected
> in a few days, the response to that should be "crap, OK, see you next
> CommitFest".  When people aren't willing to accept that, then we end
> up having arguments.

Agreed.  If we can develop consensus on a litmus test for that condition, it
can and should be the author recognizing it and marking his own patch Returned
with Feedback.  When the author fails to do so, the reviewer should do it.
When neither of them take action, only then should it fall on the CF manager.
We have relied on the CF manager to make the bulk of these calls, and that's a
demonstrated recipe for patch-specific policy debates and CF manager burnout.

> I think the fact that we insist on good design and
> high-quality code is a real strength of this project and something
> that makes me proud to be associated with it.  Yeah, it's harder that
> way.  Yeah, sometimes it means that it takes longer before a given
> feature gets committed.  Yeah, some things don't get done at all.  But
> on the flip side, it means that when you use a PostgreSQL feature, you
> can pretty much count on it working.  And if by some chance it
> doesn't, you can count on it being an oversight that will be corrected
> in the next maintenance release rather than a design problem that will
> never get fixed.  I like that, and I think our users do too.

Fully agreed.  We could occasionally benefit from allowing "experimental"
features documented as liable to mutate in any later major release.  They
should exhibit the same high implementation quality, but we can relax somewhat
about perfecting a long-term user interface.  Take pg_autovacuum as a rough
example of this strategy from the past.

Thanks,
nm