On Wed, Oct 16, 2024 at 10:32 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Tue, Oct 15, 2024 at 9:01 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Tue, Oct 15, 2024 at 11:15 PM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > > > On Sun, Oct 13, 2024 at 11:00 PM Amit Kapila <amit.kapil...@gmail.com> > > > wrote: > > > > > > > > On Fri, Oct 11, 2024 at 3:40 AM Masahiko Sawada <sawada.m...@gmail.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
