On Sat, Jul 27, 2024 at 4:04 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> On Wed, Jul 24, 2024 at 8:19 AM John Naylor <johncnaylorls@gmail.com> wrote:
> I ran my test with your patch (on my 64-bit system, non-assert build)
> and the result is great:
>
> master with my test (slightly modified to now use DELETE instead of
> UPDATE as mentioned upthread)
> 3.09s
>
> master with your patch applied, MWM set to 64kB and 9000 rows instead of 800000
> 1.06s
Glad to hear it!
> I took a look at the patch, but I can't say I know enough about the
> memory allocation subsystems and how TIDStore works to meaningfully
> review it -- nor enough about DSM to comment about the interactions.
I tried using parallel vacuum with 64kB and it succeeded, but needed
to perform an index scan for every heap page pruned. It's not hard to
imagine some code moving around so that it doesn't work anymore, but
since this is for testing only, it seems a warning comment is enough.
> I suspect 256kB would also be fast enough to avoid my test timing out
> on the buildfarm, but it is appealing to have a minimum for
> maintenance_work_mem that is the same as work_mem.
Agreed on both counts:
I came up with a simple ctid expression to make the bitmap arrays larger:
delete from test where ctid::text like '%,2__)';
With that, it still takes between 250k and 300k tuples to force a
second index scan with 256kB m_w_m, default fillfactor, and without
asserts. (It may need a few more pages for 32-bit but not many more)
The table is around 1300 pages, where on v16 it's about 900. But with
fewer tuples deleted, the WAL for deletes should be lower. So it might
be comparable to v16's test.
It also turns out that to support 64kB memory settings, we actually
wouldn't need to change radix tree to lazily create memory contexts --
at least currently, SlabCreate doesn't allocate a keeper block, so a
newly created slab context reports 0 for "mem_allocated". So I'm
inclined to go ahead change the minimum m_w_m on v17 and master to
64kB. It's the quickest and (I think) most future-proof way to make
this test work. Any objections?
