On Sat, Jul 18, 2026 at 2:22 AM Melanie Plageman <[email protected]> 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.
