Re: per backend I/O statistics

Hi,

On Wed, Nov 06, 2024 at 08:39:07AM +0900, Michael Paquier wrote:
> On Tue, Nov 05, 2024 at 05:37:15PM +0000, Bertrand Drouvot wrote:
> > I'm starting working on option 2, I think it will be easier to discuss with
> > a patch proposal to look at.
> > 
> > If in the meantime, one strongly disagree with option 2 (means implement a brand
> > new PGSTAT_KIND_BACKEND and keep PGSTAT_KIND_IO), please let me know.
> 
> Sorry for the late reply, catching up a bit.

No problem at all, thanks for looking at it!

> As you are quoting in [1], you do not expect the backend-io stats and
> the more global pg_stat_io to achieve the same level of consistency as
> the backend stats would be gone at restart, and wiped out when a
> backend shuts down.

Yes.

> So, splitting them with a different stats kind
> feels more natural because it would be possible to control how each
> stat kind behaves depending on the code shutdown and reset paths
> within their own callbacks rather than making the callbacks of
> PGSTAT_KIND_IO more complex than they already are.

Yeah, thanks for sharing your thoughts.

> And pg_stat_io is
> a fixed-numbered stats kind because of the way it aggregates its stats
> with a number states defined at compile-time.
> 
> Is the structure you have in mind different than PgStat_BktypeIO?

Very close.

> Perhaps a split is better anyway with that in mind.

The in-progress patch (not shared yet) is using the following:

"
typedef struct PgStat_Backend
{
       TimestampTz stat_reset_timestamp;
       BackendType bktype;
       PgStat_BktypeIO stats;
} PgStat_Backend;
"

The bktype is used to be able to filter the stats correctly when we display them.

> The amount of memory required to store the snapshots of backend-IO
> does not worry me much, TBH, but you are worried about a high turnover
> of connections that could cause a lot of bloat in the backend-IO
> snapshots because of the persistency that these stats would have,
> right?

Not only a high turnover but also a high number of entries created in the hash.

Furthermore I don't see any use case of relying on stats_fetch_consistency
while querying other backend's stats.

> If possible, supporting snapshots would be
> more consistent with the other stats.

I have I mind to support the snapshots _only_ when querying our own stats. I can
measure the memory impact if we use them also when querying other backends stats
too (though I don't see a use case).

> Just to be clear, I am not in favor of making PgStat_HashKey larger
> than it already is.

That's not needed, the patch I'm working on stores the proc number in the
objid field of the key.

Regards,

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