Hi Marcel,
On Sat, Feb 8, 2025 at 6:24 AM M vd H <[email protected]> wrote:
>
> I have been reading the source code of the BgWriter, and there is some
> code in BgBufferSync() that I don't fully understand.
>
> In BgBufferSync(), we have the following code:
>
> while (num_to_scan > 0 && reusable_buffers < upcoming_alloc_est)
> {
> int sync_state = SyncOneBuffer(next_to_clean, true,
> wb_context);
>
> if (++next_to_clean >= NBuffers)
> {
> next_to_clean = 0;
> next_passes++;
> }
> num_to_scan--;
>
> if (sync_state & BUF_WRITTEN)
> {
> reusable_buffers++;
> if (++num_written >= bgwriter_lru_maxpages)
> {
> PendingBgWriterStats.maxwritten_clean++;
> break;
> }
> }
> else if (sync_state & BUF_REUSABLE)
> reusable_buffers++;
> }
>
>
> In SyncOneBuffer(), we lock the bufHdr and then check if both the
> refcount and usage_count are zero. If so, we mark the return value as
> BUF_REUSABLE.
> My understanding is that this means that the buffer could be reused
> when am empty shared buffer is needed by a backend. However, in the
> code above, we seem to track these in the reusable_buffers variable.
> But that variable is always incremented when the buffer was written in
> SyncOneBuffer() even though that buffer might have a non-zero refcount
> or non-zero usage_count.
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.
I guess, a patch like the one attached will be more readable and clear.
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.
--
Best Wishes,
Ashutosh Bapat
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 80b0d0c5ded..bdacd5ad980 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -3414,17 +3414,19 @@ BgBufferSync(WritebackContext *wb_context)
}
num_to_scan--;
+ if (sync_state & BUF_REUSABLE)
+ reusable_buffers++;
+
if (sync_state & BUF_WRITTEN)
{
- reusable_buffers++;
+ Assert(sync_state & BUF_REUSABLE);
+
if (++num_written >= bgwriter_lru_maxpages)
{
PendingBgWriterStats.maxwritten_clean++;
break;
}
}
- else if (sync_state & BUF_REUSABLE)
- reusable_buffers++;
}
PendingBgWriterStats.buf_written_clean += num_written;
