On Fri, Aug 5, 2016 at 11:27 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> Hello,
>
> If concurrent deletion (marking half-dead, specifically) can't
> happen, P_ISDELETED() won't be seen in the loop but we may
> consider it as a skippable condition to continue VACUUM. But
> still I think we shouldn't release the lock on the target page
> there. But it doesn't harm except that VACUUM stops there so I
> don't mind it will be as it is. So I'd like to have opinions of
> others about this.
>
Isn't the problem here is that due to some reason (like concurrent
split), the code in question (loop --
while (P_ISDELETED(opaque) || opaque->btpo_next != target)) releases
the lock on target buffer and the caller again tries to release the
lock on target buffer when false is returned. So, rather than
changing the loop in a way suggested by you, why can't we ensure that
we don't release the lock on target buffer in that loop or we can
check in the caller such that if target buffer is valid (for a failure
case), then release it.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com