Re: Get rid of pgstat_count_backend_io_op*() functions

Hi,

On Tue, Sep 02, 2025 at 08:19:22AM +0900, Michael Paquier wrote:
> On Mon, Sep 01, 2025 at 02:07:27PM +0000, Bertrand Drouvot wrote:
> > Instead, it now copies the IO pending stats to the backend IO pending stats when
> > the pending IO stats are flushed. This behaves the same way as for some relation
> > and database stats (see pgstat_relation_flush_cb()).
> >
> > It's done that way to avoid incrementing the "same" counters twice as it produces
> > increased overhead in profiles (reported by Andres in [1]).
>
> So, is the complaint about the addition of the extra incrementations
> for backend counters on top of the existing IO counters in v17,
> leading to more instructions generated, the addition of the functions,
> or both things at the same time? Do you have an example of profile
> and/or workload where this actually matters? I am aware that you are
> doing crazy things in this area, so..

I did not observe such noticeable overhead myself, so I guess that those questions
are for Andres. That said, I think it makes fully sense to avoid incrementing
the "same" counters twice (and specially in hot path).

> > Please note that per-backend statistics have to be last when we define the
> > PGSTAT_KIND_% values in pgstat_kind.h, as some of their pending stats are
> > populated while other stats kinds are flushing.
>
> +/*
> + * per-backend statistics has to be last, as some of their pending stats are
> + * populated while other stats kinds are flushing.
> + */
> +#define PGSTAT_KIND_BACKEND 12 /* per-backend statistics */
>
> I don't think that adding ordering dependencies for the stats kinds is
> a good concept in general.

I agree that it has to be simple. I don't think that just ensuring that the
backends are last introduces much complexity.

> What makes you think that we could not
> have a custom pgstats kind willing to increment backend counters as
> well, say if we have a table AM with its own stats, for example?

Nothing, but the issue would be the same if we keep the backends
at their current position. That's right that it's "only" solving the in-core
ordering.

> Removing both the function calls and the extra counter incrementations
> means to do the same thing as the WAL stats, where we have one
> structure in charge of incrementing the counters, then the data diffs
> are fed when flushing the entries in all the stats kinds. So this
> would imply the introduction of an equivalent to WalUsage, but applied
> to the IO stats, like a IOUsage, or something like that.

Yeah, that sounds more complicated at first but I think that would solve
the "ordering" issue mentioned above.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com