Re: EXPLAIN: showing ReadStream / prefetch stats

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: Tomas Vondra <tomas(at)vondra(dot)me>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Lukas Fittl <lukas(at)fittl(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: EXPLAIN: showing ReadStream / prefetch stats
Date: 2026-04-07 00:19:06
Message-ID: CAAKRu_by3fu=VsRmHjRMi9sF6Uy8mSruApyvzRm+O_=E1-ViAA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Apr 6, 2026 at 4:39 PM Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>
> On 4/6/26 18:50, Melanie Plageman wrote:
>
> > I cleaned up the first patch in the set that refactors index-only and
> > index scan to use this pattern and realized that I wasn't sure what to
> > do about the duplication between index-only and index scans for these
> > functions.
>
<--snip-->
>
> I think this is ready to go, with a tiny amount of polishing.
>
> 1) "amount of DSM" sounds a bit strange to me. The wording "amount of
> space in ..." from the other nodes seems better to me. Or maybe I'm just
> used to it, not sure.

Fixed that before pushing.

> 2) I wonder if maybe PARALLEL_KEY_SCAN_INSTRUMENT_OFFSET should be
> placed in plannodes.h, because that's where Plan->plan_node_id is
> defined. instrument_node.h works too, but the places accessing the
> plan_node_id already have to include plannodes.h.

I don't think it really belongs there. It is very specific to
execution. We use the plan node id as the key for convenience, but it
isn't the purpose of plan_node_id.
PARALLEL_KEY_SCAN_INSTRUMENT_OFFSET's only purpose is to be used
during execution for instrumentation, so it feels like it belongs in
node_instrument.h. And we don't need a separate include for it either.
It goes with the other stuff being defined in instrument_node.h (like
SharedIndexScanInstrumentation) and being used by those callers. I
admit the comment above it is a bit odd, but I think it is ultimately
okay.

> We need to decide whether to push this into PG19. This was primarily
> motivated by the index prefetching work, but we now know that won't
> happen in PG19 :-( But the instrumentation is useful even for the three
> scans using read streams, so I think it's a meaningful improvement.

I think it is a meaningful improvement too. I think we should do it.

> If you think you can get this pushed, I'll do my best to finalize the
> instrumentation and SeqScan/TidRangeScan parts.

I've pushed the first patch for index/index-only scans. Attached is
the BHS fix that uses the new pattern. It needs at least one review
before pushing. While I was polishing it, I realized I neglected to
use add_size()/mul_size() in the index-only/index scan patches. So,
0002 is just a fix commit to do that. Feel free to push these if you
think they're ready. Otherwise, I'll do so pending your review in my
morning.

- Melanie

Attachment Content-Type Size
v11-0001-Fix-BitmapHeapScan-non-parallel-aware-EXPLAIN-AN.patch text/x-patch 8.8 KB
v11-0002-Use-add_size-mul_size-for-index-instrumentation-.patch text/x-patch 3.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2026-04-07 00:19:56 Re: Asynchronous MergeAppend
Previous Message Heikki Linnakangas 2026-04-07 00:18:05 Re: Don't use the deprecated and insecure PQcancel in our frontend tools anymore