Re: Pluggable cumulative statistics
Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-07-09T05:23:03Z
Lists: pgsql-hackers
Attachments
- v1-0001-Renaming-two-fields-in-PgStat_KindInfo.patch (text/x-diff)
Hi, On Tue, Jul 09, 2024 at 10:45:05AM +0900, Michael Paquier wrote: > On Mon, Jul 08, 2024 at 07:22:32AM +0000, Bertrand Drouvot wrote: > > Yeah, what I meant to say is that one could think for example that's the > > PgStatShared_Archiver size while in fact it's the PgStat_ArchiverStats size. > > I think it's more confusing when writing the stats. Here we are manipulating > > "snapshot" and "snapshot" offsets. It was not that confusing when reading as we > > are manipulating "shmem" and "shared" offsets. > > > > As I said, the code is fully correct, that's just the wording here that sounds > > weird to me in the "snapshot" context. > > After sleeping on it, I can see your point. If we were to do the > (shared_data_len -> stats_data_len) switch, could it make sense to > rename shared_data_off to stats_data_off to have a better symmetry? > This one is the offset of the stats data in a shmem entry, so perhaps > shared_data_off is OK, but it feels a bit inconsistent as well. Agree that if we were to rename one of them then the second one should be renamed to. I gave a second thought on it, and I think that this is the "data" part that lead to the confusion (as too generic), what about? shared_data_len -> shared_stats_len shared_data_off -> shared_stats_off That looks ok to me even in the snapshot context (shared is fine after all because that's where the stats come from). Attached a patch proposal doing so. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Commits
-
injection_points: Add some fixed-numbered statistics
- f68cd847fa40 18.0 landed
-
injection_points: Add some cumulative stats for injection points
- 75534436a477 18.0 landed
-
Add helper routines to retrieve data for custom fixed-numbered pgstats
- 2eff9e678d35 18.0 landed
-
Introduce pluggable APIs for Cumulative Statistics
- 7949d9594582 18.0 landed
-
Switch PgStat_Kind from an enum to a uint32 type
- 3188a4582a8c 18.0 landed
-
Improve comment of pgstat_read_statsfile()
- 72c0b24b2ddd 18.0 landed
-
Add a new 'F' entry type for fixed-numbered stats in pgstats file
- 9e4664d950c1 18.0 landed
-
Add PgStat_KindInfo.init_shmem_cb
- 21471f18e9d9 18.0 landed
-
Use pgstat_kind_infos to write fixed shared statistics
- b68b29bc8fec 18.0 landed
-
Replace hardcoded identifiers of pgstats file by #defines
- 9fd02525793f 18.0 landed