Re: Use instr_time for pg_stat_database block read/write time counters

From: Lukas Fittl <lukas(at)fittl(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: ahmed <gouda0x(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Melanie Plageman <melanieplageman(at)gmail(dot)com>, Bernd Reiß <bd_reiss(at)gmx(dot)at>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Use instr_time for pg_stat_database block read/write time counters
Date: 2026-10-16 17:53:41
Message-ID: CAP53Pkz-o8goa6jR_bP_+jJtLHy3hf=OHBqCid=3ttjrT+oLjA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Oct 2, 2026 at 11:40 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > We tried to implement this and directly use `pgBufferUsage` in
> > `pgstat_update_dbstats()` but we came across some issues that caused
> > pg_stat_database to overcount, specifically when having parallel workers in
> > the plan, each worker will acuumulate its own statistics in its own
> > `pgBufferUsage` and will report them at the end and update the db stats
> > entry, but the problem is after that the parent backend will also acuumulate
> > all the statistics from its parallel workers and add them to its
> > `pgBufferUsage` and update the db stats entry again hence causing each
> > worker's I/O write/read times to be counted twice.
>
> I really really dislike a lot of how the parallel worker stats stuff is done
> :(. The whole idea of just mixing it into the client backends stats is just
> bonkers IMO. We do it inconsistently, sometimes we then have to back out the
> worker numbers again during explain. Just a bad idea overall.

Mhm. The way that parallel worker instrumentation is done is very
complex and brittle. It's easy to get any changes there wrong,
introducing double counting/etc.

> I think Lukas had some WIP patches to change some of that? He might also have
> some opinions independent of parallel workers, as he has pending work to
> refactor things so that the explain stuff is not diff based anymore (the
> diffing makes it rather expensive atm).
>
> So maybe this is all moot - I'll let him chime in on that.

Yeah, the stack-based instrumentation patch works some related code,
but notably I didn't touch that call to pgStatBlock(Write|Read)Time in
analyze.c/vacuumlazy.c on prior iterations.

Looking at it again, if I read this right, if we were to sum up local
and shared I/O timings (pgstat_count_io_op_time already ignores
IOOBJECT_WAL for pgstat_count_buffer_...), we should indeed get the
same measurements via the instrumentation buffer usage data, avoiding
the need to do separate diffing. I have a draft of that locally and
hoping to share this in the next few days on the existing thread for
stack-based instrumentation.

Whilst looking at this last week I also noticed a related bug with
parallel VACUUM not reporting I/O times correctly, but coincidentally
that was fixed with the independently discussed
abf5fec634f6aac98e80e2ef6c3f3dbf0133dfaa - so the remaining user of
pgStatBlock(Read|Write)Time is only ANALYZE and pg_stat_database, both
of which could switch to the stack-based approach.

Thanks,
Lukas

--
Lukas Fittl

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Lucas Jeffrey 2026-10-16 17:56:15 Re: [PATCH] Fix segmentation fault caused by reentrancy in RI_Fkey_cascade_del (ri_triggers.c)
Previous Message Masahiko Sawada 2026-10-16 17:45:43 Re: Parallel vacuum: workers together with leader could overrun cost budget