On Wed, Jul 1, 2009 at 5:01 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> Kevin Grittner wrote:
>>> However, even the *possibility* that this could be true is pretty
>>> scary. If we need to effectively shut down new development for seven
>>> months at the end of a release, in addition to the interim commit
>>> fests, we'd better get a handle on why, so that can change. To what
>>> do you attribute the extended time needed to handle the final CF?
>>> How can that be made better?
>
>> We had many patches that had been through previous commit-fests with
>> minor adjustments and we had to finalize them before we could close the
>> final commit-fest. To be clear I am talking about patches that were
>> eventually applied in 8.4, not patches that were rejected for 8.4.
>
> I think this is simply not in agreement with the facts. The patches
> that caused the greatest amount of delay for 8.4 were the ones that
> ultimately got rejected --- notably hot standby, sync rep, and
> sepostgres. Now the fact that everybody knew they would take awhile
This is also how I remember it.
> provided some "cover" for other patches that weren't quite ready.
> If we had bounced those three on Nov. 1 the commit fest would've been
> a lot shorter. Probably some other things that did get in would've
> gotten bounced too, but on the whole I think the project would have been
> better off.
It wasn't just the big patches that dragged on forever: for example,
updateable views got submitted literally hours before the start of the
CommitFest in a state where it did not even pass regression tests, I
reviewed it, there was a loooong delay before resubmit, then it got
committed, then it got reverted. If we had ejected that patch from
the queue (for non-resubmission, if nothing else) early on in the
process, it would have freed up at least three people's time (Tom's,
mine, Peter's) to work on other patches that were in better shape and
submitted sooner.
One of the WORST mistakes that we made with the November CommitFest
was to only assign reviewers to the small patches, figuring that the
committers would look at the big ones. SE-PostgreSQL was not given a
reasonable review for a very long time. What we need to do this time
is start with the biggest patches and assign the most capable
reviewers (committers, preferably) to those patches and then work down
the list. This is another example of the uncomfortable dynamic
between committers and everyone else: non-committers don't feel
comfortable assigning committers to review patches, because who are we
to tell the commiters what to do? But when the committers are the
only ones competent to do that review, the result is a huge vacuum.
We need to put some structure around this that works.
I do agree, however, that rejecting more patches sooner (in
particular, the ones that had been reviewed and found wanting) would
have been the way to go and good for the project.
> The long and the short of it is that there is always tremendous pressure
> to include patches that are on the edge of being ready, because we all
> know that bouncing them to the next release cycle will mean an extra
> year before they're available in production. The dynamic in 8.4 was
> exactly the same as it's been in the prior release cycles: we keep
> slipping the possible release date and trying to get those patches
> ready, and we don't give up until everyone agrees the release is just
> hopelessly late. As long as we keep behaving that way, no amount of
> schedule-setting or rule-making is going to change anything.
Yep.
> It comes down to somebody having the willingness to say "no" and the
> authority to make it stick. Robert mentioned upthread that the
> committers don't want to be seen as throwing their weight around,
> which is quite true, but I have also noticed in the past that saying
> "no" does not convince whoever is arguing that "this release will suck
> if it doesn't have this feature". And there's always somebody arguing
> that side --- usually several people.
Yep. But having a review that clearly enumerates the reasons WHY the
patch needs to be rejected is certainly more compelling than a blanket
statement that the patch is big and invasive and therefore must have
bugs, even if we haven't looked at it enough to find them. The
blanket statement may be quite true, but it doesn't provide feedback
to the patch author and so fails to accomplish one of the main
purposes of the CommitFests, at least IMO.
...Robert