Message-ID: <91098347-c594-486d-9c19-26481125a399@iki.fi>
Date: Wed, 10 Apr 2024 08:30:22 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Improve eviction algorithm in ReorderBuffer
Content-Language: en-US
To: Michael Paquier <michael@paquier.xyz>, Jeff Davis <pgsql@j-davis.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>,
 Amit Kapila <amit.kapila16@gmail.com>, vignesh C <vignesh21@gmail.com>,
 Peter Smith <smithpb2250@gmail.com>,
 Tomas Vondra <tomas.vondra@enterprisedb.com>,
 "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
 Shubham Khanna <khannashubham1197@gmail.com>,
 Dilip Kumar <dilipbalaut@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: 
 <CAD21AoAd7ZUz1SVKc+WNqNcTJdBZOBUeV_iExyzqk8WLyoNy1A@mail.gmail.com>
 <f13d88915e7e197ccb78968a825d1c021f62c8ce.camel@j-davis.com>
 <CAD21AoCfP++VF+12Za7SV_vQpRpFbe+Kf_LQsRPAw_pJj0YS3Q@mail.gmail.com>
 <cecac19e0fb5739d4fd955000d19d88d060924d4.camel@j-davis.com>
 <CAD21AoAtR2A4f1-JFUE9KRkiPqMRvxS+3W=ZR7FpuXm8VSVASQ@mail.gmail.com>
 <791d98f474e518387d09eb390b8a12f265d130cc.camel@j-davis.com>
 <8cb6bbf6-57e7-4d14-9664-89505993afaa@iki.fi>
 <10f5dcecb5224c9967a7d595b6168a4144ffc24e.camel@j-davis.com>
 <ZhYD5MhjwzrPHwvu@paquier.xyz>
 <4ce8dd017b911e8b2772147992db468deed71896.camel@j-davis.com>
 <ZhYZdLN3pqueAO2A@paquier.xyz>
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <ZhYZdLN3pqueAO2A@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/91098347-c594-486d-9c19-26481125a399%40iki.fi>
Precedence: bulk

On 10/04/2024 07:45, Michael Paquier wrote:
> On Tue, Apr 09, 2024 at 09:16:53PM -0700, Jeff Davis wrote:
>> On Wed, 2024-04-10 at 12:13 +0900, Michael Paquier wrote:
>>> Wouldn't the best way forward be to revert
>>> 5bec1d6bc5e3 and revisit the whole in v18?
>>
>> Also consider commits b840508644 and bcb14f4abc.
> 
> Indeed.  These are also linked.

I don't feel the urge to revert this:

- It's not broken as such, we're just discussing better ways to 
implement it. We could also do nothing, and revisit this in v18. The 
only must-fix issue is some compiler warnings IIUC.

- It's a pretty localized change in reorderbuffer.c, so it's not in the 
way of other patches or reverts. Nothing else depends on the binaryheap 
changes yet either.

- It seems straightforward to repeat the performance tests with whatever 
alternative implementations we want to consider.

My #1 choice would be to write a patch to switch the pairing heap, 
performance test that, and revert the binary heap changes.

-- 
Heikki Linnakangas
Neon (https://neon.tech)