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@oss.nttdata.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