Thread: Fix comment for max_cached_tuplebufs definition

Fix comment for max_cached_tuplebufs definition

From
Kuntal Ghosh
Date:
Hello Hackers,

While working on some issue in logical decoding, I found some
inconsistencies in the comment for defining max_cached_tuplebufs in
reorderbuffer.c. It only exists till PG10 because after that the
definition got removed by the generational memory allocator patch. The
variable is defined as follows in reorderbuffer.c:
static const Size max_cached_tuplebufs = 4096 * 2;  /* ~8MB */

And it gets compared with rb->nr_cached_tuplebufs in
ReorderBufferReturnTupleBuf as follows:
if (tuple->alloc_tuple_size == MaxHeapTupleSize &&
    rb->nr_cached_tuplebufs < max_cached_tuplebufs)

                                   {
    rb->nr_cached_tuplebufs++;
}

So, what this variable actually tracks is 4096 * 2 times
MaxHeapTupleSize amount of memory which is approximately 64MB. I've
attached a patch to modify the comment.

But, I'm not sure whether the intention was to keep 8MB cache only. In
that case, I can come up with another patch.

Thoughts?

-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com

Attachment

Re: Fix comment for max_cached_tuplebufs definition

From
Bruce Momjian
Date:
On Fri, Feb  7, 2020 at 02:49:14PM +0530, Kuntal Ghosh wrote:
> Hello Hackers,
> 
> While working on some issue in logical decoding, I found some
> inconsistencies in the comment for defining max_cached_tuplebufs in
> reorderbuffer.c. It only exists till PG10 because after that the
> definition got removed by the generational memory allocator patch. The
> variable is defined as follows in reorderbuffer.c:
> static const Size max_cached_tuplebufs = 4096 * 2;  /* ~8MB */
> 
> And it gets compared with rb->nr_cached_tuplebufs in
> ReorderBufferReturnTupleBuf as follows:
> if (tuple->alloc_tuple_size == MaxHeapTupleSize &&
>     rb->nr_cached_tuplebufs < max_cached_tuplebufs)
> 
>                                    {
>     rb->nr_cached_tuplebufs++;
> }
> 
> So, what this variable actually tracks is 4096 * 2 times
> MaxHeapTupleSize amount of memory which is approximately 64MB. I've
> attached a patch to modify the comment.
> 
> But, I'm not sure whether the intention was to keep 8MB cache only. In
> that case, I can come up with another patch.

Yes, I see you are correct, since each tuplebuf is MaxHeapTupleSize. 
Patch applied from PG 9.5 to PG 10.  Thanks.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +