Re: Releasing in September

On Wed, Jan 20, 2016 at 01:59:59PM -0800, Peter Geoghegan wrote:
> I think that we've learned some lessons from the problems with 9.3. I
> don't think that one of those lessons was "take more time to release".
> There is reason to doubt that that would have changed matters one bit
> with 9.3. It might be a good idea to formally state what those lessons
> are.

Hindsight of 9.3 taught me to monitor three additional[1] signals for every
commit. Each is weak individually, firing often on harmless commits.
However, if commit 0ac5ad5 landed again today, I would at least prioritize
further analysis based on all three signals being present:

1. Patch came together shortly before a deadline.

The tree sees a bunch of feature commits just before feature freeze. This
list sees many fresh patch versions just before the last CommitFest submission
deadline. For an important subset, time pressure led the author or committer
to reduce vigilance. This is an analog signal, and I write "came together" to
be deliberately vague. The more the various steps of development work
(design, implementation, review, commit) cluster near some deadline, the more
to worry.

2. Committer disclaimed knowledge of the patch's correctness.

From commit 0ac5ad5 procedural history:
> Tom Lane said at one point "this is too complex to maintain". Several
> times during the development I feared he might well be right. I am sure
> he will be discovering many oversights and poor design choices, when
> gets around to reviewing the code; hopefully some extra effort will be
> all that's needed to make the whole thing work sanely and not eat
> anyone's data. I just hope that nothing so serious comes up that the
> patch will need to be reverted completely.

Author/committer personality is a large factor in determining whether this
signal arrives. If I do see it, I now believe the author about the patch
being dangerous.

3. Committer == author and no other committer certified the patch.

By "certify", I mean a direct statement like "This patch is correct." or "This
feature is now bug-free." etc. Posting a review does not, by itself, qualify,
and reviewers regularly do enough to earn a co-author credit without having
certified any version of the patch. Being listed as the patch's git
author/committer _does_ imply certification.

This signal is common, particularly for smaller patches, and it ought to
remain common. The product would advance slowly if every contrib module or
vacuumdb flag took full attention from two committers. Every release has a
few patches that do deserve this level of attention, though, and they rarely
attract it. Index-only scans did; commit a2822fb had a non-author committer
despite an author having a commit bit. Committers should recognize particular
self-authored work as too grave to self-certify, submit it for review, and not
commit it until and unless another committer certifies it. Indeed, I regard
that recognition of one's own limits as the central qualification for being a
committer. I have failed at it myself, though.

Watching those signals is the lesson I took from the PostgreSQL 9.3 aftermath.

Thanks,
nm

[1] They joined older signals, some too laborious to measure for every commit:
- complexity of the area(s) patched
- plausible scope of breakage from defects, given the area(s) patched
- gravity of follow-on repair commits