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
signature.asc
Description: PGP signature
