Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate()

Hi,

On 9/6/21 1:54 PM, Dilip Kumar wrote:
>
> *CAUTION*: This email originated from outside of the organization. Do 
> not click links or open attachments unless you can confirm the sender 
> and know the content is safe.
>
>
> On Mon, Sep 6, 2021 at 4:04 PM Amit Kapila <amit.kapila16@gmail.com 
> <mailto:amit.kapila16@gmail.com>> wrote:
>
>     On Fri, Aug 13, 2021 at 3:15 PM Drouvot, Bertrand
>     <bdrouvot@amazon.com <mailto:bdrouvot@amazon.com>> wrote:
>     >
>     >
>     > Please find attached a patch proposal to avoid the failed
>     assertion (by ensuring that ReorderBufferChangeMemoryUpdate()
>     being triggered with "addition" set to false in
>     ReorderBufferToastReplace() is done after the elog(ERROR,)).
>     >
>
>     The error can occur at multiple places (like via palloc or various
>     other places) between the first time we subtract the change_size and
>     add it back after the change is re-computed. I think the correct fix
>     would be that in the beginning we just compute the change_size by
>     ReorderBufferChangeSize and then after re-computing the change, we
>     just subtract the old change_size and add the new change_size. What do
>     you think?
>
>
> Yeah, that seems more logical to me.

Thanks for your feedback!

That seems indeed more logical, so I see 3 options to do so:

  1) Add a new API say ReorderBufferChangeMemorySubstractSize() (with a 
Size as one parameter) and make use of it in ReorderBufferToastReplace()

  2) Add a new "Size" parameter to ReorderBufferChangeMemoryUpdate(), so 
that if this parameter is > 0 then it would be used instead of "sz = 
ReorderBufferChangeSize(change)"

  3) Do the substraction directly into ReorderBufferToastReplace() 
without any API

I'm inclined to go for option 2), what do you think?

Thanks

Bertrand