Re: Avoiding superfluous buffer locking during nbtree backwards scans

On Thu, Nov 7, 2024 at 5:44 AM Masahiro Ikeda
<ikedamsh(at)oss(dot)nttdata(dot)com> wrote:0-12 08:29, Peter Geoghegan wrote:
> > On Thu, Oct 10, 2024 at 1:41 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> > * We now reset currPos state (including even its moreLeft/moreRight
> > fields) within _bt_parallel_seize, automatically and regardless of any
> > other details.
>
> IIUC, the above change is the root cause. The commit 1bd4bc8 adds a
> reset of
> the currPos state in _bt_parallel_seize(). However, this change can
> overwrite
> currPos.moreRight which should be preserved before calling
> _bt_readnextpage().

I can easily recreate the problem using your test case. I agree with
your diagnosis. Thanks for the report!

We could fix this by going back to how things were before -- don't
unset moreLeft/moreRight within _bt_parallel_seize. But that doesn't
seem like a good idea to me. It would be weird to depend on the
currPos state from the last _bt_readpage call, even after seizing the
scan for the next page in line. If the scan has already finished
(according to moreLeft/moreRight), then why even try to seize the scan
once again? Why not just end the scan instead (including its call to
_bt_parallel_done)?

> * Test case

> -- Without commit 1bd4bc8,
> the number was '236' in my environment.
> Planning Time: 0.105 ms
> Execution Time: 71.454 ms
> (18 rows)

With the attached patch, I get back to the good/correct behavior.

I'm not sure that this is the exact right way to fix the bug, but it
seems like the right general approach.

--
Peter Geoghegan