Re: mvcc catalo gsnapshots and TopTransactionContext

On Tue, Aug 06, 2013 at 09:06:59AM +0200, Andres Freund wrote:
> On 2013-08-05 13:09:31 -0400, Noah Misch wrote:
> > When we call AtEOSubXact_Inval() or AtEOXact_Inval() with a relation still
> > open, we can potentially get a relcache rebuild and therefore a syscache
> > lookup as shown above. CommitSubTransaction() is also potentially affected,
> > though I don't have an SQL-level test case for that. It calls
> > CommandCounterIncrement() after moving to TRANS_COMMIT. That CCI had better
> > find no invalidations of open relations, or we'll make syscache lookups. (In
> > simple tests, any necessary invalidations tend to happen at the CCI in
> > CommitTransactionCommand(), so the later CCI does in fact find nothing to do.
> > I have little confidence that should be counted upon, though.)
>
> > How might we best rearrange things to avoid these hazards?
>
> Ok. After a good bit of code reading, I think this isn't an actual bug
> but an overzealous Assert(). I think it should be
> Assert(IsTransactionBlock()) not Assert(IsTransactionState());

IsTransactionBlock() is for higher-level things that care about actual use of
BEGIN. It's false in the middle of executing a single-statement transaction,
but that's of course a perfectly valid time for syscache lookups.

> 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.

Interesting.

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com