Re: Avoiding superfluous buffer locking during nbtree backwards scans

On Thu, Oct 17, 2024 at 5:13 PM Matthias van de Meent
<boekewurm+postgres(at)gmail(dot)com> wrote:
> I'm a little bit concerned about this change: I'm not familiar with
> predicate locks, PLP, or anything related to SSI, but previously we
> called PLP in parallel scans while we still had hold of the scan,
> while we now call PLP only after letting other backends do things,
> allowing PLPs for this scan to happen concurrently with other backends
> of this scan. In principle, that looks like an improvement in
> parallelism by reducing the work done in the critical path.

It sort of is, but not really. FWIW, there were two thoughts behind this:

1. Try to avoid gratuitous BufferGetBlockNumber() calls (use a stashed
BlockNumber instead), since nbtree has a tendency to do too many of
those (something that Andres complained about not long ago).

2. Try to do as little as possible while the backend has seized the
scan, on general principle. If only to set a good example going
forward -- loudly advertising when the scan is seized should also help.

I doubt it matters very much if we call PredicateLockPage() while the
parallel scan is seized -- I wouldn't expect avoiding it (waiting
until the parallel scan has been released to call PredicateLockPage)
will measurably help performance. But I can't see how it could
possibly be incorrect, either.

There is no reason to think that the precise order that we call
PredicateLockPage() for each page matters, in general. Just as it
doesn't matter if the index is scanned forwards or backwards.

> Apart from these two comments, LGTM.

Pushed something very close to v6 several hours ago.

Thanks
--
Peter Geoghegan