Do not emit WAL record for FSM truncaton when unneeded.

Started by Kirill Reshkeabout 2 months ago1 messages
#1Kirill Reshke
reshkekirill@gmail.com
1 attachment(s)

Hi hackers!

While working on other matters, I noticed that the
`fsm_truncate_avail` function returns a bool parameter which is never
used. This makes me think we can make an enhancement here. There are
basically two options:
1) make return type `void`
2) make use of return value.

I tried the second option. In FreeSpaceMapPrepareTruncateRel (the only
user of fsm_truncate_avail), we call fsm_truncate_avail and then
WAL-log FSM page changes.
Instead, can use `fsm_truncate_avail ` return value to check if
WAL-logging can be skipped.
PFA with these changes.

Thoughts?

P.S. I did not make a POC of real-life scenarios when the FSM page is
unaltered by fsm_truncate_avail (yet). I wonder if there are such
scenarios. If there is none, option 1 should proceed IMHO.

--
Best regards,
Kirill Reshke

Attachments:

v1-0001-Do-not-emit-WAL-record-for-FSM-truncaton-when-unn.patchapplication/octet-stream; name=v1-0001-Do-not-emit-WAL-record-for-FSM-truncaton-when-unn.patchDownload
From 1555e4c85c836f94e241ea156b20fcc284a2a65f Mon Sep 17 00:00:00 2001
From: reshke <reshke@double.cloud>
Date: Sat, 22 Nov 2025 05:57:22 +0000
Subject: [PATCH v1] Do not emit WAL record for FSM truncaton when unneeded.

`fsm_truncate_avail` changes FSM page when relation is truncated,
resetting all FSM-tree nodes to zero. However, there is a case when
all nodes were already zero, so no actaul modiifcation made.
This commit emits WAL-logging FSM FPI for that case.
---
 src/backend/storage/freespace/freespace.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index 62e2835e85d..c678d55b510 100644
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -299,6 +299,8 @@ FreeSpaceMapPrepareTruncateRel(Relation rel, BlockNumber nblocks)
 	 */
 	if (first_removed_slot > 0)
 	{
+		bool changed;
+
 		buf = fsm_readbuf(rel, first_removed_address, false);
 		if (!BufferIsValid(buf))
 			return InvalidBlockNumber;	/* nothing to do; the FSM was already
@@ -308,7 +310,7 @@ FreeSpaceMapPrepareTruncateRel(Relation rel, BlockNumber nblocks)
 		/* NO EREPORT(ERROR) from here till changes are logged */
 		START_CRIT_SECTION();
 
-		fsm_truncate_avail(BufferGetPage(buf), first_removed_slot);
+		changed = fsm_truncate_avail(BufferGetPage(buf), first_removed_slot);
 
 		/*
 		 * This change is non-critical, because fsm_does_block_exist() would
@@ -317,7 +319,9 @@ FreeSpaceMapPrepareTruncateRel(Relation rel, BlockNumber nblocks)
 		 * of that many fsm_does_block_exist() rejections.  Use a full
 		 * MarkBufferDirty(), not MarkBufferDirtyHint().
 		 */
-		MarkBufferDirty(buf);
+
+		if (changed)
+			MarkBufferDirty(buf);
 
 		/*
 		 * WAL-log like MarkBufferDirtyHint() might have done, just to avoid
@@ -330,7 +334,7 @@ FreeSpaceMapPrepareTruncateRel(Relation rel, BlockNumber nblocks)
 		 * not changed, and our fork remains valid.  If we crash after that
 		 * flush, redo will return here.
 		 */
-		if (!InRecovery && RelationNeedsWAL(rel) && XLogHintBitIsNeeded())
+		if (changed && !InRecovery && RelationNeedsWAL(rel) && XLogHintBitIsNeeded())
 			log_newpage_buffer(buf, false);
 
 		END_CRIT_SECTION();
-- 
2.43.0