Re: nested xacts and phantom Xids

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> > Here I present the nested transactions patch and the phantom Xids patch
> > that goes with it.
>
> I looked at the phantom XIDs stuff a bit. I still have little confidence
> that the concept is correct :-( but here are some comments on the code
> level.
>
> > + * They are marked immediately in pg_subtrans as direct childs of the topmost
> > + * Xid (no matter how deep we are in the transaction tree),
>
> Why is that a good idea --- won't it cause problems when you
> commit/abort an intermediate level of subtransaction?

I don't think so. The phantom xid is used only by the outside
transaction waiting for that tuple to be committe or aborted. The
outside tranaction will sleep on the topmost xid completing, then check
the phantom xid status for commit/abort. Within the transaction, I think
he uses command counter to know the creation and destruction sub-xid.

I think right now phantom xids are used only for making parts of a
subtransaction committed or aborted and only in cases where the tuple is
created and destroyed by parts of the same transaction tree.

I don't feel too bad about the runtime cost if only subtransactions are
paying that cost. I know we are really stretching the system here but I
would like to try a little more rather than give up and taking a space
hit for all tuples.

---------------------------------------------------------------------------

> > + * All this happens when HeapTupleHeaderSetXmax is called and the Xmin of the
> > + * tuple is one of the Xids of the current transaction.
> > + *
> > + * Note that HeapTupleHeaderGetXmax/GetXmin return the masqueraded Xmin and
> > + * Xmax, not the real one in the tuple header.
>
> I think that putting this sort of logic into Set/Get Xmin/Xmax is a
> horrid idea. They are supposed to be transparent fetch/store macros,
> not arbitrarily-complex filter functions. They're also supposed to
> be reasonably fast :-(
>
> > + * ... We know to do this when SetXmax is called
> > + * on a tuple that has the HEAP_MARKED_FOR_UPDATE bit set.
>
> Uglier and uglier.
>
> > ***************
> > *** 267,274 ****
> > return true;
> >
> > /* deleting subtransaction aborted */
> > ! if (TransactionIdDidAbort(HeapTupleHeaderGetXmax(tuple)))
> > ! return true;
> >
> > Assert(TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmax(tuple)));
> >
> > --- 268,276 ----
> > return true;
> >
> > /* deleting subtransaction aborted */
> > ! if (tuple->t_infomask & HEAP_XMIN_IS_PHANTOM)
> > ! if (TransactionPhantomIsCommittedPhantom(HeapTupleHeaderGetPhantomId(tuple)))
> > ! return true;
> >
> > Assert(TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmax(tuple)));
> >
>
> Er, what happened to checking for the "deleting subtransaction aborted"
> case?
>
> On the whole, I think this was an interesting exercise but also an utter
> failure. We'd be far better off to eat the extra 4 bytes per tuple
> header and put back the separate Xmax field. The costs in both runtime
> and complexity/reliability of this patch are simply not justified by
> a 4-byte savings.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073