Error in StrategySyncStart() prologue

Started by Ashutosh Bapatabout 1 year ago4 messageshackers
Jump to latest
#1Ashutosh Bapat
ashutosh.bapat@enterprisedb.com

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+2-3
#2Michael Paquier
michael@paquier.xyz
In reply to: Ashutosh Bapat (#1)
Re: Error in StrategySyncStart() prologue

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

#3Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#2)
Re: Error in StrategySyncStart() prologue

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

#4Ashutosh Bapat
ashutosh.bapat@enterprisedb.com
In reply to: Michael Paquier (#3)
Re: Error in StrategySyncStart() prologue

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