Re: Why clearing the VM doesn't require registering vm buffer in wal record

On Sat, Jul 18, 2026 at 2:22 AM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> I've committed this.
>

While reading the code committed as ed62d26cac, I wonder whether the
change in commit requires an update in the comments atop
visibilitymap.c. See following observations:

1. The following text on WAL-logging doesn't seem to match the changed
code. "Changes to the visibility map bits are not separately
WAL-logged. Callers must make sure that whenever a visibility map bit
is cleared, the bit is cleared on WAL replay of the updating
operation…"

The VM page is still not logged by its own record. But after commit
ed62d26cac, a caller that clears a bit must register the VM buffer
(block ref *_BLKREF_VM) in the heap record. It must also PageSetLSN()
the VM page. This makes me feel that the above quoted text could be
updated.

2. The LOCKING section is now wrong about the lock order and critical section.

It describes only a pin on the VM page before locking the heap buffer,
and the pin/relock race. Callers now also take an exclusive lock on
the VM buffer before START_CRIT_SECTION() and hold it until after
XLogInsert() and release it before the heap buffer lock.

3. The last LOCKING paragraph also needs an update. I am referring to:
"But when a bit is cleared, we don't have to do that [update the VM
page LSN] because it's always safe to clear a bit in the map from
correctness point of view."

The commit ed62d26cac now sets the VM page LSN on clear when
vmbuffer_modified is true.

--
With Regards,
Amit Kapila.