Re: BgBufferSync(): clarification about reusable_buffers variable

Hi,

I went through this part of BgBufferSync() as well, and I agree the
patch is a nice improvement. Hoisting the reusable_buffers++
out of the two branches makes the real condition (BUF_REUSABLE)
explicit, and the Assert captures a real invariant of this call path:
since we pass skip_recently_used = true, SyncOneBuffer() can only
return BUF_WRITTEN together with BUF_REUSABLE, never alone.

Just a small wording nit on the comment. As written,

/*
* We instructed SyncOneBuffer not to write a recently used
* buffer.
*/
Assert(sync_state & BUF_REUSABLE);

it states the precondition rather than the invariant the Assert is
actually checking. I'd find it easier to read if it said the consequence directly, e.g.

/*
* skip_recently_used is true, so SyncOneBuffer() only writes a
* buffer when it's reusable (refcount 0 and usage count 0).
*/
Assert(sync_state & BUF_REUSABLE);

Purely cosmetic, of course, the patch is correct either way.

With or without that tweak, it looks good to me.

Regards,
Aidar Imamov

> On Aug 17, 2026, at 19:58, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> On Mon, Aug 17, 2026 at 9:42 PM Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>>
>> On Mon, May 5, 2025 at 7:07 PM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
>>>
>>> Here's a rebase.
>>
>> Sorry for a very delayed response. Adding this to the next commitfest
>> so as not to forget it again.
>
> And rebased as required by CFBot. No actual conflict.
>
> --
> Best Wishes,
> Ashutosh Bapat
> <v20260817-0001-BgBufferSync-refactor-reusable_buffers-inc.patch>