Re: Something is broken about connection startup

I wrote:
> So it's happening when RelationCacheInitializePhase3 is trying to replace
> a fake pg_class row for pg_proc (made by formrdesc) with the real one.
> That's even odder, because that's late enough that this should be a pretty
> ordinary catalog lookup. Now I wonder if it's possible that this can be
> seen during ordinary relation opens after connection startup. If so, it
> would almost surely be a recently-introduced bug, else we'd have heard
> about this from the field.

Okay, I've been tracing through this, and the reason that the catalog
search for the real pg_proc row is failing is that all it finds are
versions too new for its snapshot to see. Thus, the failure happens
when the process running the GRANTs has pruned away a committed-dead
tuple that is the version the incoming process would have needed to see.
This is not the fault of the process running the GRANTs, because *the
incoming process is advertising MyPgXact->xmin = zero* which should mean
that it has no active snapshot. It has no right to complain that
somebody truncated away data.

The sequence of events inside the incoming process seems to go
approximately like this:

* RelationCacheInitializePhase3 goes to load up the pg_class row for some
other catalog (maybe pg_class itself; whatever hash_seq_search comes to
first). This results in systable_beginscan calling GetCatalogSnapshot
which calls GetSnapshotData which sets MyPgXact->xmin nonzero as a side
effect. All well so far.

* Once we've collected that row, systable_endscan unregisters the
snapshot, which ends with SnapshotResetXmin resetting MyPgXact->xmin
to zero because the RegisteredSnapshots list is now empty.

* RelationCacheInitializePhase3 now tries to read the pg_class row for
pg_proc. This results in systable_beginscan calling GetCatalogSnapshot,
which doesn't call GetSnapshotData this time, it just returns the
snapshot it's already got. This is a fatal mistake, because there is
nothing preventing data visible to that snapshot from being removed.

So this has pretty much been broken since we put in MVCC snapshots for
catalog searches. The problem would be masked when inside a transaction
that has already got a transaction snapshot, but whenever we don't have
one already, our catalog lookups are basically unprotected against
premature row removal. I don't see any reason to think that this is
specific to connection startup.

The basic problem here, therefore, is that SnapshotResetXmin isn't aware
that GetCatalogSnapshot is keeping a possibly-unregistered snapshot in
its hip pocket. That has to change. We could either treat the saved
CatalogSnapshot as always being registered, or we could add some logic
to force invalidating the CatalogSnapshot whenever we clear MyPgXact->xmin
or advance it past that snapshot's xmin.

Neither of those is very nice from a performance standpoint. With the
first option we risk delaying global cleanup by holding back
MyPgXact->xmin to protect a CatalogSnapshot we might not actually use
anytime soon. With the second option we will likely end up doing many
more GetSnapshotData calls than we do now, because in a transaction
that hasn't yet set a regular snapshot, we will need to get a new
CatalogSnapshot for every catalog access (since we'll go back to
MyPgXact->xmin = 0 after every access). And the parser, in particular,
tends to do a lot of catalog accesses before we ever get a regular
transaction snapshot.

Ideally, perhaps, we'd treat the saved CatalogSnapshot as registered
but automatically kill it "every so often". I'm not sure how to drive
that though.

Thoughts?

regards, tom lane