Re: mvcc catalo gsnapshots and TopTransactionContext

On 2013-08-08 09:27:24 -0400, Robert Haas wrote:
> On Tue, Aug 6, 2013 at 3:06 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > The reason for that is that when we do the AtEO(Sub)?Xact_Inval(), we've
> > already done a RecordTransactionAbort(true|false) and
> > CurrentTransactionState->state = TRANS_ABORT. So the visibility routines
> > have enough information to consider rows created by the aborted
> > transaction as invisible.
> >
> > I am not really happy with the RelationReloadIndexInfo()s in
> > RelationClearRelation() when we're in an aborted state, especially as
> > the comment surrounding them are clearly out of date, but I don't see a
> > bug there anymore.
>
> How can it be safe to try to read catalogs if the transaction is aborted?

Well. It isn't. At least not in general. The specific case triggered
here though are cache invalidations being processed which can lead to
the catalog being read (pretty crummy, but not easy to get rid
of). That's actually safe since before we process the invalidations we
have done:
1) CurrentTransactionState->state = TRANS_ABORT
2) RecordTransactionAbort(), marking the transaction as aborted in the
clog
3) marked subxacts as aborted
3) ProcArrayEndTransaction() (for toplevel ones)

Due to these any tqual stuff will treat the current (sub-)xact and it's
children as aborted. So the catalog lookups will use the catalog in a
sensible state.

Now, one could argue that it's certainly not safe for anything but
xact.c itself to play such games. And would be pretty damn right. We
could add some flat to signal catcache.c to temporarily use
Assert(IsTransactionBlock()) instead of IsTransactionStmt() but that
seems overly complex. I think the danger of code doing stuff in an
aborted transaction isn't that big.

This certainly deserves a good comment...

Greetings,

Andres Freund

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