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. > The patch LGTM. -- > Best Wishes, > Ashutosh Bapat >
