pgsql: Avoid nbtree parallel scan currPos confusion. - Mailing list pgsql-committers

From Peter Geoghegan
Subject pgsql: Avoid nbtree parallel scan currPos confusion.
Date
Msg-id E1t9TRT-000rA9-0F@gemulon.postgresql.org
Whole thread Raw
List 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@bowt.ie>
Reported-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Diagnosed-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Reviewed-By: Masahiro Ikeda <ikedamsh@oss.nttdata.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(-)


pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: Re: pgsql: Fix our Windows stat() emulation to handle file sizes > 4GB.
Next
From: Tom Lane
Date:
Subject: pgsql: Improve fix for not entering parallel mode when holding interrup