nbtree backwards scan test coverage

On 20 Jul 2026, at 04:23, Peter Geoghegan <pg@bowt.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]/messages/by-id/c0577791-0cbd-4548-b809-ff056c7be61d@vondra.me:

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]: /messages/by-id/CAAKRu_bW1UOyup=jdFw+kOF9bCaAm=9UpiyZtbPMn8n_vnP+ig@mail.gmail.com
[1]: /messages/by-id/CAAKRu_aPapmdBD0DYBXVVy7tS+_JyPvC4fWTcoHBBb18EbgaXA@mail.gmail.com
[2]: /messages/by-id/c0577791-0cbd-4548-b809-ff056c7be61d@vondra.me

On Mon, Jul 20, 2026 at 8:14 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:

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.

coverage.postgresql.org shows that that P_IGNORE branch only has one
hit, which is for a forwards scan. It's quite unlikely that that's a
half-dead page; a fully deleted page is much more typical. Whereas
with a backwards scan, it has to be a half-dead page
(_bt_lock_and_validate_left won't ever return a fully deleted page).

BTW, coverage.postgresql.org now accurately represents the coverage
situation. My commit from last night (commit
b087a88c40e45453682bf6d24c42eff33f401662) makes the nbtree module
actually run on the machine that these reports are built on. That
explains why incomplete splits seemed to have no coverage prior prior
to last night.

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.

I don't think that it would, unless there were injection points in
_bt_lock_and_validate_left that the test used.

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.

I don't think that it matters if an injection point fires under a 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.

Cool, that's a useful precedent to point to.

Even if you ran the tests with a non-standard BLCKSZ, you'd still have
to enable injection points to see test "failures".

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.

The tests wouldn't test anything with a non-standard BLCKSZ. You'd
have to write an entirely different version of the test for each block
size, I think (at least if we expect to count the number of times
_bt_lock_and_validate_left moves right to recover from the
lastcurrblkno being deleted).

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.

That's not a bad idea. You'd really need more injection points to make
such a test reliable, which we now have.

--
Peter Geoghegan