Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mNGUI-0002Lk-Gi for pgsql-hackers@arkaria.postgresql.org; Mon, 06 Sep 2021 15:24:46 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1mNGUF-0004Nn-Nz for pgsql-hackers@arkaria.postgresql.org; Mon, 06 Sep 2021 15:24:43 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mNGUF-0004Nd-CI for pgsql-hackers@lists.postgresql.org; Mon, 06 Sep 2021 15:24:43 +0000 Received: from smtp-fw-6002.amazon.com ([52.95.49.90]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mNGUC-0004N8-PK for pgsql-hackers@lists.postgresql.org; Mon, 06 Sep 2021 15:24:42 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazon201209; t=1630941881; x=1662477881; h=to:cc:references:from:message-id:date:mime-version: in-reply-to:subject; bh=hlYaZpMRjRbogTKlSO9fXiyLztZn9tv6CO4gBw/nF3Q=; b=Eik3hAzuEOH17rBWsoK+huzPNeruyyz/vHuc5QWIxN/XIhPyTfHP3iPD cT0WuBGWWi7ipl7+Bn0Oh4PkedmqIhWulmJ51uA7nZpox5BuFvzd6vj4D C7ElAJzETXNLcoKl9JKOcy1JzaPLjmSUNoH6c4yMMGrX9fk5voQLhHXG3 I=; X-IronPort-AV: E=Sophos;i="5.85,272,1624320000"; d="scan'208,217";a="138208140" Subject: Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate() Received: from iad12-co-svc-p1-lb1-vlan2.amazon.com (HELO email-inbound-relay-1d-db82c96c.us-east-1.amazon.com) ([10.43.8.2]) by smtp-border-fw-6002.iad6.amazon.com with ESMTP; 06 Sep 2021 15:24:32 +0000 Received: from EX13D03EUC003.ant.amazon.com (iad12-ws-svc-p26-lb9-vlan3.iad.amazon.com [10.40.163.38]) by email-inbound-relay-1d-db82c96c.us-east-1.amazon.com (Postfix) with ESMTPS id 04471A24E4; Mon, 6 Sep 2021 15:24:30 +0000 (UTC) Received: from 38f9d36c4abb.ant.amazon.com (10.43.162.216) by EX13D03EUC003.ant.amazon.com (10.43.164.192) with Microsoft SMTP Server (TLS) id 15.0.1497.23; Mon, 6 Sep 2021 15:24:28 +0000 To: Dilip Kumar , Amit Kapila CC: PostgreSQL Hackers References: <92b0ee65-b8bd-e42d-c082-4f3f4bf12d34@amazon.com> From: "Drouvot, Bertrand" Message-ID: <73dcd904-cfee-21f2-d363-317cd517cb00@amazon.com> Date: Mon, 6 Sep 2021 17:24:23 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/alternative; boundary="------------0CF97D98DFF39178772249F6" Content-Language: en-US X-Originating-IP: [10.43.162.216] X-ClientProxiedBy: EX13D18UWA003.ant.amazon.com (10.43.160.238) To EX13D03EUC003.ant.amazon.com (10.43.164.192) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --------------0CF97D98DFF39178772249F6 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit 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 > wrote: > > On Fri, Aug 13, 2021 at 3:15 PM Drouvot, Bertrand > > 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 --------------0CF97D98DFF39178772249F6 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit

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

--------------0CF97D98DFF39178772249F6--