Thread

1. Fix comment for max_cached_tuplebufs definition

   Kuntal Ghosh <kuntalghosh.2007@gmail.com> — 2020-02-07T09:19:14Z

   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
   

2. Re: Fix comment for max_cached_tuplebufs definition

   Bruce Momjian <bruce@momjian.us> — 2020-03-14T21:38:24Z

   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 +