pgsql: Document XLOG_INCLUDE_XID a little better

From 3d4fb94fdeef2ae46511291f144b6305c9eea9f7 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Sat, 2 Oct 2021 15:21:52 +0530
Subject: [PATCH] Refactor code for logging the top transaction id in WAL

---
 src/backend/access/transam/xact.c       | 22 +++++++++++-----------
 src/backend/access/transam/xlog.c       |  7 ++++++-
 src/backend/access/transam/xloginsert.c | 23 ++++++++++-------------
 src/include/access/xact.h               |  4 ++--
 src/include/access/xlog.h               |  4 ++--
 5 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 4cc38f0..893147649 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())
@@ -6123,18 +6123,18 @@ IsSubTransactionAssignmentPending(void)
 		return false;
 
 	/* and it should not be already 'assigned' */
-	return !CurrentTransactionState->assigned;
+	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 f8c714b..78232aa 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1010,7 +1010,8 @@ XLogRecPtr
 XLogInsertRecord(XLogRecData *rdata,
 				 XLogRecPtr fpw_lsn,
 				 uint8 flags,
-				 int num_fpi)
+				 int num_fpi,
+				 bool topxid_included)
 {
 	XLogCtlInsert *Insert = &XLogCtl->Insert;
 	pg_crc32c	rdata_crc;
@@ -1163,6 +1164,10 @@ XLogInsertRecord(XLogRecData *rdata,
 
 	MarkCurrentTransactionIdLoggedIfAny();
 
+	/* Mark top transaction id is logged (if needed) */
+	if (topxid_included)
+		MarkTopTransactionIdLogged();
+
 	END_CRIT_SECTION();
 
 	/*
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index b492c65..6b5925e 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		topxid_included = 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, &topxid_included);
 
-		EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags, num_fpi);
+		EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags, num_fpi,
+								  topxid_included);
 	} 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 *topxid_included)
 {
 	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 update the flag that the top xid is included in the WAL */
+		*topxid_included = 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..c0a5602 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 topxid_included);
 extern void XLogFlush(XLogRecPtr RecPtr);
 extern bool XLogBackgroundFlush(void);
 extern bool XLogNeedsFlush(XLogRecPtr RecPtr);
-- 
1.8.3.1

From 55a6fe0256c314ed077bf8472d5f4b5dd87d2da8 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Sat, 2 Oct 2021 16:08:57 +0530
Subject: [PATCH] Refactor code for logging the top transaction id in WAL

Instead of using the curinsert_flags, which is used for different
purpose, directly use the file level static variable.  And also
change variable/function name which looks more appropriate.
---
 src/backend/access/transam/xact.c       | 22 +++++++++++-----------
 src/backend/access/transam/xloginsert.c | 22 +++++++++++++++-------
 src/include/access/xact.h               |  4 ++--
 src/include/access/xlog.h               |  1 -
 4 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 4cc38f0..893147649 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())
@@ -6123,18 +6123,18 @@ IsSubTransactionAssignmentPending(void)
 		return false;
 
 	/* and it should not be already 'assigned' */
-	return !CurrentTransactionState->assigned;
+	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/xloginsert.c b/src/backend/access/transam/xloginsert.c
index b492c65..2e2344b 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -100,6 +100,15 @@ static uint8 curinsert_flags = 0;
 static XLogRecData hdr_rdt;
 static char *hdr_scratch = NULL;
 
+/*
+ * This variable is used for indicating that the top transaction xid is
+ * included in the current WAL record.  This temporary flag is set by
+ * XLogRecordAssemble and used by XLogResetInsertion to mark the flag
+ * in CurrentTransactionState.  For more detail about this, refer to comments
+ * atop IsLogTopTransactionIdPending().
+ */
+static bool	topxid_included = false;
+
 #define SizeOfXlogOrigin	(sizeof(RepOriginId) + sizeof(char))
 #define SizeOfXLogTransactionId	(sizeof(TransactionId) + sizeof(char))
 
@@ -209,9 +218,9 @@ XLogResetInsertion(void)
 {
 	int			i;
 
-	/* reset the subxact assignment flag (if needed) */
-	if (curinsert_flags & XLOG_INCLUDE_XID)
-		MarkSubTransactionAssigned();
+	/* Mark top transaction id is logged (if needed) */
+	if (topxid_included)
+		MarkTopTransactionIdLogged();
 
 	for (i = 0; i < max_registered_block_id; i++)
 		registered_buffers[i].in_use = false;
@@ -222,6 +231,7 @@ XLogResetInsertion(void)
 	mainrdata_last = (XLogRecData *) &mainrdata_head;
 	curinsert_flags = 0;
 	begininsert_called = false;
+	topxid_included = false;
 }
 
 /*
@@ -409,8 +419,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)
@@ -788,12 +796,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);
+		topxid_included = 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..01a6a8b 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 */
-- 
1.8.3.1

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

From fcbb965e6ba7a444c9616bb0162913fda6e1cde3 Mon Sep 17 00:00:00 2001
From: Amit Kapila <akapila@postgresql.org>
Date: Mon, 18 Oct 2021 10:23:12 +0530
Subject: [PATCH v3] 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.

In passing, change the related function and variable names to make them
consistent with what the code is actually doing.

This also reverts the changes made by commit ade24dab97 where we have
improved the documentation for XLOG_INCLUDE_XID flag.

Author: Dilip Kumar
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/E1mSoYz-0007Fh-D9@gemulon.postgresql.org
---
 src/backend/access/transam/xact.c       | 25 +++++++++++++------------
 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, 41 insertions(+), 33 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index d96881c0de..8f9884a69b 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -205,7 +205,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;
 
@@ -238,7 +238,7 @@ typedef struct SerializedTransactionState
 static TransactionStateData TopTransactionStateData = {
 	.state = TRANS_DEFAULT,
 	.blockState = TBLOCK_DEFAULT,
-	.assigned = false,
+	.topXidLogged = false,
 };
 
 /*
@@ -5168,7 +5168,7 @@ PushTransaction(void)
 	GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
 	s->prevXactReadOnly = XactReadOnly;
 	s->parallelModeLevel = 0;
-	s->assigned = false;
+	s->topXidLogged = false;
 
 	CurrentTransactionState = s;
 
@@ -6102,7 +6102,7 @@ xact_redo(XLogReaderState *record)
 }
 
 /*
- * IsSubTransactionAssignmentPending
+ * IsTopTransactionIdLogged
  *
  * 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
@@ -6113,7 +6113,7 @@ xact_redo(XLogReaderState *record)
  * record.
  */
 bool
-IsSubTransactionAssignmentPending(void)
+IsTopTransactionIdLogged(void)
 {
 	/* wal_level has to be logical */
 	if (!XLogLogicalInfoActive())
@@ -6131,19 +6131,20 @@ 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.
+ * Remember that the top transaction id for the current subtransaction is WAL
+ * logged now.
  */
 void
-MarkSubTransactionAssigned(void)
+MarkTopTransactionIdLogged(void)
 {
-	Assert(IsSubTransactionAssignmentPending());
+	Assert(IsTopTransactionIdLogged());
 
-	CurrentTransactionState->assigned = true;
+	CurrentTransactionState->topXidLogged = true;
 }
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 62862255fc..c899bd88d8 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -998,6 +998,9 @@ static void WALInsertLockUpdateInsertingAt(XLogRecPtr insertingAt);
  * 'flags' gives more in-depth control on the record being inserted. See
  * XLogSetRecordFlags() for details.
  *
+ * 'topxid_included' 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
@@ -1013,7 +1016,8 @@ XLogRecPtr
 XLogInsertRecord(XLogRecData *rdata,
 				 XLogRecPtr fpw_lsn,
 				 uint8 flags,
-				 int num_fpi)
+				 int num_fpi,
+				 bool topxid_included)
 {
 	XLogCtlInsert *Insert = &XLogCtl->Insert;
 	pg_crc32c	rdata_crc;
@@ -1166,6 +1170,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 (topxid_included)
+		MarkTopTransactionIdLogged();
+
 	END_CRIT_SECTION();
 
 	/*
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index b492c656d7..8b76ca0286 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		topxid_included = 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, &topxid_included);
 
-		EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags, num_fpi);
+		EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags, num_fpi,
+								  topxid_included);
 	} 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 *topxid_included)
 {
 	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 (IsTopTransactionIdLogged())
 	{
 		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 */
+		*topxid_included = 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 134f6862da..e39915a86e 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 IsTopTransactionIdLogged(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 5e2c94a05f..c0a560204b 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 topxid_included);
 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 8d1305eae8..e06ee92a5e 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
-- 
2.28.0.windows.1

From eca2c354f24d38df7415228844e6ed928a127f25 Mon Sep 17 00:00:00 2001
From: Amit Kapila <akapila@postgresql.org>
Date: Wed, 20 Oct 2021 14:55:54 +0530
Subject: [PATCH v4] 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.

In passing, change the related function and variable names to make them
consistent with what the code is actually doing.

This also reverts the changes made by commit ade24dab97 where we have
improved the documentation for XLOG_INCLUDE_XID flag.

Author: Dilip Kumar
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/E1mSoYz-0007Fh-D9@gemulon.postgresql.org
---
 src/backend/access/transam/xact.c       | 25 +++++++++++++------------
 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, 41 insertions(+), 33 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index d96881c0de..fe77a1bab2 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -205,7 +205,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;
 
@@ -238,7 +238,7 @@ typedef struct SerializedTransactionState
 static TransactionStateData TopTransactionStateData = {
 	.state = TRANS_DEFAULT,
 	.blockState = TBLOCK_DEFAULT,
-	.assigned = false,
+	.topXidLogged = false,
 };
 
 /*
@@ -5168,7 +5168,7 @@ PushTransaction(void)
 	GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
 	s->prevXactReadOnly = XactReadOnly;
 	s->parallelModeLevel = 0;
-	s->assigned = false;
+	s->topXidLogged = false;
 
 	CurrentTransactionState = s;
 
@@ -6102,7 +6102,7 @@ xact_redo(XLogReaderState *record)
 }
 
 /*
- * IsSubTransactionAssignmentPending
+ * IsTopTransactionIdLogPending
  *
  * 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
@@ -6113,7 +6113,7 @@ xact_redo(XLogReaderState *record)
  * record.
  */
 bool
-IsSubTransactionAssignmentPending(void)
+IsTopTransactionIdLogPending(void)
 {
 	/* wal_level has to be logical */
 	if (!XLogLogicalInfoActive())
@@ -6131,19 +6131,20 @@ 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.
+ * Remember that the top transaction id for the current subtransaction is WAL
+ * logged now.
  */
 void
-MarkSubTransactionAssigned(void)
+MarkTopTransactionIdLogged(void)
 {
-	Assert(IsSubTransactionAssignmentPending());
+	Assert(IsTopTransactionIdLogPending());
 
-	CurrentTransactionState->assigned = true;
+	CurrentTransactionState->topXidLogged = true;
 }
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 62862255fc..c899bd88d8 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -998,6 +998,9 @@ static void WALInsertLockUpdateInsertingAt(XLogRecPtr insertingAt);
  * 'flags' gives more in-depth control on the record being inserted. See
  * XLogSetRecordFlags() for details.
  *
+ * 'topxid_included' 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
@@ -1013,7 +1016,8 @@ XLogRecPtr
 XLogInsertRecord(XLogRecData *rdata,
 				 XLogRecPtr fpw_lsn,
 				 uint8 flags,
-				 int num_fpi)
+				 int num_fpi,
+				 bool topxid_included)
 {
 	XLogCtlInsert *Insert = &XLogCtl->Insert;
 	pg_crc32c	rdata_crc;
@@ -1166,6 +1170,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 (topxid_included)
+		MarkTopTransactionIdLogged();
+
 	END_CRIT_SECTION();
 
 	/*
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index b492c656d7..f1babc0f02 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		topxid_included = 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, &topxid_included);
 
-		EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags, num_fpi);
+		EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags, num_fpi,
+								  topxid_included);
 	} 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 *topxid_included)
 {
 	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 (IsTopTransactionIdLogPending())
 	{
 		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 */
+		*topxid_included = 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 134f6862da..0f24bcd062 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 IsTopTransactionIdLogPending(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 5e2c94a05f..c0a560204b 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 topxid_included);
 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 8d1305eae8..e06ee92a5e 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
-- 
2.28.0.windows.1

From 797f274c395f56b72f1d7ec5ee20099f73d6fa9f Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Thu, 21 Oct 2021 09:52:09 +0530
Subject: [PATCH v5 1/2] 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.

In passing, change the related function and variable names to make them
consistent with what the code is actually doing.

This also reverts the changes made by commit ade24dab97 where we have
improved the documentation for XLOG_INCLUDE_XID flag.

Author: Dilip Kumar
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/E1mSoYz-0007Fh-D9@gemulon.postgresql.org
---
 src/backend/access/transam/xact.c       | 103 ++++++++++++++++----------------
 src/backend/access/transam/xlog.c       |  13 +++-
 src/backend/access/transam/xloginsert.c |  26 ++++----
 src/include/access/xact.h               |   4 +-
 src/include/access/xlog.h               |   4 +-
 5 files changed, 82 insertions(+), 68 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index d96881c..90e689e 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -205,7 +205,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;	/* for a subxact: is top-level XID logged? */
 	struct TransactionStateData *parent;	/* back link to parent */
 } TransactionStateData;
 
@@ -238,7 +238,7 @@ typedef struct SerializedTransactionState
 static TransactionStateData TopTransactionStateData = {
 	.state = TRANS_DEFAULT,
 	.blockState = TBLOCK_DEFAULT,
-	.assigned = false,
+	.topXidLogged = false,
 };
 
 /*
@@ -529,6 +529,56 @@ MarkCurrentTransactionIdLoggedIfAny(void)
 		CurrentTransactionState->didLogXid = true;
 }
 
+/*
+ * IsSubxactTopXidLogPending
+ *
+ * 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
+ * LogicalDecodingProcessRecord.
+ *
+ * This returns true if wal_level >= logical and we are inside a valid
+ * subtransaction, for which the assignment was not yet written to any WAL
+ * record.
+ */
+bool
+IsSubxactTopXidLogPending(void)
+{
+	/* check whether it is already logged */
+	if (CurrentTransactionState->topXidLogged)
+		return false;
+
+	/* wal_level has to be logical */
+	if (!XLogLogicalInfoActive())
+		return false;
+
+	/* we need to be in a transaction state */
+	if (!IsTransactionState())
+		return false;
+
+	/* it has to be a subtransaction */
+	if (!IsSubTransaction())
+		return false;
+
+	/* the subtransaction has to have a XID assigned */
+	if (!TransactionIdIsValid(GetCurrentTransactionIdIfAny()))
+		return false;
+
+	return true;
+}
+
+/*
+ * MarkSubxactTopXidLogged
+ *
+ * Remember that the top transaction id for the current subtransaction is WAL
+ * logged now.
+ */
+void
+MarkSubxactTopXidLogged(void)
+{
+	Assert(IsSubxactTopXidLogPending());
+
+	CurrentTransactionState->topXidLogged = true;
+}
 
 /*
  *	GetStableLatestTransactionId
@@ -5168,7 +5218,7 @@ PushTransaction(void)
 	GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
 	s->prevXactReadOnly = XactReadOnly;
 	s->parallelModeLevel = 0;
-	s->assigned = false;
+	s->topXidLogged = false;
 
 	CurrentTransactionState = s;
 
@@ -6100,50 +6150,3 @@ xact_redo(XLogReaderState *record)
 	else
 		elog(PANIC, "xact_redo: unknown op code %u", info);
 }
-
-/*
- * IsSubTransactionAssignmentPending
- *
- * 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
- * LogicalDecodingProcessRecord.
- *
- * This returns true if wal_level >= logical and we are inside a valid
- * subtransaction, for which the assignment was not yet written to any WAL
- * record.
- */
-bool
-IsSubTransactionAssignmentPending(void)
-{
-	/* wal_level has to be logical */
-	if (!XLogLogicalInfoActive())
-		return false;
-
-	/* we need to be in a transaction state */
-	if (!IsTransactionState())
-		return false;
-
-	/* it has to be a subtransaction */
-	if (!IsSubTransaction())
-		return false;
-
-	/* the subtransaction has to have a XID assigned */
-	if (!TransactionIdIsValid(GetCurrentTransactionIdIfAny()))
-		return false;
-
-	/* and it should not be already 'assigned' */
-	return !CurrentTransactionState->assigned;
-}
-
-/*
- * MarkSubTransactionAssigned
- *
- * Mark the subtransaction assignment as completed.
- */
-void
-MarkSubTransactionAssigned(void)
-{
-	Assert(IsSubTransactionAssignmentPending());
-
-	CurrentTransactionState->assigned = true;
-}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6286225..ee30315 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -998,6 +998,9 @@ static void WALInsertLockUpdateInsertingAt(XLogRecPtr insertingAt);
  * 'flags' gives more in-depth control on the record being inserted. See
  * XLogSetRecordFlags() for details.
  *
+ * 'topxid_included' 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
@@ -1013,7 +1016,8 @@ XLogRecPtr
 XLogInsertRecord(XLogRecData *rdata,
 				 XLogRecPtr fpw_lsn,
 				 uint8 flags,
-				 int num_fpi)
+				 int num_fpi,
+				 bool topxid_included)
 {
 	XLogCtlInsert *Insert = &XLogCtl->Insert;
 	pg_crc32c	rdata_crc;
@@ -1169,6 +1173,13 @@ XLogInsertRecord(XLogRecData *rdata,
 	END_CRIT_SECTION();
 
 	/*
+	 * 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 (topxid_included)
+		MarkSubxactTopXidLogged();
+
+	/*
 	 * Update shared LogwrtRqst.Write, if we crossed page boundary.
 	 */
 	if (StartPos / XLOG_BLCKSZ != EndPos / XLOG_BLCKSZ)
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index b492c65..689384a 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		topxid_included = 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, &topxid_included);
 
-		EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags, num_fpi);
+		EndPos = XLogInsertRecord(rdt, fpw_lsn, curinsert_flags, num_fpi,
+								  topxid_included);
 	} while (EndPos == InvalidXLogRecPtr);
 
 	XLogResetInsertion();
@@ -498,11 +495,14 @@ XLogInsert(RmgrId rmid, uint8 info)
  * of all of them, *fpw_lsn is set to the lowest LSN among such pages. This
  * signals that the assembled record is only good for insertion on the
  * assumption that the RedoRecPtr and doPageWrites values were up-to-date.
+ *
+ * *topxid_included is set if the topmost transaction ID is logged with the
+ * current subtransaction.
  */
 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)
 {
 	XLogRecData *rdt;
 	uint32		total_len = 0;
@@ -788,12 +788,12 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
 	}
 
 	/* followed by toplevel XID, if not already included in previous record */
-	if (IsSubTransactionAssignmentPending())
+	if (IsSubxactTopXidLogPending())
 	{
 		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 */
+		*topxid_included = 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..5546d33 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 IsSubxactTopXidLogPending(void);
+extern void MarkSubxactTopXidLogged(void);
 
 extern int	xactGetCommittedChildren(TransactionId **ptr);
 
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 5e2c94a..c0a5602 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 topxid_included);
 extern void XLogFlush(XLogRecPtr RecPtr);
 extern bool XLogBackgroundFlush(void);
 extern bool XLogNeedsFlush(XLogRecPtr RecPtr);
-- 
1.8.3.1

From df06247d10325c0b659f7fdcb8a6c6c32828ef82 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumar@localhost.localdomain>
Date: Thu, 21 Oct 2021 11:15:30 +0530
Subject: [PATCH] Code refactoring, move unwanted function out of the critical
 section

Move MarkCurrentTransactionIdLoggedIfAny out of the critical
section.
---
 src/backend/access/transam/xlog.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ee30315..28935e4 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1168,10 +1168,10 @@ XLogInsertRecord(XLogRecData *rdata,
 	 */
 	WALInsertLockRelease();
 
-	MarkCurrentTransactionIdLoggedIfAny();
-
 	END_CRIT_SECTION();
 
+	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.
-- 
1.8.3.1