MIME-Version: 1.0
References: <CAGCUe0Lwk3C0qdkBa+OLpYc7yXwW=pbaz8Sju4xMXEQAmyp+5g@mail.gmail.com>
 <da1decf3-b9b2-43aa-b885-03e76de27b72@vondra.me>
In-Reply-To: <da1decf3-b9b2-43aa-b885-03e76de27b72@vondra.me>
From: Peter Geoghegan <pg@bowt.ie>
Date: Wed, 29 Apr 2026 12:08:12 -0400
Message-ID: <CAH2-Wz=Z3+rgpAU=SxQCZFGjXvJU6f07zci4HYfuqrVNOT8dHQ@mail.gmail.com>
Subject: Re: Fix size estimation for parallel B-Tree scans with skip arrays
To: Tomas Vondra <tomas@vondra.me>
Cc: Siddharth Kothari <sidkot@google.com>, pgsql-bugs@lists.postgresql.org, 
	Vaibhav Jain <jainva@google.com>, Madhukar <madhukarprasad@google.com>, 
	Xun Cheng <xuncheng@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAH2-Wz%3DZ3%2BrgpAU%3DSxQCZFGjXvJU6f07zci4HYfuqrVNOT8dHQ%40mail.gmail.com>
Precedence: bulk

On Wed, Apr 29, 2026 at 11:42=E2=80=AFAM Tomas Vondra <tomas@vondra.me> wro=
te:
> Thanks for the report. I'm able to reproduce the crash using your
> reproducer script. At first I've been confused why you need a BRIN index
> when this report is about btree, but I suppose that's just to force a
> parallel index scan. There are easier ways to do that, though, e.g. by
> increasing cpu_tuple_cost. Then it's enough to query just the one rel.

I pushed the fix a short while ago, but didn't include the tests. I
don't think that the added test cycles would have paid for themselves.

> How did you discover this issue? I don't think anyone else reported such
> crashes, so presumably it's not quite common.

There were many ways that this issue could accidentally fail to fail.
For example, if even one of the skip arrays happened to be on a text
column, there'd almost certainly have been no crash. In general we're
very conservative about the space we request. We have to be, because
the request is made only once, long before we really know what nbtree
preprocessing will do/how many arrays it'll output.

> It does fix it for me, but I don't know enough about the skip scan
> internals to say if the fix is right.

_bt_parallel_serialize_arrays assumes that btscan->btps_arrElems[] has
so->numArrayKeys[]-many elements -- with and without the fix. The
easiest way to see that the fix is correct is by noticing that
_bt_parallel_serialize_arrays expects a certain layout in shared
memory that btestimateparallelscan wasn't fully handling.

When btestimateparallelscan estimated the amount of shared memory that
the scan will require, it previously neglected to account for how skip
arrays could contribute to the size of so->numArrayKeys[]. With
Siddharth's fix in place, we conservatively assume that preprocessing
will add the maximum possible number of skip arrays/use the largest
possible so->numArrayKeys[]/so->numArrayKeys when we determine
btscan->btps_arrElems[] space overhead. Making
_bt_parallel_serialize_arrays agree with btestimateparallelscan.

> Is there something we could do to deal with this class of bugs (buffer
> overflow in shared memory)? For buffers in private memory we have tools
> like valgrind and sentinels to make these issues more obvious, but for
> shared memory that's not the case ... :-(

I'm not sure that Valgrind style instrumentation would have actually
caught this issue.

As I said, our conservative approach could mask the issue in many
ways. Plus the test case involved an index with the maximum 32 index
columns, and an input scan key on the very last index column, which is
obviously very atypical.

--=20
Peter Geoghegan