Re: [PATCH] pg_surgery: Fix WAL corruption from concurrent heap_force_kill

On Wed, May 6, 2026 at 4:27 PM Fabrízio de Royes Mello
<fabriziomello(at)gmail(dot)com> wrote:
>
> On Tue, May 5, 2026 at 5:42 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
> >
> > I verified both the patch, and that the test case catches the bug
> > without the patch, the overall change seems good to me.

I haven't reviewed the test, but I think heap_force_common() has
bigger problems than what your fix fixes. Definitely the lock on the
vmbuffer needs to be held not just while clearing the VM but also
while emitting WAL for those changes. But, actually, even more than
that, the changes to the heap and the accompanying changes to the VM
page should be logged in the same WAL record. It is forbidden for
PD_ALL_VISIBLE to be clear while the VM is set -- which is the state
we are in after replaying the WAL record clearing PD_ALL_VISIBLE
before replaying the one clearing the VM bits. You need all the
changes to happen atomically in one record.

This is briefly discussed as part of a larger bug fix I am working on
to correctly WAL log VM clears [1]. I propose a fix there that emits a
single WAL record for the heap and VM changes while holding a lock on
both pages.

- Melanie

[1] https://www.postgresql.org/message-id/flat/CAAKRu_Z_KAAZAHtuorifR2MRc_MkEcHf-C_t4b9HZaHpa3nriw%40mail.gmail.com#61d104a35fc95fa67708c060ec61d59b