Re: Possible duplicate release of buffer lock.

At Wed, 03 Aug 2016 12:00:33 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <26373.1470240033@sss.pgh.pa.us>
> Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> > My point here is that if concurrent deletion can't be perfomed by
> > the current implement, this while loop could be removed and
> > immediately error out or log a message,
> 
> >> if (P_ISDELETED(opaque) || opaque->btpo_next != target)
> >> {
> >> elog(ERROR, "no left sibling of page %d (concurrent deletion?) in \"%s\"",..
> 
> That would certainly break things: there are valid cases for the
> loop to need to iterate, specifically where the left sibling got
> split before we could acquire lock on it.

Sorry for the garbage. It indeed is just a breakage.

> > or, the while loop at least should stop before overshooting the
> > target.
> 
> >> while (P_ISDELETED(opaque) || opaque->btpo_next != target)
> >> {
> >> /* step right one page */
> >> leftsib = opaque->btpo_next;
> >> _bt_relbuf(rel, lbuf);
> >> if (leftsib == target || leftsib == P_NONE)
> >> {
> >> elog(ERROR, "no left sibling of page %d (concurrent deletion?) in \"%s\"",..
> 
> Huh?  Surely that added test condition could never be true because
> of the second part of the while() test.

It can be masked by P_ISDELETED(opaque), this story is for the
case that the "deleted" page has the right link to the
target. More precisely, the case where a deleted page is seen
during traversing sibling-link even though starting from a live
page (or target, specifically).

As Peter's reply, this is in the case of curruption, which could
be saved just in order to continue performing VACUUM. Or a
violation on a "contract" about page locks. If it's useless, the
P_ISDELETED is also useless unless any corruption, or concurrent
deletion.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center