public inbox for [email protected]  
help / color / mirror / Atom feed
From: Kirill Reshke <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
Date: Wed, 3 Sep 2025 14:06:40 +0500
Message-ID: <CALdSSPg1=Zi3=1Tv7kT_FrjxvTYyT6LvHVgppSLz+bWjwms6YA@mail.gmail.com> (raw)
In-Reply-To: <CAAKRu_Yc1VKM+iuKuJzncPXCYNqQz_jUFBYXuDiPC5k9sUiiQQ@mail.gmail.com>
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>
	<[email protected]>
	<CAAKRu_a-aVGxNEdkJt+96HGryQXuQNuXe+PhR0KcnUhXSOPBJw@mail.gmail.com>
	<[email protected]>
	<CAAKRu_ZH8kL0Zm0j7m7DC9fzk7ru7yf9rm2pEQRvx1iXX25aPQ@mail.gmail.com>
	<CAAKRu_bGCgUuhmy1Mzkw3yCmbGcjNQAjV=OmjuW6hz90PuXKWA@mail.gmail.com>
	<CALdSSPhAU56g1gGVT0+wG8RrSWE6qW8TOfNJS1HNAWX6wPgbFA@mail.gmail.com>
	<CAAKRu_YD0ecXeAh+DmJpzQOJwcRzmMyGdcc5W_0pEF78rYSJkQ@mail.gmail.com>
	<CALdSSPhu7WZd+EfQDha1nz=DC93OtY1=UFEdWwSZsASka_2eRQ@mail.gmail.com>
	<CAAKRu_a2zU7672weJCGzAE2K44cCwnvsb-BwPh8ET3n1bsKfPQ@mail.gmail.com>
	<CAAKRu_Yc1VKM+iuKuJzncPXCYNqQz_jUFBYXuDiPC5k9sUiiQQ@mail.gmail.com>

On Wed, 3 Sept 2025 at 04:11, Melanie Plageman
<[email protected]> wrote:
>
> On Tue, Sep 2, 2025 at 5:52 PM Melanie Plageman
> <[email protected]> wrote:
> >
> > On Thu, Aug 28, 2025 at 5:12 AM Kirill Reshke <[email protected]> wrote:
> > >
> > > I did micro git-blame research here. I spotted only one related change
> > > [0]. Looks like before this change pin was indeed needed.
> > > But not after this change, so this visibilitymap_pin is just an oversight?
> > > Related thread is [1]. I quickly checked the discussion in this
> > > thread, and it looks like no one was bothered about these lines or VM
> > > logging changes (in this exact pin buffer aspect). The discussion was
> > > of other aspects of this commit.
> >
> > Wow, thanks so much for doing that research. Looking at it myself, it
> > does indeed seem like just an oversight. It isn't harmful since it
> > won't take another pin, but it is confusing, so I think we should at
> > least remove it in master. I'm not as sure about back branches.
>
> I've updated the commit message in the patch set to reflect the
> research you did in attached v8.
>
> - Melanie



Hi!

small comments regarding new series

0001, 0002, 0017 LGTM


In 0015:

```
reshke@yezzey-cbdb-bench:~/postgres$ git diff
src/backend/access/heap/pruneheap.c
diff --git a/src/backend/access/heap/pruneheap.c
b/src/backend/access/heap/pruneheap.c
index 05b51bd8d25..0794af9ae89 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -1398,7 +1398,7 @@ heap_prune_record_unchanged_lp_normal(Page page,
PruneState *prstate, OffsetNumb
                                /*
                                 * For now always use prstate->cutoffs
for this test, because
                                 * we only update 'all_visible' when
freezing is requested. We
-                                * could use
GlobalVisTestIsRemovableXid instead, if a
+                                * could use GlobalVisXidVisibleToAll
instead, if a
                                 * non-freezing caller wanted to set the VM bit.
                                 */
                                Assert(prstate->cutoffs);
```

Also, maybe GlobalVisXidTestAllVisible is a slightly better name? (The
term 'all-visible' is one that we occasionally utilize)


-- 
Best regards,
Kirill Reshke





view thread (143+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
  In-Reply-To: <CALdSSPg1=Zi3=1Tv7kT_FrjxvTYyT6LvHVgppSLz+bWjwms6YA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox