Re: MVCC catalog access

On 2013-07-02 10:38:17 -0400, Robert Haas wrote:
> On Tue, Jul 2, 2013 at 9:52 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> >> > * possibly paranoid, but I'd add a Assert to heap_beginscan_catalog or
> >> >   GetCatalogSnapshot ensuring we're dealing with a catalog relation. The
> >> >   consistency mechanisms in GetCatalogSnapshot() only work for those, so
> >> >   there doesn't seem to be a valid usecase for non-catalog relations.
> >>
> >> It'll actually work find as things stand; it'll just take a new
> >> snapshot every time.
> >
> > Ok. Doesn't really change my opinion that it's a crappy idea to use it
> > otherwise ;)
> 
> I agree, but I don't see an easy way to write the assertion you want
> using only the OID.

Let's add

/** IsSystemRelationId*             True iff the relation is a system catalog relation.*/
bool
IsSystemRelationId(Oid relid)
{      return relid < FirstNormalObjectId;
}

and change IsSystemRelation() to use that instead of what it does now...

> >> - currtid_byreloid() and currtid_byrelname() use SnapshotNow as an
> >> argument to heap_get_latest_tid().  I don't know what these functions
> >> are supposed to be good for, but taking a new snapshot for every
> >> function call seems to guarantee that someone will find a way to use
> >> these functions as a poster child for how to brutalize PGXACT, so I
> >> don't particularly want to do that.
> >
> > Heikki mentioned that at some point they were added for the odbc
> > driver. I am not particularly inclined to worry about taking too many
> > snapshots here.
> 
> Well, if it uses it with any regularity I think it's a legitimate
> concern.   We have plenty of customers who use ODBC and some of them
> allow frightening numbers of concurrent server connections. 

I've quickly verified that it indeed uses them. I wish I hadn't. Brrr. I
can't even guess what that should do from the surrounding code/function
names. Except that it looks broken under concurrency as long as
SnapshotNow is used (because the query's snapshot won't be as new as
SnapshotNow, even in read committed mode).

Heikki, do you understand the code well enough to explain it without
investing time?

> Now you may say that's a bad idea, but so is 1000 backends doing
> txid_current() in a loop.

Hehe ;).

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services