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

On Wed, May 6, 2026 at 5:46 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> I don't see anywhere where something like this is documented. Though
> I'm not entirely sure what you mean. In the cases with one VM block
> and one heap block, you mean we should have documented that you should
> acquire both locks before making changes to either page?
>
> Thinking about this made me wonder if I should refactor the patches a
> bit to acquire the vmbuffer lock before the critical section instead.

I've done this in attached v3. I've only included proposed changes to
18, as this ended up being a pretty invasive refactor that I'd like to
get feedback on first.

While working on v3, some new tests that were added (by another
commit) exposed a bug in this patch. Because it's possible and not
incorrect for the VM bits to already be clear in the VM while
PD_ALL_VISIBLE is set on the heap page and because
visibilitymap_clear() only marks the VM buffer dirty if we actually
had to clear the bits, we can end up needing to clear PD_ALL_VISIBLE
but not dirtying the VM buffer. This basically worked before we
started registering the VM buffer in the WAL record, but now that we
do that, we have to be sure we only do so if the buffer has been
modified (because the WAL machinery asserts that the registered buffer
is dirty). Tests passed before, so we must not have had coverage of
this scenario.

Unfortunately that requirement makes an already complicated patch more
complicated -- especially heap_update(). Fixing that for heap_update()
necessitated several new variables to represent the various states.
It also creates several complicated scenarios.

For example, it is possible that though both new and old pages need
PD_ALL_VISIBLE cleared and each of their VM bits were on different
pages but the old VM buffer is omitted because it was already clear.
This was previously indistinguishable from when old and new pages VM
bits are on the same VM buffer. I think this is fixed by checking if
old page's VM bits are on vmbuffer_new...but wowza, it's all pretty
complicated now.

- Melanie