Re: Pluggable cumulative statistics

On Mon, Jul 08, 2024 at 06:39:56AM +0000, Bertrand Drouvot wrote:
> +       for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++)
> +       {
> +               char       *ptr;
> +               const PgStat_KindInfo *info = pgstat_get_kind_info(kind);
>
> I wonder if we could avoid going through stats that are not fixed ones. What about
> doing something like?
> Same would apply for the read part added in 9004abf6206e.

This becomes more relevant when the custom stats are added, as this
performs a scan across the full range of IDs supported.  So this
choice is here for consistency, and to ease the pluggability.

> 2 ===
>
> +               pgstat_build_snapshot_fixed(kind);
> +               ptr = ((char *) &pgStatLocal.snapshot) + info->snapshot_ctl_off;
> +               write_chunk(fpout, ptr, info->shared_data_len);
>
> I think that using "shared_data_len" is confusing here (was not the case in the
> context of 9004abf6206e). I mean it is perfectly correct but the wording "shared"
> looks weird to me when being used here. What it really is, is the size of the
> stats. What about renaming shared_data_len with stats_data_len?

It is the stats data associated to a shared entry.  I think that's OK,
but perhaps I'm just used to it as I've been staring at this area for
days.
--
Michael