Re: Problem with txid_snapshot_in/out() functionality

On 2014-04-12 09:47:24 -0400, Tom Lane wrote:
> Heikki Linnakangas <hlinnakangas@vmware.com> writes:
> > On 04/12/2014 12:07 AM, Jan Wieck wrote:
> >> the Slony team has been getting seldom reports of a problem with the
> >> txid_snapshot data type.
> >> The symptom is that a txid_snapshot on output lists the same txid
> >> multiple times in the xip list part of the external representation.
>
> > It's two-phase commit. When preparing a transaction, the state of the
> > transaction is first transfered to a dummy PGXACT entry, and then the
> > PGXACT entry of the backend is cleared. There is a transient state when
> > both PGXACT entries have the same xid.
>
> Hm, yeah, but why is that intermediate state visible to anyone else?
> Don't we have exclusive lock on the PGPROC array while we're doing this?

It's done outside the remit of ProcArray lock :(. And documented to lead
to duplicate xids in PGXACT.
EndPrepare():/* * Mark the prepared transaction as valid.    As soon as xact.c marks * MyPgXact as not running our XID
(whichit will do immediately after * this function returns), others can commit/rollback the xact. * * NB: a side effect
ofthis is to make a dummy ProcArray entry for the * prepared XID.  This must happen before we clear the XID from
MyPgXact,* else there is a window where the XID is not running according to * TransactionIdIsInProgress, and onlookers
wouldbe entitled to assume * the xact crashed.  Instead we have a window where the same XID appears * twice in
ProcArray,which is OK. */MarkAsPrepared(gxact);
 

It doesn't sound too hard to essentially move PrepareTransaction()'s
ProcArrayClearTransaction() into MarkAsPrepared() and rejigger the
locking to remove the intermediate state. But I think it'll lead to
bigger changes than we'd be comfortable backpatching.

> If we don't, aren't we letting other backends see non-self-consistent
> state in regards to who holds which locks, for example?

I think that actually works out ok, because the locks aren't owned by
xids/xacts, but procs. Otherwise we'd be in deep trouble in
CommitTransaction() as well where ProcArrayEndTransaction() clearing
that state.
After the whole xid transfer, there's PostPrepare_Locks() transferring
the locks.

Brr.

Greetings,

Andres Freund

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