Re: lock_timeout GUC patch - Review

On Mon, Aug 2, 2010 at 3:09 PM, Kevin Grittner
<Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:
> Marc Cousin <cousinmarc(at)gmail(dot)com> wrote:
>
>> This time, it's this case that doesn't work :
>
>> I really feel that the timeout framework is the way to go here.
>
> Since Zoltán also seems to feel this way:
>
> http://archives.postgresql.org/message-id/4C516C3A.6090102@cybertec.at
>
> I wonder whether this patch shouldn't be rejected with a request
> that the timeout framework be submitted to the next CF.  Does anyone
> feel this approach (without the framework) should be pursued
> further?

I think "Returned with Feedback" would be more appropriate than
"Rejected", since we're asking for a rework, rather than saying - we
just don't want this. But otherwise, +1.

Generally, I'm of the opinion that patches needing significant rework
should be set to "Returned with Feedback" and resubmitted for the next
CF; otherwise, it just gets unmanageable. Our goal for a CF should be
to review everything thoroughly, not to get everything committed.
Otherwise, we end up with a never-ending train of what are effectively
new patches popping up, and it becomes impossible to close out the
CommitFest on time. Reviewers and committers end up getting slammed,
and it's not very much fun for patch authors (who are trying to
frantically do last-minute rewrites) either; nor is it good for the
overall quality of code the ends up in our tree. Better to take a
breather and then start fresh.

(If you don't believe in committer fatigue, look at the review I gave
Itagaki Takahiro on the partitioning patch in January vs. the review I
gave in July. One of those reviews is a whole lot more specific,
detailed, and accurate than the other one...)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company