Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate() - Mailing list pgsql-hackers

From Drouvot, Bertrand
Subject Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate()
Date
Msg-id 73dcd904-cfee-21f2-d363-317cd517cb00@amazon.com
Whole thread Raw
In response to Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate()  (Dilip Kumar <dilipbalaut@gmail.com>)
Responses Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate()  (Dilip Kumar <dilipbalaut@gmail.com>)
List pgsql-hackers

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> wrote:
On Fri, Aug 13, 2021 at 3:15 PM Drouvot, Bertrand <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

pgsql-hackers by date:

Previous
From: gkokolatos@pm.me
Date:
Subject: Re: Showing I/O timings spent reading/writing temp buffers in EXPLAIN
Next
From: Thomas Habets
Date:
Subject: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert