Re: Avoiding superfluous buffer locking during nbtree backwards scans

On 2024-11-11 12:19, Peter Geoghegan wrote:
> On Sun, Nov 10, 2024 at 9:53 PM Masahiro Ikeda
> <ikedamsh(at)oss(dot)nttdata(dot)com> wrote:
>> I understand, thanks to your explanation.
>
> Cool.
>
>> Now, there is a case where _bt_readnextpage() calls
>> _bt_parallel_seize(),
>> _bt_readpage() sets so->needPrimScan=true, and _bt_parallel_done() is
>> called
>> with so->needPrimScan=true. Prior to this bugfix, _bt_parallel_seize()
>> was
>> called after _bt_readpage() sets so->needPrimScan=true, and it just
>> returned
>> false without calling _bt_parallel_done().
>
> You influenced me to add something about this to my follow-up commit
> caca6d8d:
>
> --- a/src/backend/access/nbtree/nbtsearch.c
> +++ b/src/backend/access/nbtree/nbtsearch.c
> @@ -2230,8 +2230,9 @@ _bt_readnextpage(IndexScanDesc scan, BlockNumber
> blkno,
> !so->currPos.moreRight : !so->currPos.moreLeft))
> {
> /* most recent _bt_readpage call (for lastcurrblkno) ended
> scan */
> + Assert(so->currPos.currPage == lastcurrblkno && !seized);
> BTScanPosInvalidate(so->currPos);
> - _bt_parallel_done(scan);
> + _bt_parallel_done(scan); /* iff !so->needPrimScan */
> return false;
> }
>
> I added "iff !so->needPrimScan" to draw attention to the fact that we
> don't necessarily really end the parallel scan when _bt_parallel_done
> is called.

Thanks! The change made it easier for me to understand.

Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION