Re: Avoiding superfluous buffer locking during nbtree backwards scans

On Mon, 19 Aug 2024 at 13:43, Matthias van de Meent
<boekewurm+postgres(at)gmail(dot)com> wrote:
>
> On Sun, 11 Aug 2024 at 21:44, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> >
> > On Tue, Aug 6, 2024 at 6:31 PM Matthias van de Meent
> > <boekewurm+postgres(at)gmail(dot)com> wrote:
> > > +1, LGTM.
> > >
> > > This changes the backward scan code in _bt_readpage to have an
> > > approximately equivalent handling as the forward scan case for
> > > end-of-scan cases, which is an improvement IMO.
>
> Here's a new patch that further improves the situation, so that we
> don't try to re-lock the buffer we just accessed when we're stepping
> backward in index scans, reducing buffer lock operations in the common
> case by 1/2.

Attached is an updated version of the patch, now v2, which fixes some
assertion failures for parallel plans by passing the correct
parameters to _bt_parallel_release for forward scans.

With the test setup below, it unifies the number of buffer accesses
between forward and backward scans:

CREATE TABLE test AS
SELECT generate_series(1, 1000000) as num,
'' j;
CREATE INDEX ON test (num);
VACUUM (FREEZE) test;
SET enable_seqscan = off; SET max_parallel_workers_per_gather = 0;
EXPLAIN (ANALYZE, BUFFERS)
SELECT COUNT(j ORDER BY num DESC) -- or ASC
FROM test;

The test case will have an Index Scan, which in DESC case is backward.
Without this patch, the IS will have 7160 accesses for the ASC
ordering, but 9892 in the DESC case (an increase of 2732,
approximately equivalent to the number leaf pages in the index), while
with this patch, the IndexScan will have 7160 buffer accesses for both
ASC and DESC ordering.

In my previous mail I used buffer lock stats from an index-only scan
as proof of the patch working. It's been pointed out to me that an
IndexScan is easier to extract this data from, as it drops the pin on
the page after getting some results from a page.

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)