Re: Orphan page in _bt_split

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Константин Книжник <knizhnik(at)garret(dot)ru>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Orphan page in _bt_split
Date: 2025-09-25 04:35:07
Message-ID: aNTGe2KuCNaRjsqs@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Sep 22, 2025 at 07:44:18PM +0300, Константин Книжник wrote:
> Attached please find rebased version of the patch with fixed mistypings.

I have looked at v3.

- leftpage = PageGetTempPage(origpage);
+ leftpage = leftpage_buf.data;
+ memcpy(leftpage, origpage, BLCKSZ);
_bt_pageinit(leftpage, BufferGetPageSize(buf));

What's the point of copying the contents of origpage to leftpage?
_bt_pageinit() is going to initialize leftpage (plus a few more things
set like the page LSN, etc.), so the memcpy should not be necessary,
no?

+ rightpage = BufferGetPage(rbuf);
+ memcpy(rightpage, rightpage_buf.data, BLCKSZ);
+ ropaque = BTPageGetOpaque(rightpage);

When we reach this state of the logic, aka at the beginning of the
critical section, the right and left pages are in a correct state,
and your code is OK because we copy the contents of the right page
back into its legitimate place. What is not OK to me is that the
copy of the "temporary" right page back to "rbuf" is not explained. I
think that this deserves a comment, especially the part about ropaque
which is set once by this proposal, then manipulated while in the
critical section.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2025-09-25 04:46:35 Re: Report bytes and transactions actually sent downtream
Previous Message Ashutosh Bapat 2025-09-25 04:23:05 Re: Report bytes and transactions actually sent downtream