| From: | Tomas Vondra <tomas(at)vondra(dot)me> |
|---|---|
| To: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
| Cc: | Lukas Fittl <lukas(at)fittl(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: EXPLAIN: showing ReadStream / prefetch stats |
| Date: | 2026-04-07 16:13:18 |
| Message-ID: | 031ed83b-f474-4227-b781-af779a782c08@vondra.me |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 4/7/26 17:27, Melanie Plageman wrote:
> On Tue, Apr 7, 2026 at 8:36 AM Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>>
>> 3) Used (es_instrument & INSTRUMENT_IO) more consistently. A couple
>> places in the executor still checked just es_instrument, and so would
>> collect stats even if not needed. Be consistent.
>
> I would argue for only allocating the shared instrumentation if they
> will be displayed, but I don't feel that strongly about it.
>
OK
> I took another look at just 0002 just to double-check the read stream
> counters. It seems fine.
> Perhaps we should free the TableScanInstrumentation in heap_endscan().
> It isn't that important of a leak, but the other things palloc'd in
> heap_beginscan() are freed.
>
Good point, will fix.
> I also wondered if it is clear without a comment why we don't count a
> wait when READ_BUFFERS_SYNCHRONOUSLY sets needed_wait (which we added
> to make sure distance ramps up). I think it's fine; I'm just musing.
>
I believe this is actually a bug, the read_stream_count_wait() should be
after the block checking for READ_BUFFERS_SYNCHRONOUSLY. I recall Andres
mentioned this in one of this reviews, but I either forgot to address
that, or it got lost then transferring this between this thread and the
index prefetching one.
Thanks!
--
Tomas Vondra
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nazir Bilal Yavuz | 2026-04-07 16:18:49 | Upload only the failed tests logs to the Postgres CI (Cirrus CI) |
| Previous Message | Melanie Plageman | 2026-04-07 16:07:18 | Re: EXPLAIN: showing ReadStream / prefetch stats |