From: Drouvot, Bertrand <bdrouvot@amazon.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate()
Date: Tue, 7 Sep 2021 09:11:01 +0200
Message-ID: <7e2a1da1-80e1-64c4-1512-78e47a6711d4@amazon.com> (raw)
In-Reply-To: <CAA4eK1L9JbhsHZpeaFrAMc7ZrPUHqd6AAERxqwZQ5n9J4=X1-g@mail.gmail.com>
References: <92b0ee65-b8bd-e42d-c082-4f3f4bf12d34@amazon.com>
<CAA4eK1LfzPdTO73vW2qUkJeB1k1iL04rmqVJseBbF5qRAonmiw@mail.gmail.com>
<CAFiTN-uoTz0bUtOoZxJcB7kPt9xMrKsDATTXoJfktJtPOfC_bw@mail.gmail.com>
<73dcd904-cfee-21f2-d363-317cd517cb00@amazon.com>
<CAFiTN-vMGg3EEe4icesj4LJ_g0JWYq0SqHEPbtQzm+fwfaz4cg@mail.gmail.com>
<CAA4eK1KDYWp4mtCOpGymMHXd1oi_Wqy+LS7X-NwpF=pp2MSJSg@mail.gmail.com>
<CAFiTN-uZmCkLG=7Ak32tyE+KT38XhZ48pZTc7OLuX3zvDXSwRQ@mail.gmail.com>
<CAA4eK1L_HHFnZB+B68h9qv_qqkLbjYbw7M653+LjM93_N4OzNg@mail.gmail.com>
<CAFiTN-smGWa8zYVsQVm7qRdXsr7JD8sokcKdfBhyF1onRORGdw@mail.gmail.com>
<51babafd-4c0c-9b69-ec5c-5e4740d2606b@amazon.com>
<CAA4eK1L9JbhsHZpeaFrAMc7ZrPUHqd6AAERxqwZQ5n9J4=X1-g@mail.gmail.com>
On 9/7/21 8:51 AM, Amit Kapila wrote:
> On Tue, Sep 7, 2021 at 11:33 AM Drouvot, Bertrand <bdrouvot@amazon.com> wrote:
>> Hi,
>>
>> On 9/7/21 7:58 AM, Dilip Kumar wrote:
>>
>>
>> On Tue, Sep 7, 2021 at 11:10 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>>>>> Isn't it better if we use option 2) at all places as then we won't
>>>>> need any special check inside ReorderBufferChangeMemoryUpdate()?
>>>>
>>>> If we want to do this then be careful about REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID change. Basically, ReorderBufferChangeMemoryUpdate() ignores this type of change whereas ReorderBufferChangeSize(), consider at least sizeof(ReorderBufferChange) bytes to this change. So if we compute the size using ReorderBufferChangeSize() outside of ReorderBufferChangeMemoryUpdate(), then total size will be different from what we have now. Logically, we should be ignoring/asserting REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID in ReorderBufferChangeSize(), because ReorderBufferChangeMemoryUpdate() is the only caller for this function.
>>>>
>>> Why can't we simply ignore it in ReorderBufferChangeMemoryUpdate() as
>>> we are doing now?
>>
>> Yeah right, we can actually do that, it doesn't matter even if we are passing the size from outside.
>>
>> Agree, if no objections, I'll prepare a patch with the modified approach of option 2) proposed by Amit (means passing the size from the outside in all the cases).
>>
> Sounds reasonable. Another point that needs some thought is do we want
> to backpatch this change till v13? I am not sure if there is any
> user-visible bug here but maybe it is still good to fix this in back
> branches. What do you think?
Yes, +1 to backpatch till v13 "per precaution".
I will first come back with a proposed version for master and once we
agree on a final/polished version then I'll do the backpatch ones (if
needed).
Thanks
Bertrand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bdrouvot@amazon.com, amit.kapila16@gmail.com, dilipbalaut@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate()
In-Reply-To: <7e2a1da1-80e1-64c4-1512-78e47a6711d4@amazon.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox