Re: nbtree backwards scan test coverage - Mailing list pgsql-hackers
| From | Andrey Borodin |
|---|---|
| Subject | Re: nbtree backwards scan test coverage |
| Date | |
| Msg-id | 82D5177B-70B2-4611-BE8C-4B8223E205A9@yandex-team.ru Whole thread |
| In response to | nbtree backwards scan test coverage (Peter Geoghegan <pg@bowt.ie>) |
| Responses |
Re: nbtree backwards scan test coverage
|
| List | pgsql-hackers |
> 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]: > 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
pgsql-hackers by date: