Simon Riggs wrote:
> There's a confusion in the patch between top level xid and parent xid.
> The xl field is named parentxid but actually contains top level. That
> distinction is important because the code now uses the top level xid to
> locate the recovery proc, formerly the role of the slotid.
True, I changed the meaning halfway through. My thinking was that we can
get away by only tracking the top-level xact of each subtransaction, not
the whole tree of subtransactions.
> This leads to an error when we SubTransSetParent(child_xid, top_xid);
> since this assumes that the top_xid is the parent, which it is not.
> Mostly you wouldn't notice unless you were looking up the subtrans
> status for an xid that had committed but was the child of an aborted
> subtransaction, with the top level xid having > 64 subtransactions.
Hmm. When a subtransaction aborts, RecordTransactionAbort is called and
clog is updated to show the subtransaction and all it's subcommitted
children as aborted. I think we're safe, though it wouldn't hurt to
double-check.
It's an important point that needs documenting, though. I completely
neglected that.
> I'm wasn't looking for ways to reintroduce slotid, but it seems more
> logical to keep slotid in light of the above. However, you will probably
> view this as intransigence, so I will await your input.
Yeah, it sure does seem like intransigence :-)
> For xl_rel_lock you add a field called xid and then ask
> /* xid of the *parent* transaction. XXX why parent? */.
> You've done this because it replaced slotid. But looking at that, I
> think the 6a patch had a bug there: a subtransaction abort record would
> release locks for the whole top level xact. So we need to pass both top
> level xid (or slotid) and xid for each lock, then release using the
> actual xid only.
Hmm, would just the subtransaction xid be enough? If we use that to
release, what do we need the top-level xid for?
-- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
