Re: shared-memory based stats collector - v68 - Mailing list pgsql-hackers

From David G. Johnston
Subject Re: shared-memory based stats collector - v68
Date
Msg-id CAKFQuwYTn4PYPhEi1VtTD3UF673GpQVKRDrYVOTCcGOJ91A1ng@mail.gmail.com
Whole thread Raw
In response to Re: shared-memory based stats collector - v68  (Andres Freund <andres@anarazel.de>)
Responses Re: shared-memory based stats collector - v68  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
On Mon, Apr 4, 2022 at 2:54 PM Andres Freund <andres@anarazel.de> wrote:

> As the existing function only handles functions and relations why not just
> perform a specific Kind check for them?  Generalizing to assert on whether
> or not the function works on fixed or variable Kinds seems beyond its
> present state.  Or could it be used, as-is, for databases, replication
> slots, and subscriptions today, and we just haven't migrated those areas to
> use the now generalized function?

It couldn't quite be used for those, because it really only makes sense for
objects "within a database", because it wants to reset the timestamp of the
pg_stat_database row too (I don't like that behaviour as-is, but that's the
topic of another thread as you know...).

It will work for other per-database stats though, once we have them.


> Even then, unless we do expand the
> definition of the this publicly facing function is seems better to
> precisely define what it requires as an input Kind by checking for RELATION
> or FUNCTION specifically.

I don't see a benefit in adding a restriction on it that we'd just have to
lift again?

How about adding a
Assert(!pgstat_kind_info_for(kind)->accessed_across_databases)

and extending the function comment to say that it's used for per-database
stats and that it resets both the passed-in stats object as well as
pg_stat_database?


I could live with adding that, but...

Replacing the existing assert(!kind->fixed_amount) with assert(!kind->accessed_across_databases) produces the same result as the later presently implies the former.

Now I start to dislike the behavioral aspect of the attribute and would rather just name it: kind->is_cluster_scoped (or something else that is descriptive of the stat category itself, not how it is used)

Then reorganize the Kind documentation to note and emphasize these two primary descriptors:
variable, which can be cluster or database scoped
fixed, which are cluster scoped by definition (if this is true...but given this is an optimization category I'm thinking maybe it doesn't actually matter...)

+ /* cluster-scoped object stats having a variable number of entries */
+ PGSTAT_KIND_REPLSLOT = 1, /* per-slot statistics */
+ PGSTAT_KIND_SUBSCRIPTION, /* per-subscription statistics */
+ PGSTAT_KIND_DATABASE, /* database-wide statistics */ (I moved this to 3rd spot to be closer to the database-scoped options)
+
+ /* database-scoped object stats having a variable number of entries */
+ PGSTAT_KIND_RELATION, /* per-table statistics */
+ PGSTAT_KIND_FUNCTION, /* per-function statistics */
+
+ /* cluster-scoped stats having a fixed number of entries */ (maybe these should go first, the variable following?)
+ PGSTAT_KIND_ARCHIVER,
+ PGSTAT_KIND_BGWRITER,
+ PGSTAT_KIND_CHECKPOINTER,
+ PGSTAT_KIND_SLRU,
+ PGSTAT_KIND_WAL,

David J.

pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: Granting SET and ALTER SYSTE privileges for GUCs
Next
From: Thomas Munro
Date:
Subject: Re: wrong fds used for refilenodes after pg_upgrade relfilenode changes Reply-To: