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-21 01:32:37
Message-ID: CAD21AoDBn3=PyPfBR73o8DMZW4i=FMA10TgR+ZWT_HoXe4BBXw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Dec 21, 2023 at 10:19 AM John Naylor <johncnaylorls(at)gmail(dot)com> wrote:
>
> On Wed, Dec 20, 2023 at 6:36 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > 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.
>
> That's a good idea.
>
> > 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.
>
> Ah right, it's the address of the slot.
>
> > I'm going to update RT_DUMP() and RT_DUMP_NODE() codes for the next step.
>
> That could probably use some discussion. A few months ago, I found the
> debugging functions only worked when everything else worked. When
> things weren't working, I had to rip one of these functions apart so
> it only looked at one node. If something is broken, we can't count on
> recursion or iteration working, because we won't get that far. I don't
> remember how things are in the current patch.

Agreed.

I found the following comment and wanted to discuss:

// this might be better as "iterate over nodes", plus a callback to
RT_DUMP_NODE,
// which should really only concern itself with single nodes
RT_SCOPE void
RT_DUMP(RT_RADIX_TREE *tree)

If it means we need to somehow use the iteration functions also for
dumping the whole tree, it would probably need to refactor the
iteration codes so that the RT_DUMP() can use them while dumping
visited nodes. But we need to be careful of not adding overheads to
the iteration performance.

>
> I've finished the node shrinking and addressed some fixme/todo areas
> -- can I share these and squash your v46 changes first?

Cool! Yes, please do so.

Regards,

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2023-12-21 01:48:55 Re: broken master regress tests
Previous Message Michael Paquier 2023-12-21 01:21:45 Re: Add a perl function in Cluster.pm to generate WAL