pgsql: Avoid nbtree parallel scan currPos confusion.

From: Peter Geoghegan <pg(at)bowt(dot)ie>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Avoid nbtree parallel scan currPos confusion.
Date: 2024-11-08 18:10:43
Message-ID: E1t9TRT-000rA9-0F@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Avoid nbtree parallel scan currPos confusion.

Commit 1bd4bc85, which refactored nbtree sibling link traversal, made
_bt_parallel_seize reset the scan's currPos so that things were
consistent with the state of a serial backend moving between pages.
This overlooked the fact that _bt_readnextpage relied on the existing
currPos state to decide when to end the scan -- even though it came from
before the scan was seized. As a result of all this, parallel nbtree
scans could needlessly behave like full index scans.

To fix, teach _bt_readnextpage to explicitly allow the use of an already
read page's so->currPos when deciding whether to end the scan -- even
during parallel index scans (allow it consistently now). This requires
moving _bt_readnextpage's seizure of the scan to earlier in its loop.
That way _bt_readnextpage either deals with the true so->currPos state,
or an initialized-by-_bt_parallel_seize currPos state set from when the
scan was seized. Now _bt_steppage (the most important _bt_readnextpage
caller) takes the same uniform approach to setting up its call using
details taken from so->currPos -- regardless of whether the scan happens
to be parallel or serial.

The new loop structure in _bt_readnextpage is prone to getting confused
by P_NONE blknos set when the rightmost or leftmost page was reached.
We could avoid that by adding an explicit check, but that would be ugly.
Avoid this problem by teaching _bt_parallel_seize to end the parallel
scan instead of returning a P_NONE next block/blkno. Doing things this
way was arguably a missed opportunity for commit 1bd4bc85. It allows us
to remove a similar "blkno == P_NONE" check from _bt_first.

Oversight in commit 1bd4bc85, which refactored sibling link traversal
(as part of optimizing nbtree backward scan locking).

Author: Peter Geoghegan <pg(at)bowt(dot)ie>
Reported-By: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
Diagnosed-By: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
Reviewed-By: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
Discussion: https://postgr.es/m/f8efb9c0f8d1a71b44fd7f8e42e49c25@oss.nttdata.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/b5ee4e52026b03b19ba47cde27bed9cf740576cc

Modified Files
--------------
src/backend/access/nbtree/nbtree.c | 32 ++++++---
src/backend/access/nbtree/nbtsearch.c | 121 +++++++++++++++-------------------
src/backend/access/nbtree/nbtutils.c | 2 +
3 files changed, 79 insertions(+), 76 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2024-11-08 18:42:18 pgsql: Improve fix for not entering parallel mode when holding interrup
Previous Message Tom Lane 2024-11-08 16:03:00 Re: pgsql: Fix our Windows stat() emulation to handle file sizes > 4GB.