Re: Eagerly evict bulkwrite strategy ring

Hi Melanie,

I remember I ever reviewed this patch. But when I revisit v7, I just got a confusion to clarify.

> On Nov 19, 2025, at 03:13, Melanie Plageman <melanieplageman@gmail.com> wrote:
> 
> On Mon, Nov 3, 2025 at 3:06 PM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
>> 
>> I found an incorrect assert in CleanVictimBuffer() that was tripping
>> in CI. Attached v6 is updated with it removed.
> 
> v7 rebased over recent changes in bufmgr.c
> 
> - Melanie
> <v7-0001-Refactor-goto-into-for-loop-in-GetVictimBuffer.patch><v7-0002-Split-FlushBuffer-into-two-parts.patch><v7-0003-Eagerly-flush-bulkwrite-strategy-ring.patch>

0001 only changes “goto" to “for”, thus it's supposed no logic change.

In the old code:
```
		if (strategy != NULL)
		{
			XLogRecPtr	lsn;

			/* Read the LSN while holding buffer header lock */
			buf_state = LockBufHdr(buf_hdr);
			lsn = BufferGetLSN(buf_hdr);
			UnlockBufHdr(buf_hdr);

			if (XLogNeedsFlush(lsn)
				&& StrategyRejectBuffer(strategy, buf_hdr, from_ring))
			{
				LWLockRelease(content_lock);
				UnpinBuffer(buf_hdr);
				goto again;
			}
		}
```
It only retries when XLogNeedsFlush(lsn) is true.

In the patch, everything are merged into StrategyRejectBuffer():
```
			if (StrategyRejectBuffer(strategy, buf_hdr, from_ring))
			{
				LWLockRelease(content_lock);
				UnpinBuffer(buf_hdr);
				continue;
			}
```
When StrategyRejectBuffer(strategy, buf_hdr, from_ring) is true, retry happens. However, look into the function:
```
bool
StrategyRejectBuffer(BufferAccessStrategy strategy, BufferDesc *buf, bool from_ring)
{
	XLogRecPtr	lsn;

	if (!strategy)
		return false;

	/* We only do this in bulkread mode */
	if (strategy->btype != BAS_BULKREAD)
		return false;

	/* Don't muck with behavior of normal buffer-replacement strategy */
	if (!from_ring ||
		strategy->buffers[strategy->current] != BufferDescriptorGetBuffer(buf))
		return false;

	LockBufHdr(buf);
	lsn = BufferGetLSN(buf);
	UnlockBufHdr(buf);

	if (XLogNeedsFlush(lsn))
		return false;
```

When XLogNeedsFlush(lsn) is true, StrategyRejectBuffer returns false, thus no retry will happen, which is different from the old logic, is that an intentional change?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/