Hi, tks for working on this. I had a chance to look at this while googling BgBufferSync function.
I also think tha reusable_buffers keep track of the number of reusable buffers. BgBufferSync() calls SyncOneBuffer() with skip_recently_used = true. In that case, if SyncOneBuffer() finds the buffer with refcount or usage_count non-zero, it just unlocks the header and returns. Hence when called from BgBufferSync(), SyncOneBuffer() would write a buffer only when it is not used. Hence the result would be 0 or BUF_REUSABLE or BUF_REUSABLE | BUF_WRITTEN. It can never be just BUF_WRITTEN.
Agrees. For this call stack, if skip_recently_used is set to be true, sync_state cannot be BUF_WRITTEN alone.
I guess, a patch like the one attached will be more readable and clear.
I'm new to this part of code, and I found the patch version seems to be more straightforward and less prone to misinterpretation.
I ran pgbench for 5 minutes with this patch applied and didn't see the Assert failing. But I don't think that's a good enough test to cover all scenarios.