Store FullTransactionId in TwoPhaseFileHeader/GlobalTransactionData
Hi,
In the undo system, we use full-transaction-id for transactions. For
rollback of prepared transactions, we were planning to use
FullTransactionId by combining TransactionId and epoch, but as
suggested by multiple people in that email chain [1]/messages/by-id/CA+hUKGJ+Pg2gE9Hdt6fXHn6ezV7xJnS+rm-38ksXZGXYcZh3Gg@mail.gmail.com[2]/messages/by-id/CAA4eK1L9BhvnQfa_RJCTpKQf9QZ15pyUW7s32BH78iBC3KbV0g@mail.gmail.com, the better
idea is to store Full-transactionid in TwoPhaseFileHeader
Backward compatibility need not be handled for this scnario as upgrade
does not support having open prepared transactions.
There is also one more comment which is yet to be concluded. The
comment discusses about changing subxids which are of TransactionId
type to FullTransactionId type being written in two phase transaction
file. We could not conclude this as the data is similarly stored in
TransactionStateData.
Please find the patch having the fix for Storing FullTransactionId in
TwoPhaseFileHeader/GlobalTransactionData.
Let me know your opinion on the patch and above comment.
[1]: /messages/by-id/CA+hUKGJ+Pg2gE9Hdt6fXHn6ezV7xJnS+rm-38ksXZGXYcZh3Gg@mail.gmail.com
/messages/by-id/CA+hUKGJ+Pg2gE9Hdt6fXHn6ezV7xJnS+rm-38ksXZGXYcZh3Gg@mail.gmail.com
[2]: /messages/by-id/CAA4eK1L9BhvnQfa_RJCTpKQf9QZ15pyUW7s32BH78iBC3KbV0g@mail.gmail.com
/messages/by-id/CAA4eK1L9BhvnQfa_RJCTpKQf9QZ15pyUW7s32BH78iBC3KbV0g@mail.gmail.com
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
Attachments:
Store_FullTransactionId_TwoPhaseFileHeader_GlobalTransactionData.patchapplication/octet-stream; name=Store_FullTransactionId_TwoPhaseFileHeader_GlobalTransactionData.patchDownload
From 267ac9cbf9e8d7989486118818d751e3d224919f Mon Sep 17 00:00:00 2001
From: Vigneshwaran c <vignesh21@gmail.com>
Date: Thu, 1 Aug 2019 11:25:39 +0530
Subject: [PATCH] Store FullTransactionId in
TwoPhaseFileHeader/GlobalTransactionData
In the undo system, we use full-transaction-id for transactions. For
rollback of prepared transactions, we were planning to use
FullTransactionId by combining TransactionId and epoch, but as
suggested by multiple people in undo email chain, the better
idea is to store Full-transactionid in TwoPhaseFileHeader.
---
src/backend/access/transam/twophase.c | 53 ++++++++++++++++++-----------------
src/backend/access/transam/xact.c | 4 +--
src/include/access/transam.h | 1 +
src/include/access/twophase.h | 7 +++--
4 files changed, 35 insertions(+), 30 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 477709b..f9494f2 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -163,7 +163,7 @@ typedef struct GlobalTransactionData
*/
XLogRecPtr prepare_start_lsn; /* XLOG offset of prepare record start */
XLogRecPtr prepare_end_lsn; /* XLOG offset of prepare record end */
- TransactionId xid; /* The GXACT id */
+ FullTransactionId full_xid; /* The GXACT full_xid */
Oid owner; /* ID of user that executed the xact */
BackendId locking_backend; /* backend currently working on the xact */
@@ -224,7 +224,7 @@ static void XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len);
static char *ProcessTwoPhaseBuffer(TransactionId xid,
XLogRecPtr prepare_start_lsn,
bool fromdisk, bool setParent, bool setNextXid);
-static void MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid,
+static void MarkAsPreparingGuts(GlobalTransaction gxact, FullTransactionId full_xid,
const char *gid, TimestampTz prepared_at, Oid owner,
Oid databaseid);
static void RemoveTwoPhaseFile(TransactionId xid, bool giveWarning);
@@ -370,7 +370,7 @@ PostPrepare_Twophase(void)
* Reserve the GID for the given transaction.
*/
GlobalTransaction
-MarkAsPreparing(TransactionId xid, const char *gid,
+MarkAsPreparing(FullTransactionId full_xid, const char *gid,
TimestampTz prepared_at, Oid owner, Oid databaseid)
{
GlobalTransaction gxact;
@@ -421,7 +421,7 @@ MarkAsPreparing(TransactionId xid, const char *gid,
gxact = TwoPhaseState->freeGXacts;
TwoPhaseState->freeGXacts = gxact->next;
- MarkAsPreparingGuts(gxact, xid, gid, prepared_at, owner, databaseid);
+ MarkAsPreparingGuts(gxact, full_xid, gid, prepared_at, owner, databaseid);
gxact->ondisk = false;
@@ -444,7 +444,7 @@ MarkAsPreparing(TransactionId xid, const char *gid,
* Note: This function should be called with appropriate locks held.
*/
static void
-MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
+MarkAsPreparingGuts(GlobalTransaction gxact, FullTransactionId full_xid, const char *gid,
TimestampTz prepared_at, Oid owner, Oid databaseid)
{
PGPROC *proc;
@@ -463,8 +463,8 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
SHMQueueElemInit(&(proc->links));
proc->waitStatus = STATUS_OK;
/* We set up the gxact's VXID as InvalidBackendId/XID */
- proc->lxid = (LocalTransactionId) xid;
- pgxact->xid = xid;
+ proc->lxid = (LocalTransactionId) XidFromFullTransactionId(full_xid);
+ pgxact->xid = XidFromFullTransactionId(full_xid);
pgxact->xmin = InvalidTransactionId;
pgxact->delayChkpt = false;
pgxact->vacuumFlags = 0;
@@ -485,7 +485,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
pgxact->nxids = 0;
gxact->prepared_at = prepared_at;
- gxact->xid = xid;
+ gxact->full_xid = full_xid;
gxact->owner = owner;
gxact->locking_backend = MyBackendId;
gxact->valid = false;
@@ -915,7 +915,7 @@ typedef struct TwoPhaseFileHeader
{
uint32 magic; /* format identifier */
uint32 total_len; /* actual file length */
- TransactionId xid; /* original transaction XID */
+ FullTransactionId full_xid; /* original full transaction XID */
Oid database; /* OID of database it was in */
TimestampTz prepared_at; /* time of preparation */
Oid owner; /* user running the transaction */
@@ -1004,8 +1004,6 @@ void
StartPrepare(GlobalTransaction gxact)
{
PGPROC *proc = &ProcGlobal->allProcs[gxact->pgprocno];
- PGXACT *pgxact = &ProcGlobal->allPgXact[gxact->pgprocno];
- TransactionId xid = pgxact->xid;
TwoPhaseFileHeader hdr;
TransactionId *children;
RelFileNode *commitrels;
@@ -1028,7 +1026,7 @@ StartPrepare(GlobalTransaction gxact)
/* Create header */
hdr.magic = TWOPHASE_MAGIC;
hdr.total_len = 0; /* EndPrepare will fill this in */
- hdr.xid = xid;
+ hdr.full_xid = gxact->full_xid;
hdr.database = proc->databaseId;
hdr.prepared_at = gxact->prepared_at;
hdr.owner = gxact->owner;
@@ -1346,7 +1344,7 @@ ParsePrepareRecord(uint8 info, char *xlrec, xl_xact_parsed_prepare *parsed)
parsed->origin_lsn = hdr->origin_lsn;
parsed->origin_timestamp = hdr->origin_timestamp;
- parsed->twophase_xid = hdr->xid;
+ parsed->twophase_xid = XidFromFullTransactionId(hdr->full_xid);
parsed->dbId = hdr->database;
parsed->nsubxacts = hdr->nsubxacts;
parsed->nrels = hdr->ncommitrels;
@@ -1442,7 +1440,8 @@ StandbyTransactionIdIsPrepared(TransactionId xid)
/* Check header also */
hdr = (TwoPhaseFileHeader *) buf;
- result = TransactionIdEquals(hdr->xid, xid);
+ result = TransactionIdEquals(
+ XidFromFullTransactionId(hdr->full_xid), xid);
pfree(buf);
return result;
@@ -1493,7 +1492,8 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
* Disassemble the header area
*/
hdr = (TwoPhaseFileHeader *) buf;
- Assert(TransactionIdEquals(hdr->xid, xid));
+ Assert(TransactionIdEquals(XidFromFullTransactionId(
+ hdr->full_xid), xid));
bufptr = buf + MAXALIGN(sizeof(TwoPhaseFileHeader));
bufptr += MAXALIGN(hdr->gidlen);
children = (TransactionId *) bufptr;
@@ -1791,7 +1791,8 @@ CheckPointTwoPhase(XLogRecPtr redo_horizon)
int len;
XlogReadTwoPhaseData(gxact->prepare_start_lsn, &buf, &len);
- RecreateTwoPhaseFile(gxact->xid, buf, len);
+ RecreateTwoPhaseFile(
+ XidFromFullTransactionId(gxact->full_xid), buf, len);
gxact->ondisk = true;
gxact->prepare_start_lsn = InvalidXLogRecPtr;
gxact->prepare_end_lsn = InvalidXLogRecPtr;
@@ -1911,7 +1912,7 @@ PrescanPreparedTransactions(TransactionId **xids_p, int *nxids_p)
Assert(gxact->inredo);
- xid = gxact->xid;
+ xid = XidFromFullTransactionId(gxact->full_xid);
buf = ProcessTwoPhaseBuffer(xid,
gxact->prepare_start_lsn,
@@ -1986,7 +1987,7 @@ StandbyRecoverPreparedTransactions(void)
Assert(gxact->inredo);
- xid = gxact->xid;
+ xid = XidFromFullTransactionId(gxact->full_xid);
buf = ProcessTwoPhaseBuffer(xid,
gxact->prepare_start_lsn,
@@ -2029,7 +2030,7 @@ RecoverPreparedTransactions(void)
TransactionId *subxids;
const char *gid;
- xid = gxact->xid;
+ xid = XidFromFullTransactionId(gxact->full_xid);
/*
* Reconstruct subtrans state for the transaction --- needed because
@@ -2050,7 +2051,7 @@ RecoverPreparedTransactions(void)
(errmsg("recovering prepared transaction %u from shared memory", xid)));
hdr = (TwoPhaseFileHeader *) buf;
- Assert(TransactionIdEquals(hdr->xid, xid));
+ Assert(FullTransactionIdEquals(hdr->full_xid, gxact->full_xid));
bufptr = buf + MAXALIGN(sizeof(TwoPhaseFileHeader));
gid = (const char *) bufptr;
bufptr += MAXALIGN(hdr->gidlen);
@@ -2064,7 +2065,7 @@ RecoverPreparedTransactions(void)
* Recreate its GXACT and dummy PGPROC. But, check whether it was
* added in redo and already has a shmem entry for it.
*/
- MarkAsPreparingGuts(gxact, xid, gid,
+ MarkAsPreparingGuts(gxact, gxact->full_xid, gid,
hdr->prepared_at,
hdr->owner, hdr->database);
@@ -2185,7 +2186,8 @@ ProcessTwoPhaseBuffer(TransactionId xid,
/* Deconstruct header */
hdr = (TwoPhaseFileHeader *) buf;
- if (!TransactionIdEquals(hdr->xid, xid))
+ if (!TransactionIdEquals(XidFromFullTransactionId(
+ hdr->full_xid), xid))
{
if (fromdisk)
ereport(ERROR,
@@ -2426,7 +2428,7 @@ PrepareRedoAdd(char *buf, XLogRecPtr start_lsn,
gxact->prepared_at = hdr->prepared_at;
gxact->prepare_start_lsn = start_lsn;
gxact->prepare_end_lsn = end_lsn;
- gxact->xid = hdr->xid;
+ gxact->full_xid = hdr->full_xid;
gxact->owner = hdr->owner;
gxact->locking_backend = InvalidBackendId;
gxact->valid = false;
@@ -2445,7 +2447,8 @@ PrepareRedoAdd(char *buf, XLogRecPtr start_lsn,
false /* backward */ , false /* WAL */ );
}
- elog(DEBUG2, "added 2PC data in shared memory for transaction %u", gxact->xid);
+ elog(DEBUG2, "added 2PC data in shared memory for transaction %u",
+ XidFromFullTransactionId(gxact->full_xid));
}
/*
@@ -2471,7 +2474,7 @@ PrepareRedoRemove(TransactionId xid, bool giveWarning)
{
gxact = TwoPhaseState->prepXacts[i];
- if (gxact->xid == xid)
+ if (XidFromFullTransactionId(gxact->full_xid) == xid)
{
Assert(gxact->inredo);
found = true;
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 1bbaeee..0b03156 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2413,8 +2413,8 @@ PrepareTransaction(void)
* Reserve the GID for this transaction. This could fail if the requested
* GID is invalid or already in use.
*/
- gxact = MarkAsPreparing(xid, prepareGID, prepared_at,
- GetUserId(), MyDatabaseId);
+ gxact = MarkAsPreparing(GetCurrentFullTransactionId(), prepareGID,
+ prepared_at, GetUserId(), MyDatabaseId);
prepareGID = NULL;
/*
diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index 33fd052..1128326 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -48,6 +48,7 @@
#define XidFromFullTransactionId(x) ((uint32) (x).value)
#define U64FromFullTransactionId(x) ((x).value)
#define FullTransactionIdPrecedes(a, b) ((a).value < (b).value)
+#define FullTransactionIdEquals(a, b) ((a).value == (b).value)
#define FullTransactionIdIsValid(x) TransactionIdIsValid(XidFromFullTransactionId(x))
#define InvalidFullTransactionId FullTransactionIdFromEpochAndXid(0, InvalidTransactionId)
diff --git a/src/include/access/twophase.h b/src/include/access/twophase.h
index b9a531c..6be6e7c 100644
--- a/src/include/access/twophase.h
+++ b/src/include/access/twophase.h
@@ -37,9 +37,10 @@ extern void PostPrepare_Twophase(void);
extern PGPROC *TwoPhaseGetDummyProc(TransactionId xid, bool lock_held);
extern BackendId TwoPhaseGetDummyBackendId(TransactionId xid, bool lock_held);
-extern GlobalTransaction MarkAsPreparing(TransactionId xid, const char *gid,
- TimestampTz prepared_at,
- Oid owner, Oid databaseid);
+extern GlobalTransaction MarkAsPreparing(FullTransactionId full_xid,
+ const char *gid,
+ TimestampTz prepared_at,
+ Oid owner, Oid databaseid);
extern void StartPrepare(GlobalTransaction gxact);
extern void EndPrepare(GlobalTransaction gxact);
--
1.8.3.1
Hi Vignesh,
On Thu, Aug 1, 2019 at 9:32 PM vignesh C <vignesh21@gmail.com> wrote:
In the undo system, we use full-transaction-id for transactions. For
rollback of prepared transactions, we were planning to use
FullTransactionId by combining TransactionId and epoch, but as
suggested by multiple people in that email chain [1][2], the better
idea is to store Full-transactionid in TwoPhaseFileHeader
+1
Backward compatibility need not be handled for this scnario as upgrade
does not support having open prepared transactions.
+1
There is also one more comment which is yet to be concluded. The
comment discusses about changing subxids which are of TransactionId
type to FullTransactionId type being written in two phase transaction
file. We could not conclude this as the data is similarly stored in
TransactionStateData.
No comment on that question or the patch yet but could you please add
this to the next Commitfest so that cfbot starts testing it?
--
Thomas Munro
https://enterprisedb.com
On Thu, Aug 1, 2019 at 5:36 PM Thomas Munro <thomas.munro@gmail.com> wrote:
Hi Vignesh,
On Thu, Aug 1, 2019 at 9:32 PM vignesh C <vignesh21@gmail.com> wrote:
In the undo system, we use full-transaction-id for transactions. For
rollback of prepared transactions, we were planning to use
FullTransactionId by combining TransactionId and epoch, but as
suggested by multiple people in that email chain [1][2], the better
idea is to store Full-transactionid in TwoPhaseFileHeader+1
Backward compatibility need not be handled for this scnario as upgrade
does not support having open prepared transactions.+1
There is also one more comment which is yet to be concluded. The
comment discusses about changing subxids which are of TransactionId
type to FullTransactionId type being written in two phase transaction
file. We could not conclude this as the data is similarly stored in
TransactionStateData.No comment on that question or the patch yet but could you please add
this to the next Commitfest so that cfbot starts testing it?
I have added it to the commitfest.
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
On Fri, Aug 2, 2019 at 1:38 AM vignesh C <vignesh21@gmail.com> wrote:
On Thu, Aug 1, 2019 at 5:36 PM Thomas Munro <thomas.munro@gmail.com> wrote:
On Thu, Aug 1, 2019 at 9:32 PM vignesh C <vignesh21@gmail.com> wrote:
There is also one more comment which is yet to be concluded. The
comment discusses about changing subxids which are of TransactionId
type to FullTransactionId type being written in two phase transaction
file. We could not conclude this as the data is similarly stored in
TransactionStateData.No comment on that question or the patch yet but could you please add
this to the next Commitfest so that cfbot starts testing it?I have added it to the commitfest.
Thanks. This looks pretty reasonable to me, and I don't think we need
to worry about the subxid list for now. Here's a version I ran though
pgindent to fix some whitespace problems.
The pg_prepared_xacts view seems like as good a place as any to start
showing FullTransactionId values to users. Here's an experimental
patch on top to do that, introducing a new "xid8" type. After
pg_resetwal -e 1000000000 -D pgdata you can make transactions that
look like this:
postgres=# select transaction, gid from pg_prepared_xacts ;
transaction | gid
---------------------+-----
4294967296000000496 | tx1
(1 row)
--
Thomas Munro
https://enterprisedb.com
Attachments:
0003-Use-xid8-in-the-pg_prepared_xacts-view.patchapplication/octet-stream; name=0003-Use-xid8-in-the-pg_prepared_xacts-view.patchDownload
From 3211410e7efe4ab5b623e31037b9d22f2d476f00 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Fri, 2 Aug 2019 22:17:56 +1200
Subject: [PATCH 3/3] Use xid8 in the pg_prepared_xacts view.
Show full transaction IDs in the "transaction" column. This can be
explicitly cast to xid to get the old behavior.
---
src/backend/access/transam/twophase.c | 6 +++---
src/include/catalog/pg_proc.dat | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 89e98be7b4..07d480f734 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -107,6 +107,7 @@
#include "utils/builtins.h"
#include "utils/memutils.h"
#include "utils/timestamp.h"
+#include "utils/xid8.h"
/*
@@ -737,7 +738,7 @@ pg_prepared_xact(PG_FUNCTION_ARGS)
/* this had better match pg_prepared_xacts view in system_views.sql */
tupdesc = CreateTemplateTupleDesc(5);
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "transaction",
- XIDOID, -1, 0);
+ XID8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2, "gid",
TEXTOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 3, "prepared",
@@ -769,7 +770,6 @@ pg_prepared_xact(PG_FUNCTION_ARGS)
{
GlobalTransaction gxact = &status->array[status->currIdx++];
PGPROC *proc = &ProcGlobal->allProcs[gxact->pgprocno];
- PGXACT *pgxact = &ProcGlobal->allPgXact[gxact->pgprocno];
Datum values[5];
bool nulls[5];
HeapTuple tuple;
@@ -784,7 +784,7 @@ pg_prepared_xact(PG_FUNCTION_ARGS)
MemSet(values, 0, sizeof(values));
MemSet(nulls, 0, sizeof(nulls));
- values[0] = TransactionIdGetDatum(pgxact->xid);
+ values[0] = FullTransactionIdGetDatum(gxact->full_xid);
values[1] = CStringGetTextDatum(gxact->gid);
values[2] = TimestampTzGetDatum(gxact->prepared_at);
values[3] = ObjectIdGetDatum(gxact->owner);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 7c9d434673..17e5eb4543 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5859,7 +5859,7 @@
{ oid => '1065', descr => 'view two-phase transactions',
proname => 'pg_prepared_xact', prorows => '1000', proretset => 't',
provolatile => 'v', prorettype => 'record', proargtypes => '',
- proallargtypes => '{xid,text,timestamptz,oid,oid}',
+ proallargtypes => '{xid8,text,timestamptz,oid,oid}',
proargmodes => '{o,o,o,o,o}',
proargnames => '{transaction,gid,prepared,ownerid,dbid}',
prosrc => 'pg_prepared_xact' },
--
2.22.0
0001-Use-FullTransactionId-for-two-phase-commit.patchapplication/octet-stream; name=0001-Use-FullTransactionId-for-two-phase-commit.patchDownload
From 0ec22e855f95c9817bb56c795188c5dfdd957b4c Mon Sep 17 00:00:00 2001
From: Vigneshwaran c <vignesh21@gmail.com>
Date: Thu, 1 Aug 2019 11:25:39 +0530
Subject: [PATCH 1/3] Use FullTransactionId for two phase commit.
Store full transaction IDs in the files and data structures used
for two phase commit. For now subtransactions are still stored in
32 bit format.
Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm2fFeQC_SbM_LsSPNZG3HZ0SC34szY+451iW7oysAb8Cw@mail.gmail.com
---
src/backend/access/transam/twophase.c | 50 +++++++++++++--------------
src/backend/access/transam/xact.c | 4 +--
src/include/access/transam.h | 1 +
src/include/access/twophase.h | 3 +-
4 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 477709bbc2..89e98be7b4 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -163,7 +163,7 @@ typedef struct GlobalTransactionData
*/
XLogRecPtr prepare_start_lsn; /* XLOG offset of prepare record start */
XLogRecPtr prepare_end_lsn; /* XLOG offset of prepare record end */
- TransactionId xid; /* The GXACT id */
+ FullTransactionId full_xid; /* The GXACT full xid */
Oid owner; /* ID of user that executed the xact */
BackendId locking_backend; /* backend currently working on the xact */
@@ -224,7 +224,7 @@ static void XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len);
static char *ProcessTwoPhaseBuffer(TransactionId xid,
XLogRecPtr prepare_start_lsn,
bool fromdisk, bool setParent, bool setNextXid);
-static void MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid,
+static void MarkAsPreparingGuts(GlobalTransaction gxact, FullTransactionId full_xid,
const char *gid, TimestampTz prepared_at, Oid owner,
Oid databaseid);
static void RemoveTwoPhaseFile(TransactionId xid, bool giveWarning);
@@ -370,7 +370,7 @@ PostPrepare_Twophase(void)
* Reserve the GID for the given transaction.
*/
GlobalTransaction
-MarkAsPreparing(TransactionId xid, const char *gid,
+MarkAsPreparing(FullTransactionId full_xid, const char *gid,
TimestampTz prepared_at, Oid owner, Oid databaseid)
{
GlobalTransaction gxact;
@@ -421,7 +421,7 @@ MarkAsPreparing(TransactionId xid, const char *gid,
gxact = TwoPhaseState->freeGXacts;
TwoPhaseState->freeGXacts = gxact->next;
- MarkAsPreparingGuts(gxact, xid, gid, prepared_at, owner, databaseid);
+ MarkAsPreparingGuts(gxact, full_xid, gid, prepared_at, owner, databaseid);
gxact->ondisk = false;
@@ -444,7 +444,7 @@ MarkAsPreparing(TransactionId xid, const char *gid,
* Note: This function should be called with appropriate locks held.
*/
static void
-MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
+MarkAsPreparingGuts(GlobalTransaction gxact, FullTransactionId full_xid, const char *gid,
TimestampTz prepared_at, Oid owner, Oid databaseid)
{
PGPROC *proc;
@@ -463,8 +463,8 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
SHMQueueElemInit(&(proc->links));
proc->waitStatus = STATUS_OK;
/* We set up the gxact's VXID as InvalidBackendId/XID */
- proc->lxid = (LocalTransactionId) xid;
- pgxact->xid = xid;
+ proc->lxid = (LocalTransactionId) XidFromFullTransactionId(full_xid);
+ pgxact->xid = XidFromFullTransactionId(full_xid);
pgxact->xmin = InvalidTransactionId;
pgxact->delayChkpt = false;
pgxact->vacuumFlags = 0;
@@ -485,7 +485,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
pgxact->nxids = 0;
gxact->prepared_at = prepared_at;
- gxact->xid = xid;
+ gxact->full_xid = full_xid;
gxact->owner = owner;
gxact->locking_backend = MyBackendId;
gxact->valid = false;
@@ -915,7 +915,7 @@ typedef struct TwoPhaseFileHeader
{
uint32 magic; /* format identifier */
uint32 total_len; /* actual file length */
- TransactionId xid; /* original transaction XID */
+ FullTransactionId full_xid; /* original full transaction XID */
Oid database; /* OID of database it was in */
TimestampTz prepared_at; /* time of preparation */
Oid owner; /* user running the transaction */
@@ -1004,8 +1004,6 @@ void
StartPrepare(GlobalTransaction gxact)
{
PGPROC *proc = &ProcGlobal->allProcs[gxact->pgprocno];
- PGXACT *pgxact = &ProcGlobal->allPgXact[gxact->pgprocno];
- TransactionId xid = pgxact->xid;
TwoPhaseFileHeader hdr;
TransactionId *children;
RelFileNode *commitrels;
@@ -1028,7 +1026,7 @@ StartPrepare(GlobalTransaction gxact)
/* Create header */
hdr.magic = TWOPHASE_MAGIC;
hdr.total_len = 0; /* EndPrepare will fill this in */
- hdr.xid = xid;
+ hdr.full_xid = gxact->full_xid;
hdr.database = proc->databaseId;
hdr.prepared_at = gxact->prepared_at;
hdr.owner = gxact->owner;
@@ -1346,7 +1344,7 @@ ParsePrepareRecord(uint8 info, char *xlrec, xl_xact_parsed_prepare *parsed)
parsed->origin_lsn = hdr->origin_lsn;
parsed->origin_timestamp = hdr->origin_timestamp;
- parsed->twophase_xid = hdr->xid;
+ parsed->twophase_xid = XidFromFullTransactionId(hdr->full_xid);
parsed->dbId = hdr->database;
parsed->nsubxacts = hdr->nsubxacts;
parsed->nrels = hdr->ncommitrels;
@@ -1442,7 +1440,7 @@ StandbyTransactionIdIsPrepared(TransactionId xid)
/* Check header also */
hdr = (TwoPhaseFileHeader *) buf;
- result = TransactionIdEquals(hdr->xid, xid);
+ result = TransactionIdEquals(XidFromFullTransactionId(hdr->full_xid), xid);
pfree(buf);
return result;
@@ -1493,7 +1491,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
* Disassemble the header area
*/
hdr = (TwoPhaseFileHeader *) buf;
- Assert(TransactionIdEquals(hdr->xid, xid));
+ Assert(TransactionIdEquals(XidFromFullTransactionId(hdr->full_xid), xid));
bufptr = buf + MAXALIGN(sizeof(TwoPhaseFileHeader));
bufptr += MAXALIGN(hdr->gidlen);
children = (TransactionId *) bufptr;
@@ -1791,7 +1789,8 @@ CheckPointTwoPhase(XLogRecPtr redo_horizon)
int len;
XlogReadTwoPhaseData(gxact->prepare_start_lsn, &buf, &len);
- RecreateTwoPhaseFile(gxact->xid, buf, len);
+ RecreateTwoPhaseFile(XidFromFullTransactionId(gxact->full_xid),
+ buf, len);
gxact->ondisk = true;
gxact->prepare_start_lsn = InvalidXLogRecPtr;
gxact->prepare_end_lsn = InvalidXLogRecPtr;
@@ -1911,7 +1910,7 @@ PrescanPreparedTransactions(TransactionId **xids_p, int *nxids_p)
Assert(gxact->inredo);
- xid = gxact->xid;
+ xid = XidFromFullTransactionId(gxact->full_xid);
buf = ProcessTwoPhaseBuffer(xid,
gxact->prepare_start_lsn,
@@ -1986,7 +1985,7 @@ StandbyRecoverPreparedTransactions(void)
Assert(gxact->inredo);
- xid = gxact->xid;
+ xid = XidFromFullTransactionId(gxact->full_xid);
buf = ProcessTwoPhaseBuffer(xid,
gxact->prepare_start_lsn,
@@ -2029,7 +2028,7 @@ RecoverPreparedTransactions(void)
TransactionId *subxids;
const char *gid;
- xid = gxact->xid;
+ xid = XidFromFullTransactionId(gxact->full_xid);
/*
* Reconstruct subtrans state for the transaction --- needed because
@@ -2050,7 +2049,7 @@ RecoverPreparedTransactions(void)
(errmsg("recovering prepared transaction %u from shared memory", xid)));
hdr = (TwoPhaseFileHeader *) buf;
- Assert(TransactionIdEquals(hdr->xid, xid));
+ Assert(FullTransactionIdEquals(hdr->full_xid, gxact->full_xid));
bufptr = buf + MAXALIGN(sizeof(TwoPhaseFileHeader));
gid = (const char *) bufptr;
bufptr += MAXALIGN(hdr->gidlen);
@@ -2064,7 +2063,7 @@ RecoverPreparedTransactions(void)
* Recreate its GXACT and dummy PGPROC. But, check whether it was
* added in redo and already has a shmem entry for it.
*/
- MarkAsPreparingGuts(gxact, xid, gid,
+ MarkAsPreparingGuts(gxact, gxact->full_xid, gid,
hdr->prepared_at,
hdr->owner, hdr->database);
@@ -2185,7 +2184,7 @@ ProcessTwoPhaseBuffer(TransactionId xid,
/* Deconstruct header */
hdr = (TwoPhaseFileHeader *) buf;
- if (!TransactionIdEquals(hdr->xid, xid))
+ if (!TransactionIdEquals(XidFromFullTransactionId(hdr->full_xid), xid))
{
if (fromdisk)
ereport(ERROR,
@@ -2426,7 +2425,7 @@ PrepareRedoAdd(char *buf, XLogRecPtr start_lsn,
gxact->prepared_at = hdr->prepared_at;
gxact->prepare_start_lsn = start_lsn;
gxact->prepare_end_lsn = end_lsn;
- gxact->xid = hdr->xid;
+ gxact->full_xid = hdr->full_xid;
gxact->owner = hdr->owner;
gxact->locking_backend = InvalidBackendId;
gxact->valid = false;
@@ -2445,7 +2444,8 @@ PrepareRedoAdd(char *buf, XLogRecPtr start_lsn,
false /* backward */ , false /* WAL */ );
}
- elog(DEBUG2, "added 2PC data in shared memory for transaction %u", gxact->xid);
+ elog(DEBUG2, "added 2PC data in shared memory for transaction %u",
+ XidFromFullTransactionId(gxact->full_xid));
}
/*
@@ -2471,7 +2471,7 @@ PrepareRedoRemove(TransactionId xid, bool giveWarning)
{
gxact = TwoPhaseState->prepXacts[i];
- if (gxact->xid == xid)
+ if (XidFromFullTransactionId(gxact->full_xid) == xid)
{
Assert(gxact->inredo);
found = true;
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 1bbaeeebf4..9a61c53ff8 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2413,8 +2413,8 @@ PrepareTransaction(void)
* Reserve the GID for this transaction. This could fail if the requested
* GID is invalid or already in use.
*/
- gxact = MarkAsPreparing(xid, prepareGID, prepared_at,
- GetUserId(), MyDatabaseId);
+ gxact = MarkAsPreparing(GetCurrentFullTransactionId(), prepareGID,
+ prepared_at, GetUserId(), MyDatabaseId);
prepareGID = NULL;
/*
diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index 33fd052156..1128326aa7 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -48,6 +48,7 @@
#define XidFromFullTransactionId(x) ((uint32) (x).value)
#define U64FromFullTransactionId(x) ((x).value)
#define FullTransactionIdPrecedes(a, b) ((a).value < (b).value)
+#define FullTransactionIdEquals(a, b) ((a).value == (b).value)
#define FullTransactionIdIsValid(x) TransactionIdIsValid(XidFromFullTransactionId(x))
#define InvalidFullTransactionId FullTransactionIdFromEpochAndXid(0, InvalidTransactionId)
diff --git a/src/include/access/twophase.h b/src/include/access/twophase.h
index b9a531c96e..9a3cc573db 100644
--- a/src/include/access/twophase.h
+++ b/src/include/access/twophase.h
@@ -37,7 +37,8 @@ extern void PostPrepare_Twophase(void);
extern PGPROC *TwoPhaseGetDummyProc(TransactionId xid, bool lock_held);
extern BackendId TwoPhaseGetDummyBackendId(TransactionId xid, bool lock_held);
-extern GlobalTransaction MarkAsPreparing(TransactionId xid, const char *gid,
+extern GlobalTransaction MarkAsPreparing(FullTransactionId full_xid,
+ const char *gid,
TimestampTz prepared_at,
Oid owner, Oid databaseid);
--
2.22.0
0002-Add-SQL-type-xid8-to-expose-FullTransactionId-to-use.patchapplication/octet-stream; name=0002-Add-SQL-type-xid8-to-expose-FullTransactionId-to-use.patchDownload
From 2d8b7f94b167c0ff8cd8b31f2933f0e5e8f8d18a Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Fri, 2 Aug 2019 17:29:34 +1200
Subject: [PATCH 2/3] Add SQL type xid8 to expose FullTransactionId to users.
Similar to xid, but 64 bits wide. This new type is suitable for use
in various system views and administration functions.
Author: Thomas Munro
Discussion: https://postgr.es/m/https://www.postgresql.org/message-id/20190725000636.666m5mad25wfbrri%40alap3.anarazel.de
---
src/backend/access/hash/hashvalidate.c | 3 +
src/backend/utils/adt/xid.c | 74 ++++++++++++++++++++++++
src/fe_utils/print.c | 1 +
src/include/access/transam.h | 10 ++++
src/include/catalog/pg_amop.dat | 4 ++
src/include/catalog/pg_amproc.dat | 4 ++
src/include/catalog/pg_cast.dat | 4 ++
src/include/catalog/pg_opclass.dat | 2 +
src/include/catalog/pg_operator.dat | 8 +++
src/include/catalog/pg_opfamily.dat | 2 +
src/include/catalog/pg_proc.dat | 21 +++++++
src/include/catalog/pg_type.dat | 4 ++
src/include/utils/xid8.h | 22 +++++++
src/test/regress/expected/opr_sanity.out | 2 +
14 files changed, 161 insertions(+)
create mode 100644 src/include/utils/xid8.h
diff --git a/src/backend/access/hash/hashvalidate.c b/src/backend/access/hash/hashvalidate.c
index 9315872751..8ecbd074b2 100644
--- a/src/backend/access/hash/hashvalidate.c
+++ b/src/backend/access/hash/hashvalidate.c
@@ -313,6 +313,9 @@ check_hash_func_signature(Oid funcid, int16 amprocnum, Oid argtype)
(argtype == DATEOID ||
argtype == XIDOID || argtype == CIDOID))
/* okay, allowed use of hashint4() */ ;
+ else if ((funcid == F_HASHINT8 || funcid == F_HASHINT8EXTENDED) &&
+ (argtype == XID8OID))
+ /* okay, allowed use of hashint8() */ ;
else if ((funcid == F_TIMESTAMP_HASH ||
funcid == F_TIMESTAMP_HASH_EXTENDED) &&
argtype == TIMESTAMPTZOID)
diff --git a/src/backend/utils/adt/xid.c b/src/backend/utils/adt/xid.c
index 7853e41865..3d7c249b1c 100644
--- a/src/backend/utils/adt/xid.c
+++ b/src/backend/utils/adt/xid.c
@@ -21,6 +21,7 @@
#include "access/xact.h"
#include "libpq/pqformat.h"
#include "utils/builtins.h"
+#include "utils/xid8.h"
#define PG_GETARG_TRANSACTIONID(n) DatumGetTransactionId(PG_GETARG_DATUM(n))
#define PG_RETURN_TRANSACTIONID(x) return TransactionIdGetDatum(x)
@@ -147,6 +148,79 @@ xidComparator(const void *arg1, const void *arg2)
return 0;
}
+Datum
+xid8toxid(PG_FUNCTION_ARGS)
+{
+ FullTransactionId fxid = PG_GETARG_FULLTRANSACTIONID(0);
+
+ PG_RETURN_TRANSACTIONID(XidFromFullTransactionId(fxid));
+}
+
+Datum
+xid8in(PG_FUNCTION_ARGS)
+{
+ char *str = PG_GETARG_CSTRING(0);
+ char *end;
+ uint64 value;
+
+ value = pg_strtouint64(str, &end, 10);
+ if (*str == '\0' || *end != '\0')
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg("value \"%s\" is invalid for type xid8", str)));
+
+ PG_RETURN_FULLTRANSACTIONID(FullTransactionIdFromU64(value));
+}
+
+Datum
+xid8out(PG_FUNCTION_ARGS)
+{
+ FullTransactionId fxid = PG_GETARG_FULLTRANSACTIONID(0);
+ char *result = (char *) palloc(21);
+
+ snprintf(result, 21, UINT64_FORMAT, U64FromFullTransactionId(fxid));
+ PG_RETURN_CSTRING(result);
+}
+
+Datum
+xid8recv(PG_FUNCTION_ARGS)
+{
+ StringInfo buf = (StringInfo) PG_GETARG_POINTER(0);
+ uint64 value;
+
+ value = (uint64) pq_getmsgint64(buf);
+ PG_RETURN_FULLTRANSACTIONID(FullTransactionIdFromU64(value));
+}
+
+Datum
+xid8send(PG_FUNCTION_ARGS)
+{
+ FullTransactionId arg1 = PG_GETARG_FULLTRANSACTIONID(0);
+ StringInfoData buf;
+
+ pq_begintypsend(&buf);
+ pq_sendint64(&buf, (uint64) U64FromFullTransactionId(arg1));
+ PG_RETURN_BYTEA_P(pq_endtypsend(&buf));
+}
+
+Datum
+xid8eq(PG_FUNCTION_ARGS)
+{
+ FullTransactionId fxid1 = PG_GETARG_FULLTRANSACTIONID(0);
+ FullTransactionId fxid2 = PG_GETARG_FULLTRANSACTIONID(1);
+
+ PG_RETURN_BOOL(FullTransactionIdEquals(fxid1, fxid2));
+}
+
+Datum
+xid8neq(PG_FUNCTION_ARGS)
+{
+ FullTransactionId fxid1 = PG_GETARG_FULLTRANSACTIONID(0);
+ FullTransactionId fxid2 = PG_GETARG_FULLTRANSACTIONID(1);
+
+ PG_RETURN_BOOL(!FullTransactionIdEquals(fxid1, fxid2));
+}
+
/*****************************************************************************
* COMMAND IDENTIFIER ROUTINES *
*****************************************************************************/
diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index e41f42ea98..9fe7e0a27e 100644
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -3512,6 +3512,7 @@ column_type_alignment(Oid ftype)
case NUMERICOID:
case OIDOID:
case XIDOID:
+ case XID8OID:
case CIDOID:
case CASHOID:
align = 'r';
diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index 1128326aa7..c9a30255e3 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -72,6 +72,16 @@ FullTransactionIdFromEpochAndXid(uint32 epoch, TransactionId xid)
return result;
}
+static inline FullTransactionId
+FullTransactionIdFromU64(uint64 value)
+{
+ FullTransactionId result;
+
+ result.value = value;
+
+ return result;
+}
+
/* advance a transaction ID variable, handling wraparound correctly */
#define TransactionIdAdvance(dest) \
do { \
diff --git a/src/include/catalog/pg_amop.dat b/src/include/catalog/pg_amop.dat
index 232557ee81..2a232f1608 100644
--- a/src/include/catalog/pg_amop.dat
+++ b/src/include/catalog/pg_amop.dat
@@ -1009,6 +1009,10 @@
{ amopfamily => 'hash/xid_ops', amoplefttype => 'xid', amoprighttype => 'xid',
amopstrategy => '1', amopopr => '=(xid,xid)', amopmethod => 'hash' },
+# xid_ops
+{ amopfamily => 'hash/xid8_ops', amoplefttype => 'xid8', amoprighttype => 'xid8',
+ amopstrategy => '1', amopopr => '=(xid8,xid8)', amopmethod => 'hash' },
+
# cid_ops
{ amopfamily => 'hash/cid_ops', amoplefttype => 'cid', amoprighttype => 'cid',
amopstrategy => '1', amopopr => '=(cid,cid)', amopmethod => 'hash' },
diff --git a/src/include/catalog/pg_amproc.dat b/src/include/catalog/pg_amproc.dat
index 5e705019b4..fbe1667292 100644
--- a/src/include/catalog/pg_amproc.dat
+++ b/src/include/catalog/pg_amproc.dat
@@ -339,6 +339,10 @@
amprocrighttype => 'xid', amprocnum => '1', amproc => 'hashint4' },
{ amprocfamily => 'hash/xid_ops', amproclefttype => 'xid',
amprocrighttype => 'xid', amprocnum => '2', amproc => 'hashint4extended' },
+{ amprocfamily => 'hash/xid8_ops', amproclefttype => 'xid8',
+ amprocrighttype => 'xid8', amprocnum => '1', amproc => 'hashint8' },
+{ amprocfamily => 'hash/xid8_ops', amproclefttype => 'xid8',
+ amprocrighttype => 'xid8', amprocnum => '2', amproc => 'hashint8extended' },
{ amprocfamily => 'hash/cid_ops', amproclefttype => 'cid',
amprocrighttype => 'cid', amprocnum => '1', amproc => 'hashint4' },
{ amprocfamily => 'hash/cid_ops', amproclefttype => 'cid',
diff --git a/src/include/catalog/pg_cast.dat b/src/include/catalog/pg_cast.dat
index aabfa7af03..6dc856230f 100644
--- a/src/include/catalog/pg_cast.dat
+++ b/src/include/catalog/pg_cast.dat
@@ -93,6 +93,10 @@
{ castsource => 'bool', casttarget => 'int4', castfunc => 'int4(bool)',
castcontext => 'e', castmethod => 'f' },
+# Allow explicit coercions between xid8 and xid
+{ castsource => 'xid8', casttarget => 'xid', castfunc => 'xid(xid8)',
+ castcontext => 'e', castmethod => 'f' },
+
# OID category: allow implicit conversion from any integral type (including
# int8, to support OID literals > 2G) to OID, as well as assignment coercion
# from OID to int4 or int8. Similarly for each OID-alias type. Also allow
diff --git a/src/include/catalog/pg_opclass.dat b/src/include/catalog/pg_opclass.dat
index fdfea85efe..9f951ceddb 100644
--- a/src/include/catalog/pg_opclass.dat
+++ b/src/include/catalog/pg_opclass.dat
@@ -165,6 +165,8 @@
opcintype => 'tid' },
{ opcmethod => 'hash', opcname => 'xid_ops', opcfamily => 'hash/xid_ops',
opcintype => 'xid' },
+{ opcmethod => 'hash', opcname => 'xid8_ops', opcfamily => 'hash/xid8_ops',
+ opcintype => 'xid8' },
{ opcmethod => 'hash', opcname => 'cid_ops', opcfamily => 'hash/cid_ops',
opcintype => 'cid' },
{ opcmethod => 'hash', opcname => 'tid_ops', opcfamily => 'hash/tid_ops',
diff --git a/src/include/catalog/pg_operator.dat b/src/include/catalog/pg_operator.dat
index 96823cd59b..d98407212a 100644
--- a/src/include/catalog/pg_operator.dat
+++ b/src/include/catalog/pg_operator.dat
@@ -192,6 +192,14 @@
oprname => '<>', oprleft => 'xid', oprright => 'int4', oprresult => 'bool',
oprnegate => '=(xid,int4)', oprcode => 'xidneqint4', oprrest => 'neqsel',
oprjoin => 'neqjoinsel' },
+{ oid => '562', descr => 'equal',
+ oprname => '=', oprcanhash => 't', oprleft => 'xid8', oprright => 'xid8',
+ oprresult => 'bool', oprcom => '=(xid8,xid8)', oprnegate => '<>(xid8,xid8)',
+ oprcode => 'xid8eq', oprrest => 'eqsel', oprjoin => 'eqjoinsel' },
+{ oid => '563', descr => 'not equal',
+ oprname => '<>', oprleft => 'xid8', oprright => 'xid8',
+ oprresult => 'bool', oprcom => '<>(xid8,xid8)', oprnegate => '=(xid8,xid8)',
+ oprcode => 'xid8neq', oprrest => 'neqsel', oprjoin => 'neqjoinsel' },
{ oid => '388', descr => 'factorial',
oprname => '!', oprkind => 'r', oprleft => 'int8', oprright => '0',
oprresult => 'numeric', oprcode => 'numeric_fac' },
diff --git a/src/include/catalog/pg_opfamily.dat b/src/include/catalog/pg_opfamily.dat
index 41e40d657a..e1e72cb6f1 100644
--- a/src/include/catalog/pg_opfamily.dat
+++ b/src/include/catalog/pg_opfamily.dat
@@ -110,6 +110,8 @@
opfmethod => 'btree', opfname => 'tid_ops' },
{ oid => '2225',
opfmethod => 'hash', opfname => 'xid_ops' },
+{ oid => '1986',
+ opfmethod => 'hash', opfname => 'xid8_ops' },
{ oid => '2226',
opfmethod => 'hash', opfname => 'cid_ops' },
{ oid => '2227',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index b88e886f7d..7c9d434673 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -112,6 +112,18 @@
{ oid => '51', descr => 'I/O',
proname => 'xidout', prorettype => 'cstring', proargtypes => 'xid',
prosrc => 'xidout' },
+{ oid => '272', descr => 'I/O',
+ proname => 'xid8in', prorettype => 'xid8', proargtypes => 'cstring',
+ prosrc => 'xid8in' },
+{ oid => '273', descr => 'I/O',
+ proname => 'xid8out', prorettype => 'cstring', proargtypes => 'xid8',
+ prosrc => 'xid8out' },
+{ oid => '560', descr => 'I/O',
+ proname => 'xid8recv', prorettype => 'xid8', proargtypes => 'internal',
+ prosrc => 'xid8recv' },
+{ oid => '561', descr => 'I/O',
+ proname => 'xid8send', prorettype => 'bytea', proargtypes => 'xid8',
+ prosrc => 'xid8send' },
{ oid => '52', descr => 'I/O',
proname => 'cidin', prorettype => 'cid', proargtypes => 'cstring',
prosrc => 'cidin' },
@@ -163,6 +175,15 @@
{ oid => '3308',
proname => 'xidneq', proleakproof => 't', prorettype => 'bool',
proargtypes => 'xid xid', prosrc => 'xidneq' },
+{ oid => '1179',
+ proname => 'xid8eq', proleakproof => 't', prorettype => 'bool',
+ proargtypes => 'xid8 xid8', prosrc => 'xid8eq' },
+{ oid => '1180',
+ proname => 'xid8neq', proleakproof => 't', prorettype => 'bool',
+ proargtypes => 'xid8 xid8', prosrc => 'xid8neq' },
+{ oid => '1177', descr => 'convert xid8 to xid',
+ proname => 'xid', prorettype => 'xid', proargtypes => 'xid8',
+ prosrc => 'xid8toxid' },
{ oid => '69',
proname => 'cideq', proleakproof => 't', prorettype => 'bool',
proargtypes => 'cid cid', prosrc => 'cideq' },
diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat
index be49e00114..aaeb4d9d61 100644
--- a/src/include/catalog/pg_type.dat
+++ b/src/include/catalog/pg_type.dat
@@ -177,6 +177,10 @@
typtype => 'p', typcategory => 'P', typinput => 'pg_ddl_command_in',
typoutput => 'pg_ddl_command_out', typreceive => 'pg_ddl_command_recv',
typsend => 'pg_ddl_command_send', typalign => 'ALIGNOF_POINTER' },
+{ oid => '270', array_type_oid => '271', descr => 'full transaction id',
+ typname => 'xid8', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typcategory => 'U', typinput => 'xid8in', typoutput => 'xid8out',
+ typreceive => 'xid8recv', typsend => 'xid8send', typalign => 'd' },
# OIDS 600 - 699
diff --git a/src/include/utils/xid8.h b/src/include/utils/xid8.h
new file mode 100644
index 0000000000..3919b2f195
--- /dev/null
+++ b/src/include/utils/xid8.h
@@ -0,0 +1,22 @@
+/*-------------------------------------------------------------------------
+ *
+ * xid8.h
+ * Header file for the "xid8" ADT.
+ *
+ * Copyright (c) 2019, PostgreSQL Global Development Group
+ *
+ * src/include/utils/xid8.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef XID8_H
+#define XID8_H
+
+#include "access/transam.h"
+
+#define DatumGetFullTransactionId(X) (FullTransactionIdFromU64(DatumGetUInt64(X)))
+#define FullTransactionIdGetDatum(X) (UInt64GetDatum(U64FromFullTransactionId(X)))
+#define PG_GETARG_FULLTRANSACTIONID(X) DatumGetFullTransactionId(PG_GETARG_DATUM(X))
+#define PG_RETURN_FULLTRANSACTIONID(X) return FullTransactionIdGetDatum(X)
+
+#endif /* XID8_H */
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 33c058ff51..157763be59 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -638,6 +638,8 @@ interval_lt(interval,interval)
interval_le(interval,interval)
interval_ge(interval,interval)
interval_gt(interval,interval)
+xid8eq(xid8,xid8)
+xid8neq(xid8,xid8)
charlt("char","char")
tidne(tid,tid)
tideq(tid,tid)
--
2.22.0
On Fri, Aug 2, 2019 at 6:37 AM Thomas Munro <thomas.munro@gmail.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?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Sat, Aug 3, 2019 at 12:06 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Aug 2, 2019 at 6:37 AM Thomas Munro <thomas.munro@gmail.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. Vignesh's patch achieves something important
on its own: it avoids the needs for zheap to do a 32->64 conversion.
--
Thomas Munro
https://enterprisedb.com
Hi,
On 2019-08-05 13:15:19 +1200, Thomas Munro wrote:
On Sat, Aug 3, 2019 at 12:06 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Aug 2, 2019 at 6:37 AM Thomas Munro <thomas.munro@gmail.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
On Mon, Aug 5, 2019 at 1:44 PM Andres Freund <andres@anarazel.de> wrote:
On 2019-08-05 13:15:19 +1200, Thomas Munro wrote:
On Sat, Aug 3, 2019 at 12:06 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Aug 2, 2019 at 6:37 AM Thomas Munro <thomas.munro@gmail.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?
Yeah. I think we're agreed for now that we don't want to change
procarray (though we still need to figure out how to compute the 64
bit horizons correctly and efficiently), and we probably don't want to
change any high volume WAL contents, so maybe I was starting down the
wrong path there (I was thinking of the subxid list for 2pc as
infrequent, but obviously it isn't for some people). I don't have
time to look into that any more right now, but today's experiment made
me feel more certain about my earlier statement, that we shouldn't
worry about the subxid list for now if we don't actually have to.
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.
Yep, I agree, hence xid8.
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?
It creates a place in the undo worker patch set that wants to do an
xid -> fxid translation, as discussed here:
/messages/by-id/CAA4eK1L9BhvnQfa_RJCTpKQf9QZ15pyUW7s32BH78iBC3KbV0g@mail.gmail.com
I'm trying to stop people from supplying a general purpose footgun
that looks like "GuessFullTransactionId(xid)". I suspect that any
time you think you want to do that, there is probably a better way
that doesn't involve having to convince everyone that we didn't mess
up the epoch part in some unlikely race, which probably involves
holding onto an fxid that you had somewhere earlier that came
ultimately from the next fxid generator, or deriving it with reference
to the next fxid or a known older-but-still-running fxid with the
right interlocking, or something like that. I and others said, well,
why don't we just put the fxid in the 2pc file. That's what Vignesh
has proposed, and AFAIK it solves that immediate problem.
That caused people to ask -- entirely reasonably -- why we don't
change ALL the xids in there to fxids, which brings us here. I think
it includes lots of tricky decisions that I don't want to make right
now, hence inclination to defer that question for now.
--
Thomas Munro
https://enterprisedb.com
Hi,
On 2019-08-05 14:44:37 +1200, Thomas Munro wrote:
Yeah. I think we're agreed for now that we don't want to change
procarray (though we still need to figure out how to compute the 64
bit horizons correctly and efficiently)
Hm. Is that actually hard? Can't we just use the current logic to
compute the horizons in 32bit, and then extend that to 64bit by
comparing to nextFullXid or something like that? That shouldn't be more
than a few instructions, outside of the contended locks?
and we probably don't want to change any high volume WAL contents, so
maybe I was starting down the wrong path there (I was thinking of the
subxid list for 2pc as infrequent, but obviously it isn't for some
people). I don't have time to look into that any more right now, but
today's experiment made me feel more certain about my earlier
statement, that we shouldn't worry about the subxid list for now if we
don't actually have to.
I don't think we should start to change existing wal contents to 64bit
xids before there's a benefit from doing so (that obviously doesn't mean
that records for a hypothetical AM employing 64bit xids shouldn't
contain them, if they are actually long-term values, rather than just
about the current transaction). I think that's just going to be
confusing, without providing much in the way of benefits.
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?
It creates a place in the undo worker patch set that wants to do an
xid -> fxid translation, as discussed here:/messages/by-id/CAA4eK1L9BhvnQfa_RJCTpKQf9QZ15pyUW7s32BH78iBC3KbV0g@mail.gmail.com
Right, but I think that can just be done the suggestion from above.
I'm trying to stop people from supplying a general purpose footgun
that looks like "GuessFullTransactionId(xid)".
I think you're right - but I think we should be able to provide
functions that ensure safety for most if not all of these. For WAL
replay routines we can reference xlogrecord, for GetOldestXmin() we can
just expand internally, by referencing a 64bit xid in ShmemVariableCache
or such.
I suspect that any time you think you want to do that, there is
probably a better way that doesn't involve having to convince everyone
that we didn't mess up the epoch part in some unlikely race, which
probably involves holding onto an fxid that you had somewhere earlier
that came ultimately from the next fxid generator, or deriving it with
reference to the next fxid or a known older-but-still-running fxid
with the right interlocking, or something like that. I and others
said, well, why don't we just put the fxid in the 2pc file. That's
what Vignesh has proposed, and AFAIK it solves that immediate problem.That caused people to ask -- entirely reasonably -- why we don't
change ALL the xids in there to fxids, which brings us here. I think
it includes lots of tricky decisions that I don't want to make right
now, hence inclination to defer that question for now.
I'm against doing this just for one part of the record. That seems
supremely confusing. And I don't buy that it meaningfully helps us in
the first place, given the multitude of other records containing 32bit
xids.
Greetings,
Andres Freund
On Mon, Aug 5, 2019 at 8:31 AM Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2019-08-05 14:44:37 +1200, Thomas Munro wrote:
Yeah. I think we're agreed for now that we don't want to change
procarray (though we still need to figure out how to compute the 64
bit horizons correctly and efficiently)Hm. Is that actually hard? Can't we just use the current logic to
compute the horizons in 32bit, and then extend that to 64bit by
comparing to nextFullXid or something like that? That shouldn't be more
than a few instructions, outside of the contended locks?and we probably don't want to change any high volume WAL contents, so
maybe I was starting down the wrong path there (I was thinking of the
subxid list for 2pc as infrequent, but obviously it isn't for some
people). I don't have time to look into that any more right now, but
today's experiment made me feel more certain about my earlier
statement, that we shouldn't worry about the subxid list for now if we
don't actually have to.I don't think we should start to change existing wal contents to 64bit
xids before there's a benefit from doing so (that obviously doesn't mean
that records for a hypothetical AM employing 64bit xids shouldn't
contain them, if they are actually long-term values, rather than just
about the current transaction). I think that's just going to be
confusing, without providing much in the way of benefits.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?
It creates a place in the undo worker patch set that wants to do an
xid -> fxid translation, as discussed here:/messages/by-id/CAA4eK1L9BhvnQfa_RJCTpKQf9QZ15pyUW7s32BH78iBC3KbV0g@mail.gmail.com
Right, but I think that can just be done the suggestion from above.
I'm trying to stop people from supplying a general purpose footgun
that looks like "GuessFullTransactionId(xid)".I think you're right - but I think we should be able to provide
functions that ensure safety for most if not all of these. For WAL
replay routines we can reference xlogrecord, for GetOldestXmin() we can
just expand internally, by referencing a 64bit xid in ShmemVariableCache
or such.I suspect that any time you think you want to do that, there is
probably a better way that doesn't involve having to convince everyone
that we didn't mess up the epoch part in some unlikely race, which
probably involves holding onto an fxid that you had somewhere earlier
that came ultimately from the next fxid generator, or deriving it with
reference to the next fxid or a known older-but-still-running fxid
with the right interlocking, or something like that. I and others
said, well, why don't we just put the fxid in the 2pc file. That's
what Vignesh has proposed, and AFAIK it solves that immediate problem.That caused people to ask -- entirely reasonably -- why we don't
change ALL the xids in there to fxids, which brings us here. I think
it includes lots of tricky decisions that I don't want to make right
now, hence inclination to defer that question for now.I'm against doing this just for one part of the record. That seems
supremely confusing. And I don't buy that it meaningfully helps us in
the first place, given the multitude of other records containing 32bit
xids.
Going by the discussion shall we conclude that we don't need to
convert the subxids into fxid's as part of this fix.
Let me know if any further changes need to be done.
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
On Wed, Aug 7, 2019 at 6:56 AM vignesh C <vignesh21@gmail.com> wrote:
Going by the discussion shall we conclude that we don't need to
convert the subxids into fxid's as part of this fix.
Let me know if any further changes need to be done.
I'm not sure, but I think the prior question is whether we want this
patch at all, and I'm not sure we've achieved consensus on that.
Thomas's point, at least as I understand it, is that if we start doing
32-bit => 64-bit XID conversions all over the place, it's not going to
be long before some incautious developer inserts one that is not
actually safe. On the other hand, Andres's point, at least as I
understand it, is that putting information that we don't really need
into the twophase state file because of some overly rigid coding rule
is not smart. Both of those arguments sound right to me, but they
lead to opposite conclusions.
I am somewhat inclined to Andres's conclusion on balance. I think
that we can probably define a set of policies about 32 => 64 bit XID
conversions both in terms of when you can do them and what comments
you have to include justifying them and how the API actually works
that makes it safe. It might help to think about defining the API in
terms of a reference FullTransactionId that must be OLDER than the XID
you're promoting to an FXID. For instance, we know that all of the
relfrozenxid and datfrozenxids are from the current era because we've
got freezing machinery to enforce that. So if you got a tuple from the
heap, it's XID has got to be new, at least modulo bugs. In other
cases, we may be able to say, hey, look, this XID can't be from before
the CLOG cutoff. I'm not sure of all the details here, but I'm
tentatively inclined to think that trying to lay down policies for
when promotion can be done safely is more promising than holding our
breath and saying we're never going to promote.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company