public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v1 2/2] Make sure published XIDs are persistent 2+ messages / 2 participants [nested] [flat]
* [PATCH v1 2/2] Make sure published XIDs are persistent @ 2021-03-08 06:43 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Kyotaro Horiguchi @ 2021-03-08 06:43 UTC (permalink / raw) pg_xact_status() premises that XIDs obtained by pg_current_xact_id(_if_assigned)() are persistent beyond a crash. But XIDs are not guaranteed to go beyond WAL buffers before commit and thus XIDs may vanish if server crashes before commit. This patch guarantees the XID shown by the functions to be flushed out to disk. --- src/backend/access/transam/xact.c | 55 +++++++++++++++++++++++++------ src/backend/access/transam/xlog.c | 2 +- src/backend/utils/adt/xid8funcs.c | 12 ++++++- src/include/access/xact.h | 3 +- 4 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 4e6a3df6b8..85b1c21ee0 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -201,7 +201,7 @@ typedef struct TransactionStateData int prevSecContext; /* previous SecurityRestrictionContext */ bool prevXactReadOnly; /* entry-time xact r/o state */ bool startedInRecovery; /* did we start in recovery? */ - bool didLogXid; /* has xid been included in WAL record? */ + XLogRecPtr minLSN; /* LSN needed to reach to record the xid */ int parallelModeLevel; /* Enter/ExitParallelMode counter */ bool chain; /* start a new block after this one */ bool assigned; /* assigned to top-level XID */ @@ -520,14 +520,46 @@ GetCurrentFullTransactionIdIfAny(void) * MarkCurrentTransactionIdLoggedIfAny * * Remember that the current xid - if it is assigned - now has been wal logged. + * + * upto is the LSN up to which we need to flush WAL to ensure the current xid + * to be persistent. See EnsureCurrentTransactionIdLogged(). */ void -MarkCurrentTransactionIdLoggedIfAny(void) +MarkCurrentTransactionIdLoggedIfAny(XLogRecPtr upto) { - if (FullTransactionIdIsValid(CurrentTransactionState->fullTransactionId)) - CurrentTransactionState->didLogXid = true; + if (FullTransactionIdIsValid(CurrentTransactionState->fullTransactionId) && + XLogRecPtrIsInvalid(CurrentTransactionState->minLSN)) + CurrentTransactionState->minLSN = upto; } +/* + * EnsureCurrentTransactionIdLogged + * + * Make sure that the current top XID is WAL-logged. + */ +void +EnsureTopTransactionIdLogged(void) +{ + /* + * We need at least one WAL record for the current top transaction to be + * flushed out. Write one if we don't have one yet. + */ + if (XLogRecPtrIsInvalid(TopTransactionStateData.minLSN)) + { + xl_xact_assignment xlrec; + + xlrec.xtop = XidFromFullTransactionId(XactTopFullTransactionId); + Assert(TransactionIdIsValid(xlrec.xtop)); + xlrec.nsubxacts = 0; + + XLogBeginInsert(); + XLogRegisterData((char *) &xlrec, MinSizeOfXactAssignment); + TopTransactionStateData.minLSN = + XLogInsert(RM_XACT_ID, XLOG_XACT_ASSIGNMENT); + } + + XLogFlush(TopTransactionStateData.minLSN); +} /* * GetStableLatestTransactionId @@ -616,14 +648,14 @@ AssignTransactionId(TransactionState s) * When wal_level=logical, guarantee that a subtransaction's xid can only * be seen in the WAL stream if its toplevel xid has been logged before. * If necessary we log an xact_assignment record with fewer than - * PGPROC_MAX_CACHED_SUBXIDS. Note that it is fine if didLogXid isn't set + * PGPROC_MAX_CACHED_SUBXIDS. Note that it is fine if minLSN isn't set * for a transaction even though it appears in a WAL record, we just might * superfluously log something. That can happen when an xid is included * somewhere inside a wal record, but not in XLogRecord->xl_xid, like in * xl_standby_locks. */ if (isSubXact && XLogLogicalInfoActive() && - !TopTransactionStateData.didLogXid) + XLogRecPtrIsInvalid(TopTransactionStateData.minLSN)) log_unknown_top = true; /* @@ -693,6 +725,7 @@ AssignTransactionId(TransactionState s) log_unknown_top) { xl_xact_assignment xlrec; + XLogRecPtr endptr; /* * xtop is always set by now because we recurse up transaction @@ -707,11 +740,13 @@ AssignTransactionId(TransactionState s) XLogRegisterData((char *) unreportedXids, nUnreportedXids * sizeof(TransactionId)); - (void) XLogInsert(RM_XACT_ID, XLOG_XACT_ASSIGNMENT); + endptr = XLogInsert(RM_XACT_ID, XLOG_XACT_ASSIGNMENT); nUnreportedXids = 0; - /* mark top, not current xact as having been logged */ - TopTransactionStateData.didLogXid = true; + + /* set minLSN of top, not of current xact if not yet */ + if (XLogRecPtrIsInvalid(TopTransactionStateData.minLSN)) + TopTransactionStateData.minLSN = endptr; } } } @@ -1996,7 +2031,7 @@ StartTransaction(void) * initialize reported xid accounting */ nUnreportedXids = 0; - s->didLogXid = false; + s->minLSN = InvalidXLogRecPtr; /* * must initialize resource-management stuff first diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 377afb8732..1a503c1447 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1159,7 +1159,7 @@ XLogInsertRecord(XLogRecData *rdata, */ WALInsertLockRelease(); - MarkCurrentTransactionIdLoggedIfAny(); + MarkCurrentTransactionIdLoggedIfAny(EndPos); END_CRIT_SECTION(); diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c index cc2b4ac797..992482f8c8 100644 --- a/src/backend/utils/adt/xid8funcs.c +++ b/src/backend/utils/adt/xid8funcs.c @@ -357,6 +357,8 @@ bad_format: Datum pg_current_xact_id(PG_FUNCTION_ARGS) { + FullTransactionId xid; + /* * Must prevent during recovery because if an xid is not assigned we try * to assign one, which would fail. Programs already rely on this function @@ -365,7 +367,12 @@ pg_current_xact_id(PG_FUNCTION_ARGS) */ PreventCommandDuringRecovery("pg_current_xact_id()"); - PG_RETURN_FULLTRANSACTIONID(GetTopFullTransactionId()); + xid = GetTopFullTransactionId(); + + /* the XID is going to be published, make sure it is psersistent */ + EnsureTopTransactionIdLogged(); + + PG_RETURN_FULLTRANSACTIONID(xid); } /* @@ -380,6 +387,9 @@ pg_current_xact_id_if_assigned(PG_FUNCTION_ARGS) if (!FullTransactionIdIsValid(topfxid)) PG_RETURN_NULL(); + /* the XID is going to be published, make sure it is psersistent */ + EnsureTopTransactionIdLogged(); + PG_RETURN_FULLTRANSACTIONID(topfxid); } diff --git a/src/include/access/xact.h b/src/include/access/xact.h index f49a57b35e..536a5e653b 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -386,7 +386,8 @@ extern FullTransactionId GetTopFullTransactionId(void); extern FullTransactionId GetTopFullTransactionIdIfAny(void); extern FullTransactionId GetCurrentFullTransactionId(void); extern FullTransactionId GetCurrentFullTransactionIdIfAny(void); -extern void MarkCurrentTransactionIdLoggedIfAny(void); +extern void MarkCurrentTransactionIdLoggedIfAny(XLogRecPtr upto); +extern void EnsureTopTransactionIdLogged(void); extern bool SubTransactionIsActive(SubTransactionId subxid); extern CommandId GetCurrentCommandId(bool used); extern void SetParallelStartTimestamps(TimestampTz xact_ts, TimestampTz stmt_ts); -- 2.27.0 ----Next_Part(Mon_Mar__8_17_32_42_2021_572)---- ^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Confused about stream replication protocol documentation @ 2023-11-01 17:57 Bruce Momjian <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Bruce Momjian @ 2023-11-01 17:57 UTC (permalink / raw) To: Li Japin <[email protected]>; +Cc: Fujii Masao <[email protected]>; PostgreSQL Developers <[email protected]> Patch applied to master, thanks. --------------------------------------------------------------------------- On Thu, Dec 24, 2020 at 02:28:53AM +0000, Li Japin wrote: > > On Dec 23, 2020, at 8:11 PM, Fujii Masao <[email protected]> > wrote: > > > On 2020/12/23 11:08, Li Japin wrote: > > On Dec 22, 2020, at 11:13 PM, Fujii Masao < > [email protected] <mailto:[email protected]>> > wrote: > > ‘B’ means a backend and ‘F’ means a frontend. Maybe as [1] does, we > should > add the note like "Each is marked to indicate that it can be sent > by > a frontend (F) and a backend (B)" into the description about each > message > format for START_REPLICATION. > > [1] > https://www.postgresql.org/docs/devel/protocol-message-formats.html > <https://www.postgresql.org/docs/devel/ > protocol-message-formats.html> > > Thanks for your clarify. Maybe we should move the "protocol message > formats” > before “stream replication protocol” or referenced it in "stream > replication protocol”. > > > I like the latter. And maybe it's better to reference to also > "53.6. Message Data Types" there because the messages for > START_REPLICATION use the message data types. > > > Add reference about “protocol message types” and “protocol message formats”. > > index 4899bacda7..5793936b42 100644 > --- a/doc/src/sgml/protocol.sgml > +++ b/doc/src/sgml/protocol.sgml > @@ -2069,8 +2069,9 @@ The commands accepted in replication mode are: > </para> > > <para> > - WAL data is sent as a series of CopyData messages. (This allows > - other information to be intermixed; in particular the server can send > + WAL data is sent as a series of CopyData messages > + (See <xref linkend="protocol-message-types"/> and <xref linkend= > "protocol-message-formats"/>). > + (This allows other information to be intermixed; in particular the > server can send > an ErrorResponse message if it encounters a failure after beginning > to stream.) The payload of each CopyData message from server to the > client contains a message of one of the following formats: > > -- > Best regards > Japin Li > > diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml > index 4899bacda7..5793936b42 100644 > --- a/doc/src/sgml/protocol.sgml > +++ b/doc/src/sgml/protocol.sgml > @@ -2069,8 +2069,9 @@ The commands accepted in replication mode are: > </para> > > <para> > - WAL data is sent as a series of CopyData messages. (This allows > - other information to be intermixed; in particular the server can send > + WAL data is sent as a series of CopyData messages > + (See <xref linkend="protocol-message-types"/> and <xref linkend="protocol-message-formats"/>). > + (This allows other information to be intermixed; in particular the server can send > an ErrorResponse message if it encounters a failure after beginning > to stream.) The payload of each CopyData message from server to the > client contains a message of one of the following formats: -- Bruce Momjian <[email protected]> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you. ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2023-11-01 17:57 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-08 06:43 [PATCH v1 2/2] Make sure published XIDs are persistent Kyotaro Horiguchi <[email protected]> 2023-11-01 17:57 Re: Confused about stream replication protocol documentation Bruce Momjian <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox