MIME-Version: 1.0
References: 
 <CAAKRu_ZMw6Npd_qm2KM+FwQ3cMOMx1Dh3VMhp8-V7SOLxdK9-g@mail.gmail.com>
 <CAAKRu_YXP1Us41fkZJeBK5SSj5whNC0C9fZNT6Brdtm+F0agpg@mail.gmail.com>
 <CAAKRu_bAR5uCfjuc06vc_xrZjNCJLs493NgHjTOUDso9qGdE0w@mail.gmail.com>
 <CAAKRu_ac4vUdU-U3ax8j=N8y2OM9-pbFZhafLVfzDOM8MtzvvA@mail.gmail.com>
 <BD5C3345-42F2-4870-89B2-FD8875301714@yandex-team.ru>
 <CAAKRu_a-aVGxNEdkJt+96HGryQXuQNuXe+PhR0KcnUhXSOPBJw@mail.gmail.com>
 <87DD95AA-274F-4F4F-BAD9-7738E5B1F905@yandex-team.ru>
 <CAAKRu_ZH8kL0Zm0j7m7DC9fzk7ru7yf9rm2pEQRvx1iXX25aPQ@mail.gmail.com>
 <CAAKRu_bGCgUuhmy1Mzkw3yCmbGcjNQAjV=OmjuW6hz90PuXKWA@mail.gmail.com>
 <CALdSSPh_P1NJ4LB0BfeNQ1Bg-J=6pSpNzN0O5dHAH6upxfmG-g@mail.gmail.com>
In-Reply-To: 
 <CALdSSPh_P1NJ4LB0BfeNQ1Bg-J=6pSpNzN0O5dHAH6upxfmG-g@mail.gmail.com>
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Wed, 27 Aug 2025 15:08:41 -0400
Message-ID: 
 <CAAKRu_bUrEsbBnE=zy36_UCmYMfzxsV=8Ff8+j79co4-W=ZogA@mail.gmail.com>
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM
 on-access)
To: Kirill Reshke <reshkekirill@gmail.com>
Cc: Andrey Borodin <x4mmm@yandex-team.ru>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Andres Freund <andres@anarazel.de>,
	Robert Haas <robertmhaas@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAAKRu_bUrEsbBnE%3Dzy36_UCmYMfzxsV%3D8Ff8%2Bj79co4-W%3DZogA%40mail.gmail.com>
Precedence: bulk

On Tue, Aug 26, 2025 at 4:01=E2=80=AFPM Kirill Reshke <reshkekirill@gmail.c=
om> wrote:
>
> Few comments on 0003.
>
> 1) This patch introduces XLHP_HAS_VMFLAGS. However it lacks some
> helpful comments about this new status bit.

I added the ones you suggested in my v7 posted here [1].

> 2) Should we move conflict_xid =3D visibility_cutoff_xid; assignment
> just after heap_page_is_all_visible_except_lpdead call in
> lazy_vacuum_heap_page?

Why would we want to do that? We only want to set it if the page is
all visible, so we would have to guard it similarly.

> 3) Looking at this diff, do not comprehend one bit: how are we
> protected from passing an all-visible page to lazy_vacuum_heap_page. I
> did not manage to reproduce such behaviour though.
>
> + if ((vmflags & VISIBILITYMAP_VALID_BITS) !=3D 0)
> + {
> + Assert(!PageIsAllVisible(page));
> + set_pd_all_vis =3D true;
> + LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE);
> + PageSetAllVisible(page);
> + visibilitymap_set_vmbyte(vacrel->rel,
> + blkno,

So, for one, there is an assert just above this code in
lazy_vacuum_heap_page() that nunused > 0 -- so we know that the page
couldn't have been all-visible already because it had unused line
pointers.

Otherwise, if it was possible for an already all-visible page to get
here, the same thing would happen that happens on master --
heap_page_is_all_visible[_except_lpdead()] would return true and we
would try to set the VM which would end up being a no-op.

- Melanie

[1] https://www.postgresql.org/message-id/CAAKRu_YD0ecXeAh%2BDmJpzQOJwcRzmM=
yGdcc5W_0pEF78rYSJkQ%40mail.gmail.com