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
