Error in StrategySyncStart() prologue
Hi,
The prologue of StrategySyncStart() mentions BufferSync(), but the
latter function does not call the first function. Instead
StrategySyncStart() is called by BgBufferSync() which uses the buffer
id provided by the first function to decide the buffer from where to
start syncing as mentioned in the first function's prologue.
BufferSync, on the other hand, scans all the buffers and does not use
the output of StrategySyncStart(). I think the prologue of
StrategySyncStart() intends to refer to BgBufferSync() and not
BufferSync(). PFA patch fixing the prologue.
--
Best Wishes,
Ashutosh Bapat
Attachments:
0001-Fix-StrategySyncStart-prologue-20250130.patchtext/x-patch; charset=US-ASCII; name=0001-Fix-StrategySyncStart-prologue-20250130.patchDownload
From 0a3dd2442860f592141c7bb43bb668daea61c676 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Thu, 30 Jan 2025 10:27:26 +0530
Subject: [PATCH] Fix StrategySyncStart() prologue
The prologue of StrategySyncStart() mentions BufferSync(), but the
latter function does not call the first function. Instead
StrategySyncStart() is called by BgBufferSync() which uses the buffer id
provided by the first function to decide the buffer from where to start
syncing as mentioned in the first function's prologue. BufferSync, on
the other hand, scans all the buffers and does not use the output of
StrategySyncStart(). Hence the prologue of StrategySyncStart() intends
to refer to BgBufferSync() and not BufferSync(). Fix this error.
Author: Ashutosh Bapat
---
src/backend/storage/buffer/freelist.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 1f757d96f07..336715b6c63 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -380,10 +380,10 @@ StrategyFreeBuffer(BufferDesc *buf)
}
/*
- * StrategySyncStart -- tell BufferSync where to start syncing
+ * StrategySyncStart -- tell BgBufferSync where to start syncing
*
* The result is the buffer index of the best buffer to sync first.
- * BufferSync() will proceed circularly around the buffer array from there.
+ * BgBufferSync() will proceed circularly around the buffer array from there.
*
* In addition, we return the completed-pass count (which is effectively
* the higher-order bits of nextVictimBuffer) and the count of recent buffer
base-commit: bb3ec16e14ded1d23a46d3c7e623a965164fa124
--
2.34.1
On Thu, Jan 30, 2025 at 11:04:19AM +0530, Ashutosh Bapat wrote:
The prologue of StrategySyncStart() mentions BufferSync(), but the
latter function does not call the first function. Instead
StrategySyncStart() is called by BgBufferSync() which uses the buffer
id provided by the first function to decide the buffer from where to
start syncing as mentioned in the first function's prologue.
BufferSync, on the other hand, scans all the buffers and does not use
the output of StrategySyncStart(). I think the prologue of
StrategySyncStart() intends to refer to BgBufferSync() and not
BufferSync(). PFA patch fixing the prologue.
Indeed, your suggestion sounds right. StrategySyncStart() is only
used in BgBufferSync() for the background writer since 9cd00c457e6a,
and this commit seems to have missed the comment update.
--
Michael
On Thu, Jan 30, 2025 at 05:36:32PM +0900, Michael Paquier wrote:
Indeed, your suggestion sounds right. StrategySyncStart() is only
used in BgBufferSync() for the background writer since 9cd00c457e6a,
and this commit seems to have missed the comment update.
And done.
--
Michael
On Fri, Jan 31, 2025 at 7:38 AM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Jan 30, 2025 at 05:36:32PM +0900, Michael Paquier wrote:
Indeed, your suggestion sounds right. StrategySyncStart() is only
used in BgBufferSync() for the background writer since 9cd00c457e6a,
and this commit seems to have missed the comment update.And done.
--
Michael
Thanks a lot.
Your archaeology investigation is correct. I looked through the
changes to the names and signatures of BufferSync and BgBufferSync but
most of them were more than a decade ago. So I was doubtful about this
error being so far in the history. I traced the changes to the
function body but not as far as 8 years back.
--
Best Wishes,
Ashutosh Bapat