Re: Store FullTransactionId in TwoPhaseFileHeader/GlobalTransactionData

Hi,

On 2019-08-05 13:15:19 +1200, Thomas Munro wrote:
> On Sat, Aug 3, 2019 at 12:06 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > On Fri, Aug 2, 2019 at 6:37 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> > > Thanks. This looks pretty reasonable to me, and I don't think we need
> > > to worry about the subxid list for now.
> >
> > Why not just do them all at once?
>
> [tries for a couple of hours and abandons for now]
>
> It's a bit of a can of worms. To do it properly, I think
> TransactionStateData::childXids needs to become a pointer to a
> FullTransactionId array called childFxids, so that
> xactGetCommittedChildren() can return it, and that causes knock on
> effects all over the tree, at least xactdesc.c, clog.c, commit_ts.c,
> transam.c, twophase.c, xact.c need adjusting and you finish up writing
> the subxact array into various places in the WAL in 64 bit format (but
> not yet the main xid). Alternatively you need to convert the array of
> FullTransactionId into an array of TransactionId in various places, or
> convert TransactionId into FullTransactionId just for the 2PC stuff,
> but both of those are cop outs and require allocating extra copies.
> Of course I am in favour of moving more things to 64 bit format, but I
> don't want to do them all at once, and there are a number of policy
> decisions hiding in there, and it's not strictly needed for the change
> that Vignesh proposes.

Hm. Maybe I'm missing something, but what's the point of changing this?
We're not anytime soon going to allow transactions that are old enough
that 32bit isn't enough to reference them. Nor do I think it's likely
we're going to convert the procarray to 64bit xids - keeping the size
down is too important for cache efficiency. And as most of the data
we're talking about here references *live* transactions, rather than
information that's needed for longer (as e.g. a row's xmin/xmax would),
I don't see why it'd be useful to convert to 64bit xids?

I do see a point in converting these 32bit xids to 64bit xids when
viewing them, i.e. have pg_prepared_xacts.transaction,
pg_stat_activity.backend_{xid,xmin}, ... return a 64bit xid.

> Vignesh's patch achieves something important on its own: it avoids the
> needs for zheap to do a 32->64 conversion.

Hm, is that an actual problem?

Greetings,

Andres Freund