Re: Regression test failures

Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
> iirc the error I got was something along the line of:
> ERROR:  catalog is missing 1 attribute(s) for relid 17231

It's possible that that's the same problem but in a form triggered by
ALTER ADD COLUMN.

I was able to reproduce the problem I saw, and have now decided that
there are several interacting bugs involved.  Basically, the sequence
BEGIN;SAVEPOINT x;    CREATE TABLE foo ...;ROLLBACK TO x;

ought to *always* fail (bug #1) but chances not to do so because of bug
#2 --- except that there's a race condition (bug #3) which allows the
failure to emerge if some other backend has done the right thing during
a narrow time window.

Bug #1 is that relcache.c isn't accounting for subtransactions in its
handling of rd_isnew relcache entries.  In the above example, foo is
marked rd_isnew and so relcache.c tries to preserve the relcache entry
until transaction end.  The ROLLBACK will hit it with cache invalidation
actions telling it that the pg_class and pg_attribute entries for the
table have changed.  Normally that would cause the relcache entry to be
dropped, but since it's rd_isnew, relcache.c mulishly tries to rebuild
it instead.  So it's reading catalog entries that are now considered
deleted, and so the "deleted while in use" error is exactly what you'd
expect.

So why don't you get that all the time?  Well, bug #2 is that
TransactionIdIsCurrentTransactionId still considers the already-aborted
subtransaction ID to be current, so the validity tests in tqual.c will
think the catalog rows are still valid.

Except that there is a race condition.  If, between the time that the
subxact is marked aborted in pg_clog and the time relcache.c tries to
re-read these rows, some other backend comes along and examines the
pg_class row in question, it will mark the row as XMIN_INVALID, in
which case tqual.c doesn't bother to check
TransactionIdIsCurrentTransactionId but just declares the row no good.
So, with just the right sort of concurrent activity, it's possible to
observe the error.

I think your "catalog is missing 1 attribute" example might be the same
sort of thing, except the XMIN marking happened to a pg_attribute row
instead of a pg_class row.  (I'm not totally convinced by that
explanation though --- the failure should be transient rather than
repeatable, if this was the mechanism.)

After further thought I'm thinking that bug #2 is not so much whether
TransactionIdIsCurrentTransactionId is behaving correctly, but the fact
that it is being invoked at all.  We should never be doing catalog
accesses in transaction-aborted state.  The relcache code therefore
needs to be changed so that it doesn't try to do rebuilds immediately,
but waits until we are back in a "good" state (either out of the failed
subtransaction, or starting a whole fresh transaction if invalidation
happened at the end of a main transaction).  I think this bug exists
in existing releases too, for invalidation events affecting
nailed-in-cache system catalogs.  It's not clear you'd ever see an
actual failure in the field for that case, but it's still wrong.
        regards, tom lane