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>
In-Reply-To: 
 <CAAKRu_bGCgUuhmy1Mzkw3yCmbGcjNQAjV=OmjuW6hz90PuXKWA@mail.gmail.com>
From: Kirill Reshke <reshkekirill@gmail.com>
Date: Wed, 27 Aug 2025 01:01:24 +0500
Message-ID: 
 <CALdSSPh_P1NJ4LB0BfeNQ1Bg-J=6pSpNzN0O5dHAH6upxfmG-g@mail.gmail.com>
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM
 on-access)
To: Melanie Plageman <melanieplageman@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/CALdSSPh_P1NJ4LB0BfeNQ1Bg-J%3D6pSpNzN0O5dHAH6upxfmG-g%40mail.gmail.com>
Precedence: bulk

On Sat, 2 Aug 2025 at 02:36, Melanie Plageman <melanieplageman@gmail.com> w=
rote:
>
> On Thu, Jul 31, 2025 at 6:58=E2=80=AFPM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
> >
> > The patch "Set-pd_prune_xid-on-insert.txt" can be applied as the last
> > patch in the set. It sets pd_prune_xid on insert (so pages filled by
> > COPY or insert can also be set all-visible in the VM before they are
> > vacuumed). I gave it a .txt extension because it currently fails
> > 035_standby_logical_decoding due to a recovery conflict. I need to
> > investigate more to see if this is a bug in my patch set or elsewhere
> > in Postgres.
>
> I figured out that if we set the VM on-access, we need to enable
> hot_standby_feedback in more places in 035_standby_logical_decoding.pl
> to avoid recovery conflicts. I've done that in the attached updated
> version 6. There are a few other issues in
> 035_standby_logical_decoding.pl that I reported here [1]. With these
> changes, setting pd_prune_xid on insert passes tests. Whether or not
> we want to do it (and what the heuristic should be for deciding when
> to do it) is another question.
>
> - Melanie
>
> [1] https://www.postgresql.org/message-id/flat/CAAKRu_YO2mEm%3DZWZKPjTMU%=
3DgW5Y83_KMi_1cr51JwavH0ctd7w%40mail.gmail.com


0002 No comments from me. Looks straightforward.

Few comments on 0003.

1) This patch introduces XLHP_HAS_VMFLAGS. However it lacks some
helpful comments about this new status bit.

a) In heapam_xlog.h, in xl_heap_prune struct definition:


/*
* If XLHP_HAS_CONFLICT_HORIZON is set, the conflict horizon XID follows,
* unaligned
*/
+ /* If XLHP_HAS_VMFLAGS is set, newly set visibility map bits comes,
unaligned */

b)

we can add here comment why we use  memcpy assignment, akin to /*
memcpy() because snapshot_conflict_horizon is stored unaligned */

+ /* Next are the optionally included vmflags. Copy them out for later use.=
 */
+ if ((xlrec.flags & XLHP_HAS_VMFLAGS) !=3D 0)
+ {
+ memcpy(&vmflags, maindataptr, sizeof(uint8));
+ maindataptr +=3D sizeof(uint8);

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?

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,
+


--=20
Best regards,
Kirill Reshke