Re: Using per-transaction memory contexts for storing decoded tuples

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Using per-transaction memory contexts for storing decoded tuples
Date: 2024-10-15 17:45:01
Message-ID: CAD21AoBASsh8eDFUhR+yYHxQUevoBDf2usD=qiK-asA0LhJpxQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Oct 13, 2024 at 11:00 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Oct 11, 2024 at 3:40 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > Please find the attached patches.
> >

Thank you for reviewing the patch!

>
> @@ -343,9 +343,9 @@ ReorderBufferAllocate(void)
> */
> buffer->tup_context = GenerationContextCreate(new_ctx,
> "Tuples",
> - SLAB_LARGE_BLOCK_SIZE,
> - SLAB_LARGE_BLOCK_SIZE,
> - SLAB_LARGE_BLOCK_SIZE);
> + SLAB_DEFAULT_BLOCK_SIZE,
> + SLAB_DEFAULT_BLOCK_SIZE,
> + SLAB_DEFAULT_BLOCK_SIZE);
>
> Shouldn't we change the comment atop this change [1] which states that
> we should benchmark the existing values?

Agreed.

>
> One more thing we kept the max size as SLAB_DEFAULT_BLOCK_SIZE instead
> of something like we do with ALLOCSET_DEFAULT_SIZES, so we can
> probably write a comment as to why we choose to use the max_size same
> as init_size.

Agreed. I've updated the comment. Please review it.

> BTW, can we once try to use the max size as
> SLAB_LARGE_BLOCK_SIZE? Can it lead to the same problem with concurrent
> transactions where freeing larger blocks could be a problem, if so, we
> can at least write a comment for future reference.

I've tested with SLAB_LARGE_BLOCK_SIZE as the max size but it seems
that a huge memory fragmentation issue still happens. In the same
scenario I used before, the maximum amount of allocated memory during
logical decoding was 1.26GB with logical_decodiing_work_mem being
'256MB'.

Regards,

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

Attachment Content-Type Size
REL14_v2-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch application/octet-stream 3.2 KB
master_v2-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch application/octet-stream 3.6 KB
REL17_v2-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch application/octet-stream 3.6 KB
REL16_v2-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch application/octet-stream 3.6 KB
REL15_v2-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch application/octet-stream 3.6 KB
REL12_v2-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch application/octet-stream 3.2 KB
REL13_v2-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch application/octet-stream 3.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Lakhin 2024-10-15 18:00:00 Re: [PoC] Federated Authn/z with OAUTHBEARER
Previous Message Jacob Champion 2024-10-15 17:30:26 Re: New "raw" COPY format