Re: Avoiding superfluous buffer locking during nbtree backwards scans

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.

--
Peter Geoghegan