I noticed a couple of small clarity issues in the current version of patch for potential clean up:
1. Commit message wording
Right now it says:
“The pg_buffercache_mark_dirty_all() function provides an efficient way to dirty the entire buffer pool (e.g., ~550 ms vs. ~70 ms for 16 GB of shared buffers), complementing pg_buffercache_mark_dirty() for more granular control.”
That makes it sound like the _all function is the granular one, when really:
• pg_buffercache_mark_dirty(buffernumber) is the fine-grained, per-buffer call.
• pg_buffercache_mark_dirty_all() is the bulk, coarse-grained operation.
How about rephrasing to:
“The pg_buffercache_mark_dirty_all() function provides an efficient, bulk way to mark every buffer dirty (e.g., ~70 ms vs. ~550 ms for 16 GB of shared buffers), while pg_buffercache_mark_dirty() still allows per-buffer, granular control.”
2. Inline comment in MarkUnpinnedBufferDirty
We currently have:
PinBuffer_Locked(desc); /* releases spinlock */
Folks who’re unfamiliar with this function might get confused. Maybe we could use the one in GetVictimBuffer:
/* Pin the buffer and then release its spinlock */
PinBuffer_Locked(buf_hdr);
That spelling-out makes it obvious what’s happening.
