Re: EXPLAIN: showing ReadStream / prefetch stats

Melanie Plageman <melanieplageman@gmail.com>

From: Melanie Plageman <melanieplageman@gmail.com>
To: Tomas Vondra <tomas@vondra.me>
Cc: Andres Freund <andres@anarazel.de>, Lukas Fittl <lukas@fittl.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-04-07T00:19:06Z
Lists: pgsql-hackers

Attachments

On Mon, Apr 6, 2026 at 4:39 PM Tomas Vondra <tomas@vondra.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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. bufmgr: Return whether WaitReadBuffers() needed to wait