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(dot)kapila16(at)gmail(dot)com
> <mailto:amit(dot)kapila16(at)gmail(dot)com>> wrote:
>
> On Fri, Aug 13, 2021 at 3:15 PM Drouvot, Bertrand
> <bdrouvot(at)amazon(dot)com <mailto:bdrouvot(at)amazon(dot)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