Pluggable cumulative statistics - Mailing list pgsql-hackers

Hi all,

While looking at ways to make pg_stat_statements more scalable and
dynamically manageable (no more PGC_POSTMASTER for the max number of
entries), which came out as using a dshash, Andres has mentioned me
off-list (on twitter/X) that we'd better plug in it to the shmem
pgstats facility, moving the text file that holds the query strings
into memory (with size restrictions for the query strings, for
example).  This has challenges on its own (query ID is 8 bytes
incompatible with the dboid/objid hash key used by pgstats, discard of
entries when maximum).  Anyway, this won't happen if we don't do one
of these two things:
1) Move pg_stat_statements into core, adapting pgstats for its
requirements.
2) Make the shmem pgstats pluggable so as it is possible for extensions
to register their own stats kinds.

1) may have its advantages, still I am not sure if we want to do that.
And 2) is actually something that can be used for more things than
just pg_stat_statements, because people love extensions and
statistics (spoiler: I do).  The idea is simple: any extension
defining a custom stats kind would be able to rely on all the in-core
facilities we use for the existing in-core kinds:
a) Snapshotting and caching of the stats, via stats_fetch_consistency.
b) Native handling and persistency of the custom stats data.
c) Reuse stats after a crash, pointing at this comment in xlog.c:
     * TODO: With a bit of extra work we could just start with a pgstat file
     * associated with the checkpoint redo location we're starting from.
This means that we always remove the stats after a crash.  That's
something I have a patch for, not for this thread, but the idea is
that custom stats would also benefit from this property.

The implementation is based on the following ideas:

* A structure in shared memory that tracks the IDs of the custom stats
kinds with their names.  These are incremented starting from
PGSTAT_KIND_LAST.

* Processes use a local array cache that keeps tracks of all the
custom PgStat_KindInfos, indexed by (kind_id - PGSTAT_KIND_LAST).

* The kind IDs may change across restarts, meaning that any stats data
associated to a custom kind is stored with the *name* of the custom
stats kind.  Depending on the discussion happening here, I'd be open
to use the same concept as custom RMGRs, where custom kind IDs are
"reserved", fixed in time, and tracked in the Postgres wiki.  It is
cheaper to store the stats this way, as well, while managing conflicts
across extensions available in the community ecosystem.

* Custom stats can be added without shared_preload_libraries,
loading them from a shmem startup hook with shared_preload_libraries
is also possible.

* The shmem pgstats defines two types of statistics: the ones in a
dshash and what's called a "fixed" type like for archiver, WAL, etc.
pointing to areas of shared memory.  All the fixed types are linked to
structures for snapshotting and shmem tracking.  As a matter of
simplification and because I could not really see a case where I'd
want to plug in a fixed stats kind, the patch forbids this case.  This
case could be allowed, but I'd rather refactor the structures of
pgstat_internal.h so as we don't have traces of the "fixed" stats
structures in so many areas.

* Making custom stats data persistent is an interesting problem, and
there are a couple of approaches I've considered:
** Allow custom kinds to define callbacks to read and write data from
a source they'd want, like their own file through a fd.  This has the
disadvantage to remove the benefit of c) above.
** Store everything in the existing stats file, adding one type of
entry like 'S' and 'N' with a "custom" type, where the *name* of the
custom stats kind is stored instead of its ID computed from shared
memory.
A mix of both?  The patch attached has used the second approach.  If
the process reading/writing the stats does not know about the custom
stats data, the data is discarded.

* pgstat.c has a big array called pgstat_kind_infos to define all the
existing stats kinds.  Perhaps the code should be refactored to use
this new API?  That would make the code more consistent with what we
do for resource managers, for one, while moving the KindInfos into
their own file.  With that in mind, storing the kind ID in KindInfos
feels intuitive.

While thinking about a use case to show what these APIs can do, I have
decided to add statistics to the existing module injection_points
rather than implement a new test module, gathering data about them and
have tests that could use this data (like tracking the number of times
a point is taken).  This is simple enough that it can be used as a
template, as well.  There is a TAP test checking the data persistence
across restarts, so I did not mess up this part much, hopefully.

Please find attached a patch set implementing these ideas:
- 0001 switches PgStat_Kind from an enum to a uint32, for the internal
counters.
- 0002 is some cleanup for the hardcoded S, N and E in pgstat.c.
- 0003 introduces the backend-side APIs, with the shmem table counter
and the routine to give code paths a way to register their own stats
kind (see pgstat_add_kind).
- 0004 implements an example of how to use these APIs, see
injection_stats.c in src/test/modules/injection_points/.
- 0005 adds some docs.
- 0006 is an idea of how to make this custom stats data persistent.

This will hopefully spark a discussion, and I was looking for answers
regarding these questions:
- Should the pgstat_kind_infos array in pgstat.c be refactored to use
something similar to pgstat_add_kind?
- How should the persistence of the custom stats be achieved?
Callbacks to give custom stats kinds a way to write/read their data,
push everything into a single file, or support both?
- Should this do like custom RMGRs and assign to each stats kinds ID
that are set in stone rather than dynamic ones?

Thanks for reading.
--
Michael