The RelationNeedsWAL() code block within _bt_delitems_delete() has had
the following comment for many years now:

/*
 * We need the target-offsets array whether or not we store the whole
 * buffer, to allow us to find the latestRemovedXid on a standby
 * server.
 */
XLogRegisterData((char *) itemnos, nitems * sizeof(OffsetNumber));

However, we don't actually need to do it that way these days. We won't
go on to determine a latestRemovedXid on a standby as of commit
558a9165e08 (that happens on the primary instead), so the comment
seems wrong.

Rather than just changing the comment, I propose that we tweak the
behavior of _bt_delitems_delete() to match its sibling function
_bt_delitems_vacuum(). That is, it should use XLogRegisterBufData(),
not XLogRegisterData(). This is cleaner, and ought to be a minor win.

Attached patch shows what I have in mind. The new comment block has
been copied from _bt_delitems_vacuum().

--
Peter Geoghegan
From a383d30b3c0404ea0095ea8f130214f4c8dd4dd9 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Wed, 1 Jan 2020 12:30:26 -0800
Subject: [PATCH] Associate LP_DEAD offsets with WAL record's buffer.

Commit 558a9165e08 taught _bt_delitems_delete() to produce its own XID
horizon on the primary instead of having standbys generate their own
latestRemovedXid.  This brought _bt_delitems_delete() closer to the
behavior of its sibling function, _bt_delitems_vacuum().  A
latestRemovedXid is generated on the primary for both VACUUM and
opportunistic deletion of items with their LP_DEAD bits set.

Bring _bt_delitems_delete() a bit further in the direction of matching
its sibling in how it WAL-logs items that are deleted from the page now.
Associate its array of items to delete with the leaf page buffer, rather
than making it generic WAL data.  This optimization is correct now
because there is no longer any need to generate a latestRemovedXid on a
standby using the array.
---
 src/backend/access/nbtree/nbtpage.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 76069661d4..99e22d6ff7 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -1096,11 +1096,12 @@ _bt_delitems_delete(Relation rel, Buffer buf,
 		XLogRegisterData((char *) &xlrec_delete, SizeOfBtreeDelete);
 
 		/*
-		 * We need the target-offsets array whether or not we store the whole
-		 * buffer, to allow us to find the latestRemovedXid on a standby
-		 * server.
+		 * The target-offsets array is not in the buffer, but pretend that it
+		 * is.  When XLogInsert stores the whole buffer, the offsets array
+		 * need not be stored too.
 		 */
-		XLogRegisterData((char *) itemnos, nitems * sizeof(OffsetNumber));
+		XLogRegisterBufData(0, (char *) itemnos,
+							nitems * sizeof(OffsetNumber));
 
 		recptr = XLogInsert(RM_BTREE_ID, XLOG_BTREE_DELETE);
 
-- 
2.17.1

Reply via email to