v2-0001-Replace-XLOG_INCLUDE_XID-flag-with-a-more-localiz.patch

From 05ae038d92be6743ede04fcfc509303850d3a50a Mon Sep 17 00:00:00 2001
From: Amit Kapila <akapila@postgresql.org>
Date: Thu, 7 Oct 2021 10:37:06 +0530
Subject: [PATCH v2] Replace XLOG_INCLUDE_XID flag with a more localized flag.

Commit 0bead9af484c introduced XLOG_INCLUDE_XID flag to indicate that the
WAL record contains subXID-to-topXID association. It uses that flag later
to mark in CurrentTransactionState that top-xid is logged so that we
should not try to log it again with the next WAL record in the current
subtransaction. However, we can use a localized variable to pass that
information.

This also reverts the changes made by commit ade24dab97 where we have
improved the documentation for XLOG_INCLUDE_XID flag.
---
 src/backend/access/transam/xact.c       | 24 ++++++++++++------------
 src/backend/access/transam/xlog.c       | 13 ++++++++++++-
 src/backend/access/transam/xloginsert.c | 23 ++++++++++-------------
 src/include/access/xact.h               |  4 ++--
 src/include/access/xlog.h               |  4 ++--
 src/include/access/xlogrecord.h         |  5 ++---
 6 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 4cc38f0..bf31949 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -204,7 +204,7 @@ typedef struct TransactionStateData
 	bool		didLogXid;		/* has xid been included in WAL record? */
 	int			parallelModeLevel;	/* Enter/ExitParallelMode counter */
 	bool		chain;			/* start a new block after this one */
-	bool		assigned;		/* assigned to top-level XID */
+	bool		topXidLogged;	/* top-level XID is logged? */
 	struct TransactionStateData *parent;	/* back link to parent */
 } TransactionStateData;
 
@@ -237,7 +237,7 @@ typedef struct SerializedTransactionState
 static TransactionStateData TopTransactionStateData = {
 	.state = TRANS_DEFAULT,
 	.blockState = TBLOCK_DEFAULT,
-	.assigned = false,
+	.topXidLogged = false,
 };
 
 /*
@@ -5159,7 +5159,7 @@ PushTransaction(void)
 	GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
 	s->prevXactReadOnly = XactReadOnly;
 	s->parallelModeLevel = 0;
-	s->assigned = false;
+	s->topXidLogged = false;
 
 	CurrentTransactionState = s;
 
@@ -6093,7 +6093,7 @@ xact_redo(XLogReaderState *record)
 }
 
 /*
- * IsSubTransactionAssignmentPending
+ * IsLogTopTransactionIdPending
  *
  * This is used to decide whether we need to WAL log the top-level XID for
  * operation in a subtransaction.  We require that for logical decoding, see
@@ -6104,7 +6104,7 @@ xact_redo(XLogReaderState *record)
  * record.
  */
 bool
-IsSubTransactionAssignmentPending(void)
+IsLogTopTransactionIdPending(void)
 {
 	/* wal_level has to be logical */
 	if (!XLogLogicalInfoActive())
@@ -6122,19 +6122,19 @@ IsSubTransactionAssignmentPending(void)
 	if (!TransactionIdIsValid(GetCurrentTransactionIdIfAny()))
 		return false;
 
-	/* and it should not be already 'assigned' */
-	return !CurrentTransactionState->assigned;
+	/* and it should not be already 'logged' */
+	return !CurrentTransactionState->topXidLogged;
 }
 
 /*
- * MarkSubTransactionAssigned
+ * MarkTopTransactionIdLogged
  *
- * Mark the subtransaction assignment as completed.
+ * Mark top transaction id is WAL logged for the current subtransaction.
  */
 void
-MarkSubTransactionAssigned(void)
+MarkTopTransactionIdLogged(void)
 {
-	Assert(IsSubTransactionAssignmentPending());
+	Assert(IsLogTopTransactionIdPending());
 
-	CurrentTransactionState->assigned = true;
+	CurrentTransactionState->topXidLogged = true;
 }
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 26dcc00..496387f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -995,6 +995,9 @@ static void WALInsertLockUpdateInsertingAt(XLogRecPtr insertingAt);
  * 'flags' gives more in-depth control on the record being inserted. See
  * XLogSetRecordFlags() for details.
  *
+ * 'include_topxid' tells whether the top-transaction id is logged along with
+ * current subtransaction. See XLogRecordAssemble().
+ *
  * The first XLogRecData in the chain must be for the record header, and its
  * data must be MAXALIGNed.  XLogInsertRecord fills in the xl_prev and
  * xl_crc fields in the header, the rest of the header must already be filled
@@ -1010,7 +1013,8 @@ XLogRecPtr
 XLogInsertRecord(XLogRecData *rdata,
 				 XLogRecPtr fpw_lsn,
 				 uint8 flags,
-				 int num_fpi)
+				 int num_fpi,
+				 bool include_topxid)
 {
 	XLogCtlInsert *Insert = &XLogCtl->Insert;
 	pg_crc32c	rdata_crc;
@@ -1163,6 +1167,13 @@ XLogInsertRecord(XLogRecData *rdata,
 
 	MarkCurrentTransactionIdLoggedIfAny();
 
+	/*
+	 * Mark top transaction id is logged (if needed) so that we should not try
+	 * to log it again with the next WAL record in the current subtransaction.
+	 */
+	if (include_topxid)
+		MarkTopTransactionIdLogged();
+
 	END_CRIT_SECTION();
 
 	/*
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index b492c65..411b57d 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -123,7 +123,8 @@ static MemoryContext xloginsert_cxt;
 
 static XLogRecData *XLogRecordAssemble(RmgrId rmid, uint8 info,
 									   XLogRecPtr RedoRecPtr, bool doPageWrites,
-									   XLogRecPtr *fpw_lsn, int *num_fpi);
+									   XLogRecPtr *fpw_lsn, int *num_fpi,
+									   bool *topxid_included);
 static bool XLogCompressBackupBlock(char *page, uint16 hole_offset,
 									uint16 hole_length, char *dest, uint16 *dlen);
 
@@ -209,10 +210,6 @@ XLogResetInsertion(void)
 {
 	int			i;
 
-	/* reset the subxact assignment flag (if needed) */
-	if (curinsert_flags & XLOG_INCLUDE_XID)
-		MarkSubTransactionAssigned();
-
 	for (i = 0; i < max_registered_block_id; i++)
 		registered_buffers[i].in_use = false;
 
@@ -409,8 +406,6 @@ XLogRegisterBufData(uint8 block_id, char *data, int len)
  * - XLOG_MARK_UNIMPORTANT, to signal that the record is not important for
  *	 durability, which allows to avoid triggering WAL archiving and other
  *	 background activity.
- * - XLOG_INCLUDE_XID, a message-passing hack between XLogRecordAssemble
- *	 and XLogResetInsertion.
  */
 void
 XLogSetRecordFlags(uint8 flags)
@@ -465,6 +460,7 @@ XLogInsert(RmgrId rmid, uint8 info)
 	{
 		XLogRecPtr	RedoRecPtr;
 		bool		doPageWrites;
+		bool		include_topxid = false;
 		XLogRecPtr	fpw_lsn;
 		XLogRecData *rdt;
 		int			num_fpi = 0;
@@ -477,9 +473,10 @@ XLogInsert(RmgrId rmid, uint8 info)
 		GetFullPageWriteInfo(&RedoRecPtr, &doPageWrites);
 
 		rdt = XLogRecordAssemble(rmid, info, RedoRecPtr, doPageWrites,
-								 &fpw_lsn, &num_fpi);
+								 &fpw_lsn, &num_fpi, &include_topxid);
 
-		EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags, num_fpi);
+		EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags, num_fpi,
+								  include_topxid);
 	} while (EndPos == InvalidXLogRecPtr);
 
 	XLogResetInsertion();
@@ -502,7 +499,7 @@ XLogInsert(RmgrId rmid, uint8 info)
 static XLogRecData *
 XLogRecordAssemble(RmgrId rmid, uint8 info,
 				   XLogRecPtr RedoRecPtr, bool doPageWrites,
-				   XLogRecPtr *fpw_lsn, int *num_fpi)
+				   XLogRecPtr *fpw_lsn, int *num_fpi, bool *include_topxid)
 {
 	XLogRecData *rdt;
 	uint32		total_len = 0;
@@ -788,12 +785,12 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
 	}
 
 	/* followed by toplevel XID, if not already included in previous record */
-	if (IsSubTransactionAssignmentPending())
+	if (IsLogTopTransactionIdPending())
 	{
 		TransactionId xid = GetTopTransactionIdIfAny();
 
-		/* update the flag (later used by XLogResetInsertion) */
-		XLogSetRecordFlags(XLOG_INCLUDE_XID);
+		/* Set the flag that the top xid is included in the WAL */
+		*include_topxid = true;
 
 		*(scratch++) = (char) XLR_BLOCK_ID_TOPLEVEL_XID;
 		memcpy(scratch, &xid, sizeof(TransactionId));
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index 134f686..e2dafe6 100644
--- a/src/include/access/xact.h
+++ b/src/include/access/xact.h
@@ -433,8 +433,8 @@ extern void UnregisterXactCallback(XactCallback callback, void *arg);
 extern void RegisterSubXactCallback(SubXactCallback callback, void *arg);
 extern void UnregisterSubXactCallback(SubXactCallback callback, void *arg);
 
-extern bool IsSubTransactionAssignmentPending(void);
-extern void MarkSubTransactionAssigned(void);
+extern bool IsLogTopTransactionIdPending(void);
+extern void MarkTopTransactionIdLogged(void);
 
 extern int	xactGetCommittedChildren(TransactionId **ptr);
 
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 5e2c94a..c616512 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -212,7 +212,6 @@ extern bool XLOG_DEBUG;
  */
 #define XLOG_INCLUDE_ORIGIN		0x01	/* include the replication origin */
 #define XLOG_MARK_UNIMPORTANT	0x02	/* record not important for durability */
-#define XLOG_INCLUDE_XID		0x04	/* WAL-internal message-passing hack */
 
 
 /* Checkpoint statistics */
@@ -258,7 +257,8 @@ struct XLogRecData;
 extern XLogRecPtr XLogInsertRecord(struct XLogRecData *rdata,
 								   XLogRecPtr fpw_lsn,
 								   uint8 flags,
-								   int num_fpi);
+								   int num_fpi,
+								   bool include_topxid);
 extern void XLogFlush(XLogRecPtr RecPtr);
 extern bool XLogBackgroundFlush(void);
 extern bool XLogNeedsFlush(XLogRecPtr RecPtr);
diff --git a/src/include/access/xlogrecord.h b/src/include/access/xlogrecord.h
index 8d1305e..e06ee92 100644
--- a/src/include/access/xlogrecord.h
+++ b/src/include/access/xlogrecord.h
@@ -215,9 +215,8 @@ typedef struct XLogRecordDataHeaderLong
  * Block IDs used to distinguish different kinds of record fragments. Block
  * references are numbered from 0 to XLR_MAX_BLOCK_ID. A rmgr is free to use
  * any ID number in that range (although you should stick to small numbers,
- * because the WAL machinery is optimized for that case). A few ID
- * numbers are reserved to denote the "main" data portion of the record,
- * as well as replication-supporting transaction metadata.
+ * because the WAL machinery is optimized for that case). A couple of ID
+ * numbers are reserved to denote the "main" data portion of the record.
  *
  * The maximum is currently set at 32, quite arbitrarily. Most records only
  * need a handful of block references, but there are a few exceptions that
-- 
1.8.3.1