From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
---|---|
To: | John Naylor <john(dot)naylor(at)enterprisedb(dot)com> |
Cc: | 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>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PoC] Improve dead tuple storage for lazy vacuum |
Date: | 2023-08-28 14:43:22 |
Message-ID: | CAD21AoAcitpQ_wt5rReE885oh86CMUdS=-+JDfvo6SVFAzvEZQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Aug 28, 2023 at 4:20 PM John Naylor
<john(dot)naylor(at)enterprisedb(dot)com> wrote:
>
> On Sun, Aug 27, 2023 at 7:53 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > I've updated the regression tests for tidstore so that it uses SQL
> > functions to add blocks/offsets and dump its contents. The new test
> > covers the same test coverages but it's executed using SQL functions
> > instead of executing all tests in one SQL function.
>
> This is much nicer and more flexible, thanks! A few questions/comments:
>
> tidstore_dump_tids() returns a string -- is it difficult to turn this into a SRF, or is it just a bit more work?
It's not difficult. I've changed it in v42 patch.
>
> The lookup test seems fine for now. The output would look nicer with an "order by tid".
Agreed.
>
> I think we could have the SQL function tidstore_create() take a boolean for shared memory. That would allow ad-hoc testing without a recompile, if I'm not mistaken.
Agreed.
>
> +SELECT tidstore_set_block_offsets(blk, array_agg(offsets.off)::int2[])
> + FROM blocks, offsets
> + GROUP BY blk;
> + tidstore_set_block_offsets
> +----------------------------
> +
> +
> +
> +
> +
> +(5 rows)
>
> Calling a void function multiple times leads to vertical whitespace, which looks a bit strange and may look better with some output, even if irrelevant:
>
> -SELECT tidstore_set_block_offsets(blk, array_agg(offsets.off)::int2[])
> +SELECT row_number() over(order by blk), tidstore_set_block_offsets(blk, array_agg(offsets.off)::int2[])
>
> row_number | tidstore_set_block_offsets
> ------------+----------------------------
> 1 |
> 2 |
> 3 |
> 4 |
> 5 |
> (5 rows)
Yes, it looks better.
I've attached v42 patch set. I improved tidstore regression test codes
in addition of imcorporating the above comments.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v42-ART.tar.gz | application/x-gzip | 47.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jacob Champion | 2023-08-28 15:12:45 | Re: Fix error handling in be_tls_open_server() |
Previous Message | Aleksander Alekseev | 2023-08-28 14:31:58 | Re: Commitfest manager for September |