On 8/21/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Tom Lane wrote:
> >> (I wouldn't do it like this though --- TransactionIdAdvance itself is
> >> the place to bump the secondary counter.)
>
> > Agreed.
>
> I reconsidered after trying to do it that way --- although fixing
> TransactionIdAdvance itself to maintain a 2-word counter isn't hard,
> there are a whole lot of other places that can advance nextXid,
> mostly bits like this in WAL recovery:
>
> /* Make sure nextXid is beyond any XID mentioned in the record */
> max_xid = xid;
> for (i = 0; i < xlrec->nsubxacts; i++)
> {
> if (TransactionIdPrecedes(max_xid, sub_xids[i]))
> max_xid = sub_xids[i];
> }
> if (TransactionIdFollowsOrEquals(max_xid,
> ShmemVariableCache->nextXid))
> {
> ShmemVariableCache->nextXid = max_xid;
> TransactionIdAdvance(ShmemVariableCache->nextXid);
> }
>
> We could hack all these places to know about maintaining an XID-epoch
> value, but it's not looking like a simple single-place-to-touch fix :-(
As I was asked to rework the patch, I planned to use
TransactionIdAdvance(ShmemVariableCache), although that would
be conceptually ugly. Right Thing for this approach would be
to have special struct, but that would touch half the codebase.
That was also the reason I did not want to go that path.
> There's still a lot more cruft in the submitted patch than I think
> belongs in core, but I'll work on extracting something we can apply.
The only cruft I see is the snapshot on-disk "compression" and maybe
the pg_sync_txid() funtionality. Dropping the compression would not
matter much, snapshots would waste space, but at least for our
usage it would not be a problem. The reast of the functions are all
required for efficient handling.
Dropping the pg_sync_txid() would be loss, because that means that
user cannot just dump and restore the data and just continue where
it left off. Maybe its not a problem for replication but for generic
queueing it would need delicate juggling when restoring backup.
Although I must admit the pg_sync_txid() is indeed ugly part
of the patch, and it creates new mode for failure - wrapping
epoch. So I can kind of agree for removing it.
I hope you don't mean that none of the user-level functions belong
to core. It's not like there is several ways to expose the info.
And it not like there are much more interesting ways for using
the long xid in C level. Having long xid available in SQL level
means that efficient async replication can be done without any
use of C.
Now that I am back from vacation I can do some coding myself,
if you give hints what needs rework.
--
marko