Re: [PoC] Improve dead tuple storage for lazy vacuum

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: John Naylor <johncnaylorls(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>
Subject: Re: [PoC] Improve dead tuple storage for lazy vacuum
Date: 2024-02-20 06:59:04
Message-ID: CAD21AoB6OUYiEEemOWKEBr7ewn-ofr8j-KZVUr0zUgVFxQ=qeg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Feb 19, 2024 at 7:47 PM John Naylor <johncnaylorls(at)gmail(dot)com> wrote:
>
> On Mon, Feb 19, 2024 at 9:02 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > I think that vacuum and tidbitmap (and future users) would end up
> > having the same max block size calculation. And it seems slightly odd
> > layering to me that max-block-size-specified context is created on
> > vacuum (or tidbitmap) layer, a varlen-value radix tree is created by
> > tidstore layer, and the passed context is used for leaves (if
> > varlen-value is used) on radix tree layer.
>
> That sounds slightly more complicated than I was thinking of, but we
> could actually be talking about the same thing: I'm drawing a
> distinction between "used = must be detected / #ifdef'd" and "used =
> actually happens to call allocation". I meant that the passed context
> would _always_ be used for leaves, regardless of varlen or not. So
> with fixed-length values short enough to live in child pointer slots,
> that context would still be used for iteration etc.
>
> > Another idea is to create a
> > max-block-size-specified context on the tidstore layer. That is,
> > vacuum and tidbitmap pass a work_mem and a flag indicating whether the
> > tidstore can use the bump context, and tidstore creates a (aset of
> > bump) memory context with the calculated max block size and passes it
> > to the radix tree.
>
> That might be a better abstraction since both uses have some memory limit.

I've drafted this idea, and fixed a bug in tidstore.c. Here is the
summary of updates from v62:

- removed v62-0007 patch as we discussed
- squashed v62-0006 and v62-0008 patches into 0003 patch
- v63-0008 patch fixes a bug in tidstore.
- v63-0009 patch is a draft idea of cleanup memory context handling.

>
> > As for using the bump memory context, I feel that we need to store
> > iterator struct in aset context at least as it can be individually
> > freed and re-created. Or it might not be necessary to allocate the
> > iterator struct in the same context as radix tree.
>
> Okay, that's one thing I was concerned about. Since we don't actually
> have a bump context yet, it seems simple to assume aset for non-nodes,
> and if we do get it, we can adjust slightly. Anyway, this seems like a
> good thing to try to clean up, but it's also not a show-stopper.
>
> On that note: I will be going on honeymoon shortly, and then to PGConf
> India, so I will have sporadic connectivity for the next 10 days and
> won't be doing any hacking during that time.

Thank you for letting us know. Enjoy yourself!

Regards

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v63-ART.tar.gz application/x-gzip 56.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2024-02-20 07:05:06 Re: confirmed flush lsn seems to be move backward in certain error cases
Previous Message Bharath Rupireddy 2024-02-20 06:35:00 Re: Introduce XID age and inactive timeout based replication slot invalidation