On Thu, Aug 8, 2013 at 1:28 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> 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...
Do you want to propose something?
I basically don't have a very good feeling about this. Processing
invalidations should invalidate stuff, not try to reread it. You may
be right that our MVCC snapshot is OK at the point invalidations are
processed, but we don't know what caused the transaction to abort in
the first place.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
