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 1mNUDE-0003Wt-7f for pgsql-hackers@arkaria.postgresql.org; Tue, 07 Sep 2021 06:04:04 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1mNUDC-0003nm-VA for pgsql-hackers@arkaria.postgresql.org; Tue, 07 Sep 2021 06:04:02 +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 1mNUDC-0003ne-Jy for pgsql-hackers@lists.postgresql.org; Tue, 07 Sep 2021 06:04:02 +0000 Received: from smtp-fw-33001.amazon.com ([207.171.190.10]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mNUD9-0004xe-Sf for pgsql-hackers@lists.postgresql.org; Tue, 07 Sep 2021 06:04:01 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazon201209; t=1630994640; x=1662530640; h=subject:to:cc:references:from:message-id:date: mime-version:in-reply-to; bh=eUKsv/1r9EbcEGednnIhu7tcqNWQ5w1G9cgA8g1Rz7Y=; b=CO724jl54tmhwnavH9vPdTw4LekXke2INZfsrptJzaxlD5D9rQXAeQiP E6pydDZ4pUu9bpw07sw1jeF62S3aovxOdHbyW2k1oZ5vU+3DMzabUw4PP qgLssrGEkk+4e6Q8Tr7c2LVkoRu7WJuTYDgfp0DKaIHJtHbRJ3XHBRxf+ g=; X-IronPort-AV: E=Sophos;i="5.85,274,1624320000"; d="scan'208,217";a="146292592" Received: from iad12-co-svc-p1-lb1-vlan2.amazon.com (HELO email-inbound-relay-2a-90c42d1d.us-west-2.amazon.com) ([10.43.8.2]) by smtp-border-fw-33001.sea14.amazon.com with ESMTP; 07 Sep 2021 06:03:50 +0000 Received: from EX13D03EUC003.ant.amazon.com (pdx1-ws-svc-p6-lb9-vlan3.pdx.amazon.com [10.236.137.198]) by email-inbound-relay-2a-90c42d1d.us-west-2.amazon.com (Postfix) with ESMTPS id 007BEA1BED; Tue, 7 Sep 2021 06:03:48 +0000 (UTC) Received: from 38f9d36c4abb.ant.amazon.com (10.43.161.176) by EX13D03EUC003.ant.amazon.com (10.43.164.192) with Microsoft SMTP Server (TLS) id 15.0.1497.23; Tue, 7 Sep 2021 06:03:43 +0000 Subject: Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate() To: Dilip Kumar , Amit Kapila CC: PostgreSQL Hackers References: <92b0ee65-b8bd-e42d-c082-4f3f4bf12d34@amazon.com> <73dcd904-cfee-21f2-d363-317cd517cb00@amazon.com> From: "Drouvot, Bertrand" Message-ID: <51babafd-4c0c-9b69-ec5c-5e4740d2606b@amazon.com> Date: Tue, 7 Sep 2021 08:03:38 +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="------------5B60F531DC1677A8A57C3E82" Content-Language: en-US X-Originating-IP: [10.43.161.176] X-ClientProxiedBy: EX13D17UWC004.ant.amazon.com (10.43.162.195) 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 --------------5B60F531DC1677A8A57C3E82 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Hi, On 9/7/21 7:58 AM, Dilip Kumar wrote: > > On Tue, Sep 7, 2021 at 11:10 AM Amit Kapila > 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). Thanks Bertrand --------------5B60F531DC1677A8A57C3E82 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit

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).

Thanks

Bertrand

--------------5B60F531DC1677A8A57C3E82--