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

On Fri, Oct 16, 2026 at 10:53 AM Lukas Fittl <lukas(at)fittl(dot)com> wrote:
>
> 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.

Whelp, turns out after some more digging we have an existing bug with
cumulative WAL statistics, see [0].

> > 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.

After digging into this more, I think this is actually orthogonal to
what the stack-based instrumentation is trying to do. With the
stack-based instrumentation, and its more explicit accumulation to
parent instrumentations, there is a world where we could split out the
worker instrumentation cleanly and never add it to the global, but
introduces a lot of extra code and is hard to get right.

I think the better model is to remember in a separate global counter
the amount of worker instrumentation we imported, and then subtract
that when it comes to reporting to the cumulative statistics system.
That solution is independent of the stack-based instrumentation patch,
it really just needs the fixes from [0]. I'd suggest we discuss that
fix there, and then come back here to convert pg_stat_database to
pgBufferUsage after we've found a pattern that works for WAL usage.

Thanks,
Lukas

[0]: https://www.postgresql.org/message-id/flat/CAP53PkzL-jF-Pm%2Bev2vBDvBYqDFNhcN4bpjGbGGbBfM_gtSdag%40mail.gmail.com

--
Lukas Fittl