Re: Improve the efficiency of _bt_killitems.

On 01/11/2024 10:41, feichanghong wrote:
> 
> 
>> On Nov 1, 2024, at 16:24, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>
>> On 01/11/2024 09:19, feichanghong wrote:
>>> Hi hackers,
>>> In the _bt_killitems function, the following logic is present: we 
>>> search to the right for an index item that matches the heap TID and 
>>> attempt to mark it as dead. If that index item has already been 
>>> marked as dead by other concurrent processes, we will continue
>>> searching. However, there should not be any more matching index items
>>> on the current page.
>> Why could there not be more matching items on the page?
>>
>> Are you assuming a unique index? Even then it's not right; you can 
>> have multiple index entries point to different heap tuples with the 
>> same key, as long as they're not visible at the same time. For 
>> example, if you UPDATE or DELETE+INSERT a row.
> 
> Maybe I didn't describe it clearly. What I meant to say is that there 
> shouldn't
> be multiple index items on the current page pointing to the same heap 
> TID(ctid).
> rather than the same index key.

Ah, gotcha. That makes sense.

There's more we could do here in the similar vein:

If the TID was found in a posting list, but not all of the items were 
dead, 'killtuple' is set to false so we will still continue the search.

Then there's this comment:

>                 /*
>                  * Don't bother advancing the outermost loop's int iterator to
>                  * avoid processing killed items that relate to the same
>                  * offnum/posting list tuple.  This micro-optimization hardly
>                  * seems worth it.  (Further iterations of the outermost loop
>                  * will fail to match on this same posting list's first heap
>                  * TID instead, so we'll advance to the next offnum/index
>                  * tuple pretty quickly.)
>                  */

I think we should do that micro-optimization. It's a single line of code 
and a single instruction to advance 'i' variable AFAICS. While I agree 
with the comment that it doesn't matter much performance-wise, it seems 
simpler to just do it than explain why we don't do it.

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