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: 2023-12-20 11:35:38
Message-ID: CAD21AoCAPTM3RFNAn44W_b=uWOL1E7Wke5k-LTOK9jqyMMoK0w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Dec 19, 2023 at 4:37 PM John Naylor <johncnaylorls(at)gmail(dot)com> wrote:
>
> On Tue, Dec 19, 2023 at 12:37 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Mon, Dec 18, 2023 at 3:41 PM John Naylor <johncnaylorls(at)gmail(dot)com> wrote:
> > > Let's do it in just one place. In TidStoreCreate(), do
> > >
> > > /* clamp max_bytes to at least the size of the empty tree with
> > > allocated blocks, so it doesn't immediately appear full */
> > > ts->control->max_bytes = Max(max_bytes, {rt, shared_rt}_memory_usage);
> > >
> > > Then we can get rid of all the worry about 1MB/2MB, 64kB, 70kB -- all that.
> >
> > But doesn't it mean that even if we create a shared tidstore with
> > small memory, say 64kB, it actually uses 1MB?
>
> This sounds like an argument for controlling the minimum DSA segment
> size. (I'm not really in favor of that, but open to others' opinion)
>
> I wasn't talking about that above -- I was saying we should have only
> one place where we clamp max_bytes so that the tree doesn't
> immediately appear full.

Thank you for your clarification. Understood.

I've updated the new patch set that incorporated comments I got so
far. 0007, 0008, and 0012 patches are updates from the v45 patch set.
In addition to the review comments, I made some changes in tidstore to
make it independent from heap. Specifically, it uses MaxOffsetNumber
instead of MaxHeapTuplesPerPage. Now we don't need to include
htup_details.h. It enlarged MaxBlocktableEntrySize but it's still 272
bytes.

BTW regarding the previous comment I got before:

> - RT_PTR_ALLOC *slot;
> + RT_PTR_ALLOC *slot = NULL;
>
> We have a macro for invalid pointer because of DSA.

I think that since *slot is a pointer to a RT_PTR_ALLOC it's okay to set NULL.

As for the initial and maximum DSA segment sizes, I've sent a summary
on that thread:

https://www.postgresql.org/message-id/CAD21AoCVMw6DSmgZY9h%2BxfzKtzJeqWiwxaUD2T-FztVcV-XibQ%40mail.gmail.com

I'm going to update RT_DUMP() and RT_DUMP_NODE() codes for the next step.

Regards,

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

Attachment Content-Type Size
v46-ART.tar.gz application/gzip 70.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2023-12-20 11:36:51 Re: Synchronizing slots from primary to standby
Previous Message Jelte Fennema-Nio 2023-12-20 11:23:04 Support a wildcard in backtrace_functions