Re: Why don't we have a small reserved OID range for patch revisions? - Mailing list pgsql-hackers

From Peter Geoghegan
Subject Re: Why don't we have a small reserved OID range for patch revisions?
Date
Msg-id CAH2-Wz=Wvha=AZV0hMqFjTFuU7vexdpQi9P4w6RhxFNOYk9thQ@mail.gmail.com
Whole thread Raw
In response to Re: Why don't we have a small reserved OID range for patch revisions?  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Why don't we have a small reserved OID range for patch revisions?  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: Why don't we have a small reserved OID range for patch revisions?  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
List pgsql-hackers
On Wed, Feb 27, 2019 at 1:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > OID collision doesn't seem to be a significant problem (for me).
>
> Um, I beg to differ.  It's not at all unusual for pending patches to
> bit-rot for no reason other than suddenly getting an OID conflict.
> I don't have to look far for a current example:
> https://travis-ci.org/postgresql-cfbot/postgresql/builds/498955351

OID conflicts are not that big of a deal when building a patch
locally, because almost everyone knows what the exact problem is
immediately, and because you probably have more than a passing
interest in the patch to even do that much. However, the continuous
integration stuff has created an expectation that your patch shouldn't
be left to bitrot for long. Silly mechanical bitrot now seems like a
much bigger problem than it was before these developments. It unfairly
puts reviewers off engaging.

Patch authors shouldn't be left with any excuse for leaving their
patch to bitrot for long. And, more casual patch reviewers shouldn't
have any excuse for not downloading a patch and applying it locally,
so that they can spend a spare 10 minutes kicking the tires.

> Perhaps it'd be useful for genbki.pl to spit out a warning (NOT an
> error) if it sees OIDs in the reserved range.  I'm not sure that that'd
> really be worth the trouble though, since one could easily forget
> about it while reviewing/testing just before commit, and it'd just be
> useless noise up until it was time to commit.

My sense is that we should err on the side of being informative.

> Another issue, as Robert pointed out, is that this does need to be
> a formal convention not something undocumented.  Naylor's patch adds
> a mention of it in bki.sgml, but I wonder if anyplace else should
> talk about it.

Why not have unused_oids reference the convention as a "tip"?

> I concede your point that a prudent committer would do a rebuild and
> retest rather than just trusting the tool.  But really, how much
> extra work is that?  If you've spent any time optimizing your workflow,
> a full rebuild and check-world should be under five minutes on any
> hardware anyone would be using for development today.

If you use the "check-world parallel" recipe on the committing
checklist Wiki page, and if you use ccache, ~2 minutes is attainable
for optimized builds (though the recipe doesn't work on all release
branches). I don't think that a committer should be a committing
anything if they're not willing to do this much. It's not just prudent
-- it is the *bare minimum* when committing a patch that creates
system catalog entries.

-- 
Peter Geoghegan


pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Refactoring the checkpointer's fsync request queue
Next
From: Tom Lane
Date:
Subject: Re: Why don't we have a small reserved OID range for patch revisions?