Re: Last gasp - Mailing list pgsql-hackers

On Mon, Apr 09, 2012 at 09:25:36AM -0400, Robert Haas wrote:
> On Mon, Apr 9, 2012 at 1:38 AM, Noah Misch <noah@leadboat.com> wrote:
> > 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.
> 
> Fully agreed.  However, attempts by me to vigorously enforce that
> policy in previous releases met with resistance.  However, not
> enforcing it has led to the exact same amount of unhappiness by, well,
> more or less the exact same set of people.

Incidentally, a limit of ~8 total days in Waiting on Author might work better
than a limit of 5 consecutive days in Waiting on Author.  It would bring fewer
perverse incentives at the cost of taking more than a glance to calculate.

> > 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?)
> 
> I guess my problem is that I have always viewed it as the
> responsibility of patch submitters to furnish commensurate review
> effort.  The original intent of the CommitFest process was that
> everyone would stop working on their own patches and review other
> people's patches.  That's clearly not happening any more.

Maybe we'd just reemphasize/formalize that past understanding, then.

> Of course, part of the problem here is that it's very hard to enforce
> sanctions.  First, people don't like to be sanctioned and tend to
> argue about it, which is not only un-fun for the person attempting to
> impose the sanction but also chews up even more of the limited review
> time in argument.  Second, the standard is inevitably going to be
> fuzzy.  If person A submits a large patch and two small patches and
> reviews two medium-size patches and misses a serious design flaw in
> one of them that Tom spends four days fixing, what's the appropriate
> sanction for that?  Especially if their own patches are already
> committed?  Does it matter whether they missed the design flaw due to
> shoddy reviewing or just because most of us aren't as smart as Tom?  I
> mean, we can't go put time clocks on everyone's desk and measure the
> amount of time they spend on patch development and patch review and
> start imposing sanctions when that falls below some agreed-upon ratio.
>  In the absence of some ability to objectively measure people's
> contributions in this area, we rely on everyone's good faith.
> 
> So the we-should-require-people-to-review thing seems like a bit of a
> straw man to me.  It's news to me that any such policy has ever been
> lacking.  The thing is that, aside from the squishiness of the
> criteria, we have no enforcement mechanism.  As a result, some people
> choose to take advantage of the system, and the longer we fail to
> enforce, the more people go that route, somewhat understandably.

I don't envision need for sanctions based on missing things in reviews.  It
doesn't take much of a review to be better than nothing, so let's keep the
process friendly to new and tentative reviewers.  When an experienced hacker
misses something sufficiently-obvious, the self-recognition will regularly
motivate greater care going forward.  Nonetheless, yes, I don't see any of
this insulating us from bad faith or gaming of the system.  I tentatively
assume that we have people acting in good faith on perverse incentives, not
people acting in bad faith.

> David Fetter has floated the idea, a few times, of appointing a
> release manager who, AIUI, would be given dictatorial power to evict
> patches from the last CommitFest according to that person's technical
> judgement and ultimately at their personal discretion to make sure
> that the release happens in a timely fashion.  I remarked at the last
> developer meeting that I would be happy to have such a role, as long
> as I got to occupy it.  This was actually intended as a joking remark,
> but I think several people took it more seriously than I meant it.
> The point I was going for is: nobody really likes having a dictator,
> unless either (1) they themselves are the dictator or (2) the dictator
> is widely perceived as benevolent and impartial.  In reality, there
> are probably half a dozen people I would trust in such a role, or
> maybe it could be some small group of individuals.  I would in every
> way prefer not to have to go this route, because I think that
> self-policing is in every way better: less adversarial and, if people
> are honest with themselves and each other, more fair.  But the current
> system has become dysfunctional, so we're going to have to do
> something.

Right.  If this hypothetical dictator made all decisions justly, who could
complain?  But objective rules do not require a just judge, and they have a
different advantage: predictability.  If I know that a clock starts ticking
the moment I get my first review, I'll shape my personal plan accordingly.
That works even if I don't favor that timer to govern CFs.  There's no
comparable coping strategy under the administration of an unjust dictator.
Similarly, seeing a technicality kick a cherished patch from a final CF will
hurt, but that will feed back into earlier submissions of future large,
cherished patches from the same author.  When we let 10, 20, 30 days elapse
between patch revisions, authors have no advance idea how much slack is
available and proceed to contend for more of it as needed.  This bears
similarity to the contrast between civil law and common law legal systems.
Civil law systems have greater potential to remedy situation-specific
injustice, but they're less predictable.

Thanks,
nm