On Thu, Jun 13, 2024 at 04:59:50PM +0900, Michael Paquier wrote: > - 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?
These two questions have been itching me in terms of how it would work for extension developers, after noticing that custom RMGRs are used more than I thought: https://wiki.postgresql.org/wiki/CustomWALResourceManagers The result is proving to be nicer, shorter by 300 lines in total and much simpler when it comes to think about the way stats are flushed because it is possible to achieve the same result as the first patch set without manipulating any of the code paths doing the read and write of the pgstats file. In terms of implementation, pgstat.c's KindInfo data is divided into two parts, for efficiency: - The exiting in-core stats with designated initializers, renamed as built-in stats kinds. - The custom stats kinds are saved in TopMemoryContext, and can only be registered with shared_preload_libraries. The patch reserves a set of 128 harcoded slots for all the custom kinds making the lookups for the KindInfos quite cheap. Upon registration, a custom stats kind needs to assign a unique ID, with uniqueness on the names and IDs checked at registration. The backend code does ID -> information lookups in the hotter paths, meaning that the code only checks if an ID is built-in or custom, then redirects to the correct array where the information is stored. There is one code path that does a name -> information lookup for the undocumented SQL function pg_stat_have_stats() used in the tests, which is a bit less efficient now, but that does not strike me as an issue. modules/injection_points/ works as previously as a template to show how to use these APIs, with tests for the whole. With that in mind, the patch set is more pleasant to the eye, and the attached v2 consists of: - 0001 and 0002 are some cleanups, same as previously to prepare for the backend-side APIs. - 0003 adds the backend support to plug-in custom stats. - 0004 includes documentation. - 0005 is an example of how to use them, with a TAP test providing coverage. Note that the patch I've proposed to make stats persistent at checkpoint so as we don't discard everything after a crash is able to work with the custom stats proposed on this thread: https://commitfest.postgresql.org/48/5047/ -- Michael
From 4c065f73cb744d1735c01e6f276d658853810f2e Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 13 Jun 2024 13:25:05 +0900
Subject: [PATCH v2 1/5] Switch PgStat_Kind from enum to uint32
A follow-up patch is planned to make this counter extensible, and
keeping a trace of the kind behind a type is useful in the internal
routines used by pgstats. While on it, switch pgstat_is_kind_valid() to
use PgStat_Kind, to be more consistent with its callers.
---
src/include/pgstat.h | 35 ++++++++++++++---------------
src/backend/utils/activity/pgstat.c | 6 ++---
2 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 2136239710..2d30fadaf1 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -32,26 +32,25 @@
#define PG_STAT_TMP_DIR "pg_stat_tmp"
/* The types of statistics entries */
-typedef enum PgStat_Kind
-{
- /* use 0 for INVALID, to catch zero-initialized data */
- PGSTAT_KIND_INVALID = 0,
+#define PgStat_Kind uint32
- /* stats for variable-numbered objects */
- PGSTAT_KIND_DATABASE, /* database-wide statistics */
- PGSTAT_KIND_RELATION, /* per-table statistics */
- PGSTAT_KIND_FUNCTION, /* per-function statistics */
- PGSTAT_KIND_REPLSLOT, /* per-slot statistics */
- PGSTAT_KIND_SUBSCRIPTION, /* per-subscription statistics */
+/* use 0 for INVALID, to catch zero-initialized data */
+#define PGSTAT_KIND_INVALID 0
- /* stats for fixed-numbered objects */
- PGSTAT_KIND_ARCHIVER,
- PGSTAT_KIND_BGWRITER,
- PGSTAT_KIND_CHECKPOINTER,
- PGSTAT_KIND_IO,
- PGSTAT_KIND_SLRU,
- PGSTAT_KIND_WAL,
-} PgStat_Kind;
+/* stats for variable-numbered objects */
+#define PGSTAT_KIND_DATABASE 1 /* database-wide statistics */
+#define PGSTAT_KIND_RELATION 2 /* per-table statistics */
+#define PGSTAT_KIND_FUNCTION 3 /* per-function statistics */
+#define PGSTAT_KIND_REPLSLOT 4 /* per-slot statistics */
+#define PGSTAT_KIND_SUBSCRIPTION 5 /* per-subscription statistics */
+
+/* stats for fixed-numbered objects */
+#define PGSTAT_KIND_ARCHIVER 6
+#define PGSTAT_KIND_BGWRITER 7
+#define PGSTAT_KIND_CHECKPOINTER 8
+#define PGSTAT_KIND_IO 9
+#define PGSTAT_KIND_SLRU 10
+#define PGSTAT_KIND_WAL 11
#define PGSTAT_KIND_FIRST_VALID PGSTAT_KIND_DATABASE
#define PGSTAT_KIND_LAST PGSTAT_KIND_WAL
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index dcc2ad8d95..d558cc1414 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -173,7 +173,7 @@ static void pgstat_prep_snapshot(void);
static void pgstat_build_snapshot(void);
static void pgstat_build_snapshot_fixed(PgStat_Kind kind);
-static inline bool pgstat_is_kind_valid(int ikind);
+static inline bool pgstat_is_kind_valid(PgStat_Kind kind);
/* ----------
@@ -1254,9 +1254,9 @@ pgstat_get_kind_from_str(char *kind_str)
}
static inline bool
-pgstat_is_kind_valid(int ikind)
+pgstat_is_kind_valid(PgStat_Kind kind)
{
- return ikind >= PGSTAT_KIND_FIRST_VALID && ikind <= PGSTAT_KIND_LAST;
+ return kind >= PGSTAT_KIND_FIRST_VALID && kind <= PGSTAT_KIND_LAST;
}
const PgStat_KindInfo *
--
2.45.1
From 87d3c9a0f8197bcc962b8b74f85d5710a3a0dba2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 13 Jun 2024 13:25:48 +0900
Subject: [PATCH v2 2/5] Replace hardcoded identifiers in pgstats file by
variables
This changes three variable types:
- N for named entries.
- S for entries identified by a hash.
- E for end-of-file
---
src/backend/utils/activity/pgstat.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index d558cc1414..f03fee7cd5 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -127,6 +127,13 @@
#define PGSTAT_SNAPSHOT_HASH_SIZE 512
+/* ---------
+ * Identifiers in stats file.
+ * ---------
+ */
+#define PGSTAT_FILE_END 'E' /* end of file */
+#define PGSTAT_FILE_NAME 'N' /* stats entry identified by name */
+#define PGSTAT_FILE_SYSTEM 'S' /* stats entry identified by PgStat_HashKey */
/* hash table for statistics snapshots entry */
typedef struct PgStat_SnapshotEntry
@@ -1408,7 +1415,7 @@ pgstat_write_statsfile(void)
if (!kind_info->to_serialized_name)
{
/* normal stats entry, identified by PgStat_HashKey */
- fputc('S', fpout);
+ fputc(PGSTAT_FILE_SYSTEM, fpout);
write_chunk_s(fpout, &ps->key);
}
else
@@ -1418,7 +1425,7 @@ pgstat_write_statsfile(void)
kind_info->to_serialized_name(&ps->key, shstats, &name);
- fputc('N', fpout);
+ fputc(PGSTAT_FILE_NAME, fpout);
write_chunk_s(fpout, &ps->key.kind);
write_chunk_s(fpout, &name);
}
@@ -1435,7 +1442,7 @@ pgstat_write_statsfile(void)
* pgstat.stat with it. The ferror() check replaces testing for error
* after each individual fputc or fwrite (in write_chunk()) above.
*/
- fputc('E', fpout);
+ fputc(PGSTAT_FILE_END, fpout);
if (ferror(fpout))
{
@@ -1572,8 +1579,8 @@ pgstat_read_statsfile(void)
switch (t)
{
- case 'S':
- case 'N':
+ case PGSTAT_FILE_SYSTEM:
+ case PGSTAT_FILE_NAME:
{
PgStat_HashKey key;
PgStatShared_HashEntry *p;
@@ -1581,7 +1588,7 @@ pgstat_read_statsfile(void)
CHECK_FOR_INTERRUPTS();
- if (t == 'S')
+ if (t == PGSTAT_FILE_SYSTEM)
{
/* normal stats entry, identified by PgStat_HashKey */
if (!read_chunk_s(fpin, &key))
@@ -1647,8 +1654,8 @@ pgstat_read_statsfile(void)
break;
}
- case 'E':
- /* check that 'E' actually signals end of file */
+ case PGSTAT_FILE_END:
+ /* check that PGSTAT_FILE_END actually signals end of file */
if (fgetc(fpin) != EOF)
goto error;
--
2.45.1
From 10ecf14b8bc51d870e2aaccb57386c3fda91615e Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Thu, 20 Jun 2024 09:19:54 +0900 Subject: [PATCH v2 3/5] Introduce pluggable APIs for Cumulative Statistics This commit adds support in the backend for $subject, allowing out-of-core extensions to add their own custom statistics kinds. The stats kinds are divided into two parts for efficiency: - The built-in stats kinds, with designated initializers. - The custom kinds, able to use a range of IDs (128 slots available as of this patch), with information saved in TopMemoryContext. Custom cumulative statistics can only be loaded with shared_preload_libraries at startup, and must allocate a unique ID shared across all the PostgreSQL extension ecosystem with the following wiki page: https://wiki.postgresql.org/wiki/CustomCumulativeStats --- src/include/pgstat.h | 32 +++++- src/include/utils/pgstat_internal.h | 4 +- src/backend/utils/activity/pgstat.c | 171 +++++++++++++++++++++++++--- src/backend/utils/adt/pgstatfuncs.c | 2 +- 4 files changed, 189 insertions(+), 20 deletions(-) diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 2d30fadaf1..59abd422c3 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -52,9 +52,35 @@ #define PGSTAT_KIND_SLRU 10 #define PGSTAT_KIND_WAL 11 -#define PGSTAT_KIND_FIRST_VALID PGSTAT_KIND_DATABASE -#define PGSTAT_KIND_LAST PGSTAT_KIND_WAL -#define PGSTAT_NUM_KINDS (PGSTAT_KIND_LAST + 1) +#define PGSTAT_KIND_MIN_BUILTIN PGSTAT_KIND_DATABASE +#define PGSTAT_KIND_MAX_BUILTIN PGSTAT_KIND_WAL + +/* Custom stats kinds */ +#define PGSTAT_KIND_MAX 256 /* Maximum ID allowed */ + +/* Range of IDs allowed for custom stats kinds */ +#define PGSTAT_KIND_CUSTOM_MIN 128 +#define PGSTAT_KIND_CUSTOM_MAX PGSTAT_KIND_MAX +#define PGSTAT_KIND_CUSTOM_SIZE (PGSTAT_KIND_CUSTOM_MAX - PGSTAT_KIND_CUSTOM_MIN + 1) + +/* + * PgStat_Kind to use for extensions that require an ID, but are still in + * development and have not reserved their own unique kind ID yet. See: + * https://wiki.postgresql.org/wiki/CustomCumulativeStats + */ +#define PGSTAT_KIND_EXPERIMENTAL 128 + +static inline bool +pgstat_is_kind_builtin(PgStat_Kind kind) +{ + return kind > PGSTAT_KIND_INVALID && kind <= PGSTAT_KIND_MAX_BUILTIN; +} + +static inline bool +pgstat_is_kind_custom(PgStat_Kind kind) +{ + return kind >= PGSTAT_KIND_CUSTOM_MIN && kind <= PGSTAT_KIND_CUSTOM_MAX; +} /* Values for track_functions GUC variable --- order is significant! */ typedef enum TrackFunctionsLevel diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index dbbca31602..0fcee1ff3d 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -446,7 +446,7 @@ typedef struct PgStat_Snapshot /* time at which snapshot was taken */ TimestampTz snapshot_timestamp; - bool fixed_valid[PGSTAT_NUM_KINDS]; + bool fixed_valid[PGSTAT_KIND_MAX_BUILTIN + 1]; PgStat_ArchiverStats archiver; @@ -503,6 +503,8 @@ static inline void *pgstat_get_entry_data(PgStat_Kind kind, PgStatShared_Common */ extern const PgStat_KindInfo *pgstat_get_kind_info(PgStat_Kind kind); +extern void pgstat_register_kind(PgStat_Kind kind, + const PgStat_KindInfo *kind_info); #ifdef USE_ASSERT_CHECKING extern void pgstat_assert_is_up(void); diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index f03fee7cd5..5b18d78782 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -49,8 +49,16 @@ * pgStatPending list. Pending statistics updates are flushed out by * pgstat_report_stat(). * + * It is possible for out-of-core modules to define custom statistics kinds, + * that can use the same properties as any in-core stats kinds. Each custom + * stats kind needs to assign a unique ID to ensure that it does not overlap + * with other extensions. In order to reserve a unique stats kind ID, refer + * to https://wiki.postgresql.org/wiki/CustomCumulativeStats. + * * The behavior of different kinds of statistics is determined by the kind's - * entry in pgstat_kind_infos, see PgStat_KindInfo for details. + * entry in pgstat_kind_builtin_infos for all statistics kinds defined in + * core, and pgstat_kind_custom_infos for custom kinds registered at startup + * by pgstat_register_kind(). See PgStat_KindInfo for details. * * The consistency of read accesses to statistics can be configured using the * stats_fetch_consistency GUC (see config.sgml and monitoring.sgml for the @@ -249,7 +257,7 @@ static bool pgstat_is_shutdown = false; /* - * The different kinds of statistics. + * The different kinds of built-in statistics. * * If reasonably possible, handling specific to one kind of stats should go * through this abstraction, rather than making more of pgstat.c aware. @@ -261,7 +269,7 @@ static bool pgstat_is_shutdown = false; * seem to be a great way of doing that, given the split across multiple * files. */ -static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = { +static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_MAX_BUILTIN + 1] = { /* stats kinds for variable-numbered objects */ @@ -399,6 +407,15 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = { }, }; +/* + * Information about custom statistics kinds. + * + * These are saved in a different array than the built-in kinds to save + * in clarity with the initializations. + * + * Indexed by PGSTAT_KIND_CUSTOM_MIN, of size PGSTAT_KIND_CUSTOM_SIZE. + */ +static const PgStat_KindInfo **pgstat_kind_custom_infos = NULL; /* ------------------------------------------------------------ * Functions managing the state of the stats system for all backends. @@ -1051,7 +1068,7 @@ pgstat_build_snapshot(void) /* * Build snapshot of all fixed-numbered stats. */ - for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + for (int kind = PGSTAT_KIND_MIN_BUILTIN; kind <= PGSTAT_KIND_MAX_BUILTIN; kind++) { const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); @@ -1248,22 +1265,33 @@ pgstat_flush_pending_entries(bool nowait) PgStat_Kind pgstat_get_kind_from_str(char *kind_str) { - for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + for (int kind = PGSTAT_KIND_MIN_BUILTIN; kind <= PGSTAT_KIND_MAX_BUILTIN; kind++) { - if (pg_strcasecmp(kind_str, pgstat_kind_infos[kind].name) == 0) + if (pg_strcasecmp(kind_str, pgstat_kind_builtin_infos[kind].name) == 0) return kind; } + /* Check the custom set of cumulative stats */ + if (pgstat_kind_custom_infos) + { + for (int kind = 0; kind < PGSTAT_KIND_CUSTOM_SIZE; kind++) + { + if (pgstat_kind_custom_infos[kind] && + pg_strcasecmp(kind_str, pgstat_kind_custom_infos[kind]->name) == 0) + return kind + PGSTAT_KIND_CUSTOM_MIN; + } + } + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid statistics kind: \"%s\"", kind_str))); - return PGSTAT_KIND_DATABASE; /* avoid compiler warnings */ + return PGSTAT_KIND_INVALID; /* avoid compiler warnings */ } static inline bool pgstat_is_kind_valid(PgStat_Kind kind) { - return kind >= PGSTAT_KIND_FIRST_VALID && kind <= PGSTAT_KIND_LAST; + return pgstat_is_kind_builtin(kind) || pgstat_is_kind_custom(kind); } const PgStat_KindInfo * @@ -1271,7 +1299,94 @@ pgstat_get_kind_info(PgStat_Kind kind) { Assert(pgstat_is_kind_valid(kind)); - return &pgstat_kind_infos[kind]; + if (pgstat_is_kind_builtin(kind)) + return &pgstat_kind_builtin_infos[kind]; + + if (pgstat_is_kind_custom(kind)) + { + uint32 idx = kind - PGSTAT_KIND_CUSTOM_MIN; + + if (pgstat_kind_custom_infos == NULL && + pgstat_kind_custom_infos[idx] == NULL) + return NULL; + return pgstat_kind_custom_infos[idx]; + } + + Assert(false); + return NULL; /* keep compiler quiet */ +} + +/* + * Register a new stats kind. + * + * PgStat_Kinds must be globally unique across all extensions. Refer + * to https://wiki.postgresql.org/wiki/CustomCumulativeStats to reserve a + * unique ID for your extension, to avoid conflicts with other extension + * developers. During development, use PGSTAT_KIND_EXPERIMENTAL to avoid + * needlessly reserving a new ID. + */ +void +pgstat_register_kind(PgStat_Kind kind, const PgStat_KindInfo *kind_info) +{ + uint32 idx = kind - PGSTAT_KIND_CUSTOM_MIN; + + if (kind_info->name == NULL || strlen(kind_info->name) == 0) + ereport(ERROR, + (errmsg("custom cumulative statistics name is invalid"), + errhint("Provide a non-empty name for the custom cumulative statistics."))); + + if (!pgstat_is_kind_custom(kind)) + ereport(ERROR, (errmsg("custom cumulative statistics ID %u is out of range", kind), + errhint("Provide a custom cumulative statistics ID between %u and %u.", + PGSTAT_KIND_CUSTOM_MIN, PGSTAT_KIND_CUSTOM_MAX))); + + if (!process_shared_preload_libraries_in_progress) + ereport(ERROR, + (errmsg("failed to register custom cumulative statistics \"%s\" with ID %u", kind_info->name, kind), + errdetail("Custom cumulative statistics must be registered while initializing modules in \"shared_preload_libraries\"."))); + + /* + * These are not supported for now, as these point out to fixed areas of + * shared memory. + */ + if (kind_info->fixed_amount) + ereport(ERROR, + (errmsg("custom cumulative statistics property is invalid"), + errhint("Custom cumulative statistics cannot use a fixed amount of data."))); + + /* + * If pgstat_kind_custom_infos is not available yet, allocate it. + */ + if (pgstat_kind_custom_infos == NULL) + { + pgstat_kind_custom_infos = (const PgStat_KindInfo **) + MemoryContextAllocZero(TopMemoryContext, + sizeof(PgStat_KindInfo *) * PGSTAT_KIND_CUSTOM_SIZE); + } + + if (pgstat_kind_custom_infos[idx] != NULL && + pgstat_kind_custom_infos[idx]->name != NULL) + ereport(ERROR, + (errmsg("failed to register custom cumulative statistics \"%s\" with ID %u", kind_info->name, kind), + errdetail("Custom resource manager \"%s\" already registered with the same ID.", + pgstat_kind_custom_infos[idx]->name))); + + /* check for existing custom stats with the same name */ + for (int existing_kind = 0; existing_kind < PGSTAT_KIND_CUSTOM_SIZE; existing_kind++) + { + if (pgstat_kind_custom_infos[existing_kind] == NULL) + continue; + if (!pg_strcasecmp(pgstat_kind_custom_infos[existing_kind]->name, kind_info->name)) + ereport(ERROR, + (errmsg("failed to register custom cumulative statistics \"%s\" with ID %u", kind_info->name, kind), + errdetail("Existing cumulative statistics with ID %u has the same name.", existing_kind + PGSTAT_KIND_CUSTOM_MIN))); + } + + /* Register it */ + pgstat_kind_custom_infos[idx] = kind_info; + ereport(LOG, + (errmsg("registered custom resource manager \"%s\" with ID %u", + kind_info->name, kind))); } /* @@ -1349,7 +1464,7 @@ pgstat_write_statsfile(void) /* * XXX: The following could now be generalized to just iterate over - * pgstat_kind_infos instead of knowing about the different kinds of + * pgstat_kind_builtin_infos instead of knowing about the different kinds of * stats. */ @@ -1405,6 +1520,17 @@ pgstat_write_statsfile(void) if (ps->dropped) continue; + /* + * This discards data related to custom stats kinds that are unknown + * to this process. + */ + if (!pgstat_is_kind_valid(ps->key.kind)) + { + elog(WARNING, "found unknown stats entry %u/%u/%u", + ps->key.kind, ps->key.dboid, ps->key.objoid); + continue; + } + shstats = (PgStatShared_Common *) dsa_get_address(pgStatLocal.dsa, ps->body); kind_info = pgstat_get_kind_info(ps->key.kind); @@ -1529,8 +1655,8 @@ pgstat_read_statsfile(void) /* * XXX: The following could now be generalized to just iterate over - * pgstat_kind_infos instead of knowing about the different kinds of - * stats. + * pgstat_kind_builtin_infos instead of knowing about the different kinds + * of stats. */ /* @@ -1639,7 +1765,7 @@ pgstat_read_statsfile(void) if (found) { dshash_release_lock(pgStatLocal.shared_hash, p); - elog(WARNING, "found duplicate stats entry %d/%u/%u", + elog(WARNING, "found duplicate stats entry %u/%u/%u", key.kind, key.dboid, key.objoid); goto error; } @@ -1692,8 +1818,8 @@ pgstat_reset_after_failure(void) { TimestampTz ts = GetCurrentTimestamp(); - /* reset fixed-numbered stats */ - for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + /* reset fixed-numbered stats for built-in */ + for (int kind = PGSTAT_KIND_MIN_BUILTIN; kind <= PGSTAT_KIND_MAX_BUILTIN; kind++) { const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); @@ -1703,6 +1829,21 @@ pgstat_reset_after_failure(void) kind_info->reset_all_cb(ts); } + /* do the same for custom stats */ + if (pgstat_kind_custom_infos) + { + for (int kind = PGSTAT_KIND_CUSTOM_MIN; kind <= PGSTAT_KIND_CUSTOM_MAX; kind++) + { + const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); + + if (!kind_info) + continue; + if (!kind_info->fixed_amount) + continue; + kind_info->reset_all_cb(ts); + } + } + /* and drop variable-numbered ones */ pgstat_drop_all_entries(); } diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 3876339ee1..3221137123 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -1696,7 +1696,7 @@ pg_stat_reset(PG_FUNCTION_ARGS) * Reset some shared cluster-wide counters * * When adding a new reset target, ideally the name should match that in - * pgstat_kind_infos, if relevant. + * pgstat_kind_builtin_infos, if relevant. */ Datum pg_stat_reset_shared(PG_FUNCTION_ARGS) -- 2.45.1
From e74cbbdedbb1cf4945b401057e5e111116bdba22 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 13 Jun 2024 13:37:33 +0900
Subject: [PATCH v2 4/5] doc: Add section for Custom Cumulative Statistics APIs
This provides a short description of what can be done, with a pointer to
the template in the tree.
---
doc/src/sgml/xfunc.sgml | 63 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 1d0b65193e..a1c2319857 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3682,6 +3682,69 @@ extern bool InjectionPointDetach(const char *name);
</para>
</sect2>
+ <sect2 id="xfunc-addin-custom-cumulative-statistics">
+ <title>Custom Cumulative Statistics</title>
+
+ <para>
+ Is is possible for add-ins written in C-language to use custom types
+ of cumulative statistics registered in the
+ <link linkend="monitoring-stats-setup">Cumulative Statistics System</link>.
+ </para>
+
+ <para>
+ First, define a <literal>PgStat_KindInfo</literal> that includes all
+ the information related to the custom type registered. For example:
+<programlisting>
+static const PgStat_KindInfo custom_stats = {
+ .name = "custom_stats",
+ .fixed_amount = false,
+ .shared_size = sizeof(PgStatShared_Custom),
+ .shared_data_off = offsetof(PgStatShared_Custom, stats),
+ .shared_data_len = sizeof(((PgStatShared_Custom *) 0)->stats),
+ .pending_size = sizeof(PgStat_StatCustomEntry),
+}
+</programlisting>
+
+ Then, each backend that needs to use this custom type needs to register
+ it with <literal>pgstat_register_kind</literal> and a unique ID used to
+ store the entries related to this type of statistics:
+<programlisting>
+extern PgStat_Kind pgstat_add_kind(PgStat_Kind kind,
+ const PgStat_KindInfo *kind_info);
+</programlisting>
+ While developing a new extension, use
+ <literal>PGSTAT_KIND_EXPERIMENTAL</literal> for
+ <parameter>kind</parameter>. When you are ready to release the extension
+ to users, reserve a kind ID at the
+ <ulink url="https://wiki.postgresql.org/wiki/CustomCumulativeStats";>
+ Custom Cumulative Statistics</ulink> page.
+ </para>
+
+ <para>
+ The details of the API for <literal>PgStat_KindInfo</literal> can
+ be found in <filename>src/include/utils/pgstat_internal.h</filename>.
+ </para>
+
+ <para>
+ The type of statistics registered is associated with a name and a unique
+ ID shared across the server in shared memory. Each backend using a
+ custom type of statistics maintains a local cache storing the information
+ of each custom <literal>PgStat_KindInfo</literal>.
+ </para>
+
+ <para>
+ Place the extension module implementing the custom cumulative statistics
+ type in <xref linkend="guc-shared-preload-libraries"/> so that it will
+ be loaded early during <productname>PostgreSQL</productname> startup.
+ </para>
+
+ <para>
+ An example describing how to register and use custom statistics can be
+ found in
+ <filename>src/test/modules/injection_points/injection_stats.c</filename>.
+ </para>
+ </sect2>
+
<sect2 id="extend-cpp">
<title>Using C++ for Extensibility</title>
--
2.45.1
From c798aba13372aa2afdf6b9452fa28b8915e6043e Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 20 Jun 2024 09:18:35 +0900
Subject: [PATCH v2 5/5] injection_points: Add statistics for custom points
This acts as a template of what can be achieved with the pluggable
cumulative stats APIs, while being useful on its own for injection
points.
Currently, the only data gathered is the number of times an injection
point is called. This can be extended as required. All the routines
related to the stats are located in their own file, for clarity.
A TAP test is included to provide coverage for these new APIs, showing
the persistency of the data across restarts.
---
src/test/modules/injection_points/Makefile | 11 +-
.../injection_points--1.0.sql | 10 +
.../injection_points/injection_points.c | 27 +++
.../injection_points/injection_stats.c | 194 ++++++++++++++++++
.../injection_points/injection_stats.h | 23 +++
src/test/modules/injection_points/meson.build | 9 +
.../modules/injection_points/t/001_stats.pl | 50 +++++
src/tools/pgindent/typedefs.list | 2 +
8 files changed, 324 insertions(+), 2 deletions(-)
create mode 100644 src/test/modules/injection_points/injection_stats.c
create mode 100644 src/test/modules/injection_points/injection_stats.h
create mode 100644 src/test/modules/injection_points/t/001_stats.pl
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 31bd787994..9d84d9aaf3 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -1,7 +1,10 @@
# src/test/modules/injection_points/Makefile
-MODULES = injection_points
-
+MODULE_big = injection_points
+OBJS = \
+ $(WIN32RES) \
+ injection_points.o \
+ injection_stats.o
EXTENSION = injection_points
DATA = injection_points--1.0.sql
PGFILEDESC = "injection_points - facility for injection points"
@@ -9,9 +12,13 @@ PGFILEDESC = "injection_points - facility for injection points"
REGRESS = injection_points
REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
+TAP_TESTS = 1
+
# The injection points are cluster-wide, so disable installcheck
NO_INSTALLCHECK = 1
+export enable_injection_points enable_injection_points
+
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index c16a33b08d..deaf47d8ae 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -54,3 +54,13 @@ CREATE FUNCTION injection_points_detach(IN point_name TEXT)
RETURNS void
AS 'MODULE_PATHNAME', 'injection_points_detach'
LANGUAGE C STRICT PARALLEL UNSAFE;
+
+--
+-- injection_points_stats_numcalls()
+--
+-- Reports statistics, if any, related to the given injection point.
+--
+CREATE FUNCTION injection_points_stats_numcalls(IN point_name TEXT)
+RETURNS bigint
+AS 'MODULE_PATHNAME', 'injection_points_stats_numcalls'
+LANGUAGE C STRICT;
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index 5c44625d1d..46bec5059c 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -18,6 +18,7 @@
#include "postgres.h"
#include "fmgr.h"
+#include "injection_stats.h"
#include "miscadmin.h"
#include "nodes/pg_list.h"
#include "nodes/value.h"
@@ -170,6 +171,9 @@ injection_points_cleanup(int code, Datum arg)
char *name = strVal(lfirst(lc));
(void) InjectionPointDetach(name);
+
+ /* Remove stats entry */
+ pgstat_drop_inj(name);
}
}
@@ -182,6 +186,8 @@ injection_error(const char *name, const void *private_data)
if (!injection_point_allowed(condition))
return;
+ pgstat_report_inj(name);
+
elog(ERROR, "error triggered for injection point %s", name);
}
@@ -193,6 +199,8 @@ injection_notice(const char *name, const void *private_data)
if (!injection_point_allowed(condition))
return;
+ pgstat_report_inj(name);
+
elog(NOTICE, "notice triggered for injection point %s", name);
}
@@ -211,6 +219,8 @@ injection_wait(const char *name, const void *private_data)
if (!injection_point_allowed(condition))
return;
+ pgstat_report_inj(name);
+
/*
* Use the injection point name for this custom wait event. Note that
* this custom wait event name is not released, but we don't care much for
@@ -299,6 +309,10 @@ injection_points_attach(PG_FUNCTION_ARGS)
inj_list_local = lappend(inj_list_local, makeString(pstrdup(name)));
MemoryContextSwitchTo(oldctx);
}
+
+ /* Add entry for stats */
+ pgstat_create_inj(name);
+
PG_RETURN_VOID();
}
@@ -400,5 +414,18 @@ injection_points_detach(PG_FUNCTION_ARGS)
MemoryContextSwitchTo(oldctx);
}
+ /* Remove stats entry */
+ pgstat_drop_inj(name);
+
PG_RETURN_VOID();
}
+
+
+void
+_PG_init(void)
+{
+ if (!process_shared_preload_libraries_in_progress)
+ return;
+
+ pgstat_register_inj();
+}
diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c
new file mode 100644
index 0000000000..c37b0b33d3
--- /dev/null
+++ b/src/test/modules/injection_points/injection_stats.c
@@ -0,0 +1,194 @@
+/*--------------------------------------------------------------------------
+ *
+ * injection_stats.c
+ * Code for statistics of injection points.
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ * src/test/modules/injection_points/injection_stats.c
+ *
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "fmgr.h"
+
+#include "common/hashfn.h"
+#include "injection_stats.h"
+#include "pgstat.h"
+#include "utils/builtins.h"
+#include "utils/pgstat_internal.h"
+
+/* Structures for statistics of injection points */
+typedef struct PgStat_StatInjEntry
+{
+ PgStat_Counter numcalls; /* number of times point has been run */
+} PgStat_StatInjEntry;
+
+typedef struct PgStatShared_InjectionPoint
+{
+ PgStatShared_Common header;
+ PgStat_StatInjEntry stats;
+} PgStatShared_InjectionPoint;
+
+static bool injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
+
+static const PgStat_KindInfo injection_stats = {
+ .name = "injection_points",
+ .fixed_amount = false, /* Bounded by the number of points */
+
+ /* Injection points are system-wide */
+ .accessed_across_databases = true,
+
+ .shared_size = sizeof(PgStatShared_InjectionPoint),
+ .shared_data_off = offsetof(PgStatShared_InjectionPoint, stats),
+ .shared_data_len = sizeof(((PgStatShared_InjectionPoint *) 0)->stats),
+ .pending_size = sizeof(PgStat_StatInjEntry),
+ .flush_pending_cb = injection_stats_flush_cb,
+};
+
+/*
+ * Compute stats entry idx from point name with a 4-byte hash.
+ */
+#define PGSTAT_INJ_IDX(name) hash_bytes((const unsigned char *) name, strlen(name))
+
+#define PGSTAT_KIND_INJECTION PGSTAT_KIND_EXPERIMENTAL
+
+/* Track if stats are loaded */
+static bool inj_stats_loaded = false;
+
+/*
+ * Callback for stats handling
+ */
+static bool
+injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
+{
+ PgStat_StatInjEntry *localent;
+ PgStatShared_InjectionPoint *shfuncent;
+
+ localent = (PgStat_StatInjEntry *) entry_ref->pending;
+ shfuncent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats;
+
+ if (!pgstat_lock_entry(entry_ref, nowait))
+ return false;
+
+ shfuncent->stats.numcalls += localent->numcalls;
+ return true;
+}
+
+/*
+ * Support function for the SQL-callable pgstat* functions. Returns
+ * a pointer to the injection point statistics struct.
+ */
+static PgStat_StatInjEntry *
+pgstat_fetch_stat_injentry(const char *name)
+{
+ PgStat_StatInjEntry *entry = NULL;
+
+ if (!inj_stats_loaded)
+ return NULL;
+
+ /* Compile the lookup key as a hash of the point name */
+ entry = (PgStat_StatInjEntry *) pgstat_fetch_entry(PGSTAT_KIND_INJECTION,
+ InvalidOid,
+ PGSTAT_INJ_IDX(name));
+ return entry;
+}
+
+/*
+ * Workhorse to do the registration work, called in _PG_init().
+ */
+void
+pgstat_register_inj(void)
+{
+ pgstat_register_kind(PGSTAT_KIND_INJECTION, &injection_stats);
+
+ /* mark stats as loaded */
+ inj_stats_loaded = true;
+}
+
+/*
+ * Report injection point creation.
+ */
+void
+pgstat_create_inj(const char *name)
+{
+ PgStat_EntryRef *entry_ref;
+ PgStatShared_InjectionPoint *shstatent;
+
+ /* leave if disabled */
+ if (!inj_stats_loaded)
+ return;
+
+ entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_INJECTION, InvalidOid,
+ PGSTAT_INJ_IDX(name), false);
+ shstatent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats;
+
+ /* initialize shared memory data */
+ memset(&shstatent->stats, 0, sizeof(shstatent->stats));
+ pgstat_unlock_entry(entry_ref);
+}
+
+/*
+ * Report injection point drop.
+ */
+void
+pgstat_drop_inj(const char *name)
+{
+ /* leave if disabled */
+ if (!inj_stats_loaded)
+ return;
+
+ if (!pgstat_drop_entry(PGSTAT_KIND_INJECTION, InvalidOid,
+ PGSTAT_INJ_IDX(name)))
+ pgstat_request_entry_refs_gc();
+}
+
+/*
+ * Report statistics for injection point.
+ *
+ * This is simple because the set of stats to report currently is simple:
+ * track the number of times a point has been run.
+ */
+void
+pgstat_report_inj(const char *name)
+{
+ PgStat_EntryRef *entry_ref;
+ PgStatShared_InjectionPoint *shstatent;
+ PgStat_StatInjEntry *statent;
+
+ /* leave if disabled */
+ if (!inj_stats_loaded)
+ return;
+
+ entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_INJECTION, InvalidOid,
+ PGSTAT_INJ_IDX(name), false);
+
+ shstatent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats;
+ statent = &shstatent->stats;
+
+ /* Update the injection point statistics */
+ statent->numcalls++;
+
+ pgstat_unlock_entry(entry_ref);
+}
+
+/*
+ * SQL function returning the number of times an injection point
+ * has been called.
+ */
+PG_FUNCTION_INFO_V1(injection_points_stats_numcalls);
+Datum
+injection_points_stats_numcalls(PG_FUNCTION_ARGS)
+{
+ char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ PgStat_StatInjEntry *entry = pgstat_fetch_stat_injentry(name);
+
+ if (entry == NULL)
+ PG_RETURN_NULL();
+
+ PG_RETURN_INT64(entry->numcalls);
+}
diff --git a/src/test/modules/injection_points/injection_stats.h b/src/test/modules/injection_points/injection_stats.h
new file mode 100644
index 0000000000..3e99705483
--- /dev/null
+++ b/src/test/modules/injection_points/injection_stats.h
@@ -0,0 +1,23 @@
+/*--------------------------------------------------------------------------
+ *
+ * injection_stats.h
+ * Definitions for statistics of injection points.
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ * src/test/modules/injection_points/injection_stats.h
+ *
+ * -------------------------------------------------------------------------
+ */
+
+#ifndef INJECTION_STATS
+#define INJECTION_STATS
+
+extern void pgstat_register_inj(void);
+extern void pgstat_create_inj(const char *name);
+extern void pgstat_drop_inj(const char *name);
+extern void pgstat_report_inj(const char *name);
+
+#endif
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index 8e1b5b4539..612216c144 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -6,6 +6,7 @@ endif
injection_points_sources = files(
'injection_points.c',
+ 'injection_stats.c',
)
if host_system == 'windows'
@@ -37,4 +38,12 @@ tests += {
# The injection points are cluster-wide, so disable installcheck
'runningcheck': false,
},
+ 'tap': {
+ 'env': {
+ 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+ },
+ 'tests': [
+ 't/001_stats.pl',
+ ],
+ },
}
diff --git a/src/test/modules/injection_points/t/001_stats.pl b/src/test/modules/injection_points/t/001_stats.pl
new file mode 100644
index 0000000000..f8d90a3869
--- /dev/null
+++ b/src/test/modules/injection_points/t/001_stats.pl
@@ -0,0 +1,50 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+use locale;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Test persistency of statistics generated for injection points.
+if ($ENV{enable_injection_points} ne 'yes')
+{
+ plan skip_all => 'Injection points not supported by this build';
+}
+
+# Node initialization
+my $node = PostgreSQL::Test::Cluster->new('master');
+$node->init;
+$node->append_conf('postgresql.conf',
+ "shared_preload_libraries = 'injection_points'");
+$node->start;
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# This should count for two calls.
+$node->safe_psql('postgres',
+ "SELECT injection_points_attach('stats-notice', 'notice');");
+$node->safe_psql('postgres',
+ "SELECT injection_points_run('stats-notice');");
+$node->safe_psql('postgres',
+ "SELECT injection_points_run('stats-notice');");
+my $numcalls = $node->safe_psql('postgres',
+ "SELECT injection_points_stats_numcalls('stats-notice');");
+is($numcalls, '2', 'number of stats calls');
+
+# Restart the node cleanly, stats should still be around.
+$node->restart;
+$numcalls = $node->safe_psql('postgres',
+ "SELECT injection_points_stats_numcalls('stats-notice');");
+is($numcalls, '2', 'number of stats after clean restart');
+
+# On crash the stats are gone.
+$node->stop('immediate');
+$node->start;
+$numcalls = $node->safe_psql('postgres',
+ "SELECT injection_points_stats_numcalls('stats-notice');");
+is($numcalls, '', 'number of stats after clean restart');
+
+done_testing();
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 61ad417cde..481baee16d 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2116,6 +2116,7 @@ PgStatShared_Common
PgStatShared_Database
PgStatShared_Function
PgStatShared_HashEntry
+PgStatShared_InjectionPoint
PgStatShared_IO
PgStatShared_Relation
PgStatShared_ReplSlot
@@ -2147,6 +2148,7 @@ PgStat_Snapshot
PgStat_SnapshotEntry
PgStat_StatDBEntry
PgStat_StatFuncEntry
+PgStat_StatInjEntry
PgStat_StatReplSlotEntry
PgStat_StatSubEntry
PgStat_StatTabEntry
--
2.45.1
signature.asc
Description: PGP signature
