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-16 19:25:40
Message-ID: CAD21AoBH2_0KFi0n+u7Zaj3y60ZpfFVze4WeOPXCe-5cMxHw5w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Oct 16, 2024 at 10:32 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Tue, Oct 15, 2024 at 9:01 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Tue, Oct 15, 2024 at 11:15 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > 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.
> > >
> >
> > Can we slightly tweak the comments as follows so that it doesn't sound
> > like a fix for a bug?
> >
> > /* To minimize memory fragmentation caused by long-running
> > transactions with changes spanning multiple memory blocks, we use a
> > single fixed-size memory block for decoded tuple storage. The tests
> > showed that the default memory block size maintains logical decoding
> > performance without causing fragmentation due to concurrent
> > transactions. One might think that we can use the max size as
> > SLAB_LARGE_BLOCK_SIZE but the test also showed it doesn't help resolve
> > the memory fragmentation.
>
> Agreed. I've incorporated your comment in the latest patches. I'm
> going to push them today, barring any objections or further comments.

Pushed. Thank you all for reviewing and testing the patch.

Regards,

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2024-10-16 20:19:53 Soft errors and ThrowErrorData() comment updates
Previous Message Joel Jacobson 2024-10-16 19:13:55 Re: New "raw" COPY format