Re: nbtree backwards scan test coverage

From: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: nbtree backwards scan test coverage
Date: 2026-07-20 12:14:27
Message-ID: 82D5177B-70B2-4611-BE8C-4B8223E205A9@yandex-team.ru
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 20 Jul 2026, at 04:23, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> Attached patches add an isolation test to the nbtree test module,
> which uses injection points. This provides thorough test coverage for
> nbtree backwards scans, including cases where we must recover from
> concurrent page splits, and page deletions by VACUUM.

Hi Peter,

Thank you for adding coverage here. _bt_lock_and_validate_left() really
worth testing. The permutations read well and the mapping "one notice
per recovery path" makes sense.

Some nits:

1. The "returns a half-dead page" case

Maybe this is out of scope of this patch.

Returning a half-dead page is the ordinary return, not one
of the recovery paths this patch is about. The actual half-dead
handling then lives in the caller's P_IGNORE branch.

There is an existing nbtree_half_dead_pages regress test, which already
tests a half-dead page, but only forwards:

> select * from nbtree_half_dead_pages where id > 99998 and id < 120002;

A sibling query with ORDER BY id DESC (a backward Index Scan) would
close the loop for the P_IGNORE-on-the-left branch without adding a
permutation here. Purely optional, just mentioning.

2. Only two of the four points are lwlock-free; the other two fire
under a buffer lock

The test only ever 'wait's on lock-and-validate-left and uses 'notice'
for the two in-lock points, so it is fine as written.
Optionally consider a one-line comment near to INJECTION_POINT noting
that it fires under buffer lock.

3. Fixed BLCKSZ

I think that is fine, and there is recent precedent in GIN amcheck tests.
When that went in, Arseniy noted that "some tests will fail with not
standard block size", and Tomas's conclusion was [2]:

> AFAIK we don't expect tests to be 100% stable with other block
> sizes. It shouldn't crash / segfault, ofc, but some tests may be
> sensitive to this.

Personally, I'd like to enable test on various page sizes one day. But
it is large project anyway. An isolation spec has no runtime block_size
hook right now to skip the test.

There's another similar test that may be useful. When we converted B-tree
vacuum to the read stream API, Melanie built essentially the same style of
test for the concurrent page splits during a physical-order nbtree scan
forcing backtracking [0,1]. At that time we decided to postpone the test.

Thank you!

Best regards, Andrey Borodin.

[0] https://www.postgresql.org/message-id/flat/CAAKRu_bW1UOyup%3DjdFw%2BkOF9bCaAm%3D9UpiyZtbPMn8n_vnP%2Big%40mail.gmail.com
[1] https://www.postgresql.org/message-id/CAAKRu_aPapmdBD0DYBXVVy7tS+_JyPvC4fWTcoHBBb18EbgaXA@mail.gmail.com
[2] https://www.postgresql.org/message-id/c0577791-0cbd-4548-b809-ff056c7be61d@vondra.me

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2026-07-20 12:15:36 Re: log_min_messages per backend type
Previous Message Nazir Bilal Yavuz 2026-07-20 12:01:12 read stream: Backward I/O combining