Обсуждение: issue: nbtpage.c,_bt_pagedel may get wrong result
# issue
/*
* To avoid deadlocks, we'd better drop the leaf page lock
* before going further.
*/
_bt_unlockbuf(rel, leafbuf);
/*
* Check that the left sibling of leafbuf (if any) is not
* marked with INCOMPLETE_SPLIT flag before proceeding
*/
Assert(leafblkno == scanblkno);
if (_bt_leftsib_splitflag(rel, leftsib, leafblkno))
{
ReleaseBuffer(leafbuf);
return;
}
After unlocking leaf, but before call _bt_leftsib_splitflag, leftsib may be already split, and leafbuf's current left sibling is in INCOMPLETE_SPLIT status.
# how to fix
In function _bt_leftsib_splitflag, after lock leftsib, we should lock leafbuf again, then recheck if leafbuf's leftsib is still leftsib, if check passed, keep lock on leafbuf.
-- Regards
Wang Pengfei
Wang Pengfei
On 23/05/2024 13:06, Pengfei Wang wrote: > # issue > https://github.com/postgres/postgres/blob/da32f5c4bca7f3447b869de2afbbfa0b74443d45/src/backend/access/nbtree/nbtpage.c#L1943 <https://github.com/postgres/postgres/blob/da32f5c4bca7f3447b869de2afbbfa0b74443d45/src/backend/access/nbtree/nbtpage.c#L1943> > > /* > * To avoid deadlocks, we'd better drop the leaf page lock > * before going further. > */ > _bt_unlockbuf(rel, leafbuf); > > /* > * Check that the left sibling of leafbuf (if any) is not > * marked with INCOMPLETE_SPLIT flag before proceeding > */ > Assert(leafblkno == scanblkno); > if (_bt_leftsib_splitflag(rel, leftsib, leafblkno)) > { > ReleaseBuffer(leafbuf); > return; > } > > After unlocking leaf, but before call _bt_leftsib_splitflag, leftsib may > be already split, and leafbuf's current left sibling is in > INCOMPLETE_SPLIT status. In that case, there should be a downlink for 'leafbuf' in the parent, and we're good to proceed, right? See the comments in _bt_leftsib_splitflag(). -- Heikki Linnakangas Neon (https://neon.tech)
Yes, you are right. I've checked _bt_leftsib_splitflag, it checks if the right sibling of 'leftisb' is till leafbuf.
Thank you for your explanation.
On Thu, May 23, 2024 at 7:13 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 23/05/2024 13:06, Pengfei Wang wrote:
> # issue
> https://github.com/postgres/postgres/blob/da32f5c4bca7f3447b869de2afbbfa0b74443d45/src/backend/access/nbtree/nbtpage.c#L1943 <https://github.com/postgres/postgres/blob/da32f5c4bca7f3447b869de2afbbfa0b74443d45/src/backend/access/nbtree/nbtpage.c#L1943>
>
> /*
> * To avoid deadlocks, we'd better drop the leaf page lock
> * before going further.
> */
> _bt_unlockbuf(rel, leafbuf);
>
> /*
> * Check that the left sibling of leafbuf (if any) is not
> * marked with INCOMPLETE_SPLIT flag before proceeding
> */
> Assert(leafblkno == scanblkno);
> if (_bt_leftsib_splitflag(rel, leftsib, leafblkno))
> {
> ReleaseBuffer(leafbuf);
> return;
> }
>
> After unlocking leaf, but before call _bt_leftsib_splitflag, leftsib may
> be already split, and leafbuf's current left sibling is in
> INCOMPLETE_SPLIT status.
In that case, there should be a downlink for 'leafbuf' in the parent,
and we're good to proceed, right? See the comments in
_bt_leftsib_splitflag().
--
Heikki Linnakangas
Neon (https://neon.tech)
Regards
Wang Pengfei
Wang Pengfei