public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v8 3/9] Make sure published XIDs are persistent
4+ messages / 3 participants
[nested] [flat]

* [PATCH v8 3/9] Make sure published XIDs are persistent
@ 2021-03-08 06:43  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 4+ 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.

Copied from: https://www.postgresql.org/message-id/20210308.173242.463790587797836129.horikyota.ntt%40gmail.com
---
 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 441445927e..da8a460722 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 64094e7175..18d8743715 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1162,7 +1162,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 134f6862da..593a4140df 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.17.0


--P+33d92oIH25kiaB
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v8-0004-wal_compression_method-default-to-zlib.patch"



^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: new commitfest transition guidance
@ 2025-03-05 17:00  Daniel Gustafsson <[email protected]>
  0 siblings, 1 reply; 4+ messages in thread

From: Daniel Gustafsson @ 2025-03-05 17:00 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers

> On 4 Mar 2025, at 15:10, jian he <[email protected]> wrote:
> On Tue, Feb 4, 2025 at 2:39 PM Daniel Gustafsson <[email protected]> wrote:

>> They will be on the wiki shortly, I've taken notes of all discussions and am
>> busy tidying them up to be able to publish them once the participants have
>> proofread to ensure noone is misattributed.
> 
> hi.
> i can only found 2024 version.
> https://wiki.postgresql.org/wiki/FOSDEM/PGDay_2024_Developer_Meeting#FOSDEM_2024_Developer_Meeting_s...
> 
> google:
> site: https://wiki.postgresql.org/wiki  PGDay 2025 fosdem
> didn't have any interesting results.

I would avoid using Google for finding content on the wiki, the search function
on the wiki itself is generally more reliable.  Searching for FOSDEM 2025
returns the following as the top result:

    https://wiki.postgresql.org/wiki/FOSDEM/PGDay_2025_Developer_Meeting

--
Daniel Gustafsson







^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: new commitfest transition guidance
@ 2025-03-05 18:32  Álvaro Herrera <[email protected]>
  parent: Daniel Gustafsson <[email protected]>
  0 siblings, 1 reply; 4+ messages in thread

From: Álvaro Herrera @ 2025-03-05 18:32 UTC (permalink / raw)
  To: Daniel Gustafsson <[email protected]>; +Cc: jian he <[email protected]>; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers

On 2025-Mar-05, Daniel Gustafsson wrote:

> I would avoid using Google for finding content on the wiki, the search function
> on the wiki itself is generally more reliable.  Searching for FOSDEM 2025
> returns the following as the top result:
> 
>     https://wiki.postgresql.org/wiki/FOSDEM/PGDay_2025_Developer_Meeting

The category system in the wiki is very useful for this kind of thing.

https://wiki.postgresql.org/wiki/Category:Developer_Meeting

I encourage everybody to tag the pages they create with one or more
category tags.  In this case, it means adding this at the bottom of the
page:

[[Category:Developer_Meeting]]

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/






^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: new commitfest transition guidance
@ 2025-03-06 07:53  Daniel Gustafsson <[email protected]>
  parent: Álvaro Herrera <[email protected]>
  0 siblings, 0 replies; 4+ messages in thread

From: Daniel Gustafsson @ 2025-03-06 07:53 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: jian he <[email protected]>; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers

> On 5 Mar 2025, at 19:32, Álvaro Herrera <[email protected]> wrote:
> 
> On 2025-Mar-05, Daniel Gustafsson wrote:
> 
>> I would avoid using Google for finding content on the wiki, the search function
>> on the wiki itself is generally more reliable.  Searching for FOSDEM 2025
>> returns the following as the top result:
>> 
>>    https://wiki.postgresql.org/wiki/FOSDEM/PGDay_2025_Developer_Meeting
> 
> The category system in the wiki is very useful for this kind of thing.
> 
> https://wiki.postgresql.org/wiki/Category:Developer_Meeting
> 
> I encourage everybody to tag the pages they create with one or more
> category tags.  In this case, it means adding this at the bottom of the
> page:
> 
> [[Category:Developer_Meeting]]

Very good point, and a belated thank you for tagging this page with the right
category which I had failed to do.

--
Daniel Gustafsson







^ permalink  raw  reply  [nested|flat] 4+ messages in thread


end of thread, other threads:[~2025-03-06 07:53 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 06:43 [PATCH v8 3/9] Make sure published XIDs are persistent Kyotaro Horiguchi <[email protected]>
2025-03-05 17:00 Re: new commitfest transition guidance Daniel Gustafsson <[email protected]>
2025-03-05 18:32 ` Re: new commitfest transition guidance Álvaro Herrera <[email protected]>
2025-03-06 07:53   ` Re: new commitfest transition guidance Daniel Gustafsson <[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