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. > Other than that the changes in the patch look good to me. Note that I > haven't tested the patch by myself but the test results shown by you > and others in the thread seem sufficient to proceed with this change. Understood. Thank you for the discussion and your help reviewing the patch. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
REL16_v3-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch
Description: Binary data
REL15_v3-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch
Description: Binary data
REL14_v3-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch
Description: Binary data
master_v3-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch
Description: Binary data
REL17_v3-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch
Description: Binary data
REL12_v3-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch
Description: Binary data
REL13_v3-0001-Reduce-memory-block-size-for-decoded-tuple-storag.patch
Description: Binary data
