On Wed, Jul 10, 2024 at 09:00:31AM +0000, Bertrand Drouvot wrote: > On Wed, Jul 10, 2024 at 08:28:56AM +0000, Bertrand Drouvot wrote: >> v5-0001 LGTM.
Thanks. I've applied this refactoring piece. > /* > * We found an existing statistics file. Read it and put all the hash > * table entries into place. > */ Indeed. Reworded that slightly and applied it as well. So we are down to the remaining parts of the patch, and this is going to need a consensus about a few things because this impacts the developer experience when implementing one's own custom stats: - Are folks OK with the point of fixing the kind IDs in time like RMGRs with a control in the wiki? Or should a more artistic approach be used like what I am mentioning at the bottom of [1]. The patch allows a range of IDs to be used, to make the access to the stats faster even if some area of memory may not be used. - The fixed-numbered custom stats kinds are stored in an array in PgStat_Snapshot and PgStat_ShmemControl, so as we have something consistent with the built-in kinds. This makes the tracking of the validity of the data in the snapshots split into parts of the structure for builtin and custom kinds. Perhaps there are better ideas than that? The built-in fixed-numbered kinds have no redirection. - The handling of both built-in and custom kinds touches some areas of pgstat.c and pgstat_shmem.c, which is the minimal I could come up with. Attached is a rebased patch set with the remaining pieces. [1]: https://www.postgresql.org/message-id/ZoshTO9K7O7Z1wrX%40paquier.xyz -- Michael
From 2cb5aa81d0d93a910934105756d893d2c0586ea2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 8 Jul 2024 12:28:26 +0900
Subject: [PATCH v6 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 6b99bb8aad..cc97274708 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 c0ec9e8195..805712d414 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -182,7 +182,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);
/* ----------
@@ -1298,9 +1298,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.2
From 0869c45608b7acb73afa4afc7bf2e481c239d705 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Mon, 8 Jul 2024 14:03:17 +0900 Subject: [PATCH v6 2/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 This is able to support fixed-numbered (like WAL, archiver, bgwriter) and variable-numbered stats kinds. --- src/include/pgstat.h | 35 +++- src/include/utils/pgstat_internal.h | 22 ++- src/backend/utils/activity/pgstat.c | 231 +++++++++++++++++++--- src/backend/utils/activity/pgstat_shmem.c | 31 ++- src/backend/utils/adt/pgstatfuncs.c | 2 +- 5 files changed, 287 insertions(+), 34 deletions(-) diff --git a/src/include/pgstat.h b/src/include/pgstat.h index cc97274708..21c3705ce4 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -34,6 +34,10 @@ /* The types of statistics entries */ #define PgStat_Kind uint32 +/* Range of IDs allowed, for built-in and custom kinds */ +#define PGSTAT_KIND_MIN 1 /* Minimum ID allowed */ +#define PGSTAT_KIND_MAX 256 /* Maximum ID allowed */ + /* use 0 for INVALID, to catch zero-initialized data */ #define PGSTAT_KIND_INVALID 0 @@ -52,9 +56,34 @@ #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 */ + +/* 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 778f625ca1..39f63362a3 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -195,7 +195,8 @@ typedef struct PgStat_KindInfo /* * The size of an entry in the shared stats hash table (pointed to by - * PgStatShared_HashEntry->body). + * PgStatShared_HashEntry->body). For fixed-numbered statistics, this is + * the size of an entry in PgStat_ShmemControl->custom_data. */ uint32 shared_size; @@ -446,6 +447,13 @@ typedef struct PgStat_ShmemControl PgStatShared_IO io; PgStatShared_SLRU slru; PgStatShared_Wal wal; + + /* + * Custom stats data with fixed-numbered objects, indexed by (PgStat_Kind + * - PGSTAT_KIND_CUSTOM_MIN). + */ + void *custom_data[PGSTAT_KIND_CUSTOM_SIZE]; + } PgStat_ShmemControl; @@ -459,7 +467,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; @@ -473,6 +481,14 @@ typedef struct PgStat_Snapshot PgStat_WalStats wal; + /* + * Data in snapshot for custom fixed-numbered statistics, indexed by + * (PgStat_Kind - PGSTAT_KIND_CUSTOM_MIN). Each entry is allocated in + * TopMemoryContext, for a size of shared_data_len. + */ + bool custom_valid[PGSTAT_KIND_CUSTOM_SIZE]; + void *custom_data[PGSTAT_KIND_CUSTOM_SIZE]; + /* to free snapshot in bulk */ MemoryContext context; struct pgstat_snapshot_hash *stats; @@ -516,6 +532,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 805712d414..aceb4bed00 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 external modules to define custom statistics kinds, + * that can use the same properties as any built-in 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 the built-in statistics kinds + * defined, 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 @@ -174,6 +182,8 @@ typedef struct PgStat_SnapshotEntry static void pgstat_write_statsfile(void); static void pgstat_read_statsfile(void); +static void pgstat_init_snapshot_fixed(void); + static void pgstat_reset_after_failure(void); static bool pgstat_flush_pending_entries(bool nowait); @@ -251,7 +261,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. @@ -263,7 +273,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 */ @@ -436,6 +446,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. @@ -586,6 +605,8 @@ pgstat_initialize(void) pgstat_init_wal(); + pgstat_init_snapshot_fixed(); + /* Set up a process-exit hook to clean up */ before_shmem_exit(pgstat_shutdown_hook, 0); @@ -829,6 +850,8 @@ pgstat_clear_snapshot(void) memset(&pgStatLocal.snapshot.fixed_valid, 0, sizeof(pgStatLocal.snapshot.fixed_valid)); + memset(&pgStatLocal.snapshot.custom_valid, 0, + sizeof(pgStatLocal.snapshot.custom_valid)); pgStatLocal.snapshot.stats = NULL; pgStatLocal.snapshot.mode = PGSTAT_FETCH_CONSISTENCY_NONE; @@ -992,7 +1015,29 @@ pgstat_snapshot_fixed(PgStat_Kind kind) else pgstat_build_snapshot_fixed(kind); - Assert(pgStatLocal.snapshot.fixed_valid[kind]); + if (pgstat_is_kind_builtin(kind)) + Assert(pgStatLocal.snapshot.fixed_valid[kind]); + else if (pgstat_is_kind_custom(kind)) + Assert(pgStatLocal.snapshot.custom_valid[kind - PGSTAT_KIND_CUSTOM_MIN]); +} + +static void +pgstat_init_snapshot_fixed(void) +{ + /* + * Initialize fixed-numbered statistics data in snapshots, only for custom + * stats kinds. + */ + 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 || !kind_info->fixed_amount) + continue; + + pgStatLocal.snapshot.custom_data[kind - PGSTAT_KIND_CUSTOM_MIN] = + MemoryContextAlloc(TopMemoryContext, kind_info->shared_data_len); + } } static void @@ -1088,10 +1133,12 @@ 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; kind <= PGSTAT_KIND_MAX; kind++) { const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); + if (!kind_info) + continue; if (!kind_info->fixed_amount) { Assert(kind_info->snapshot_cb == NULL); @@ -1108,6 +1155,20 @@ static void pgstat_build_snapshot_fixed(PgStat_Kind kind) { const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); + int idx; + bool *valid; + + /* Position in fixed_valid or custom_valid */ + if (pgstat_is_kind_builtin(kind)) + { + idx = kind; + valid = pgStatLocal.snapshot.fixed_valid; + } + else + { + idx = kind - PGSTAT_KIND_CUSTOM_MIN; + valid = pgStatLocal.snapshot.custom_valid; + } Assert(kind_info->fixed_amount); Assert(kind_info->snapshot_cb != NULL); @@ -1115,21 +1176,21 @@ pgstat_build_snapshot_fixed(PgStat_Kind kind) if (pgstat_fetch_consistency == PGSTAT_FETCH_CONSISTENCY_NONE) { /* rebuild every time */ - pgStatLocal.snapshot.fixed_valid[kind] = false; + valid[idx] = false; } - else if (pgStatLocal.snapshot.fixed_valid[kind]) + else if (valid[idx]) { /* in snapshot mode we shouldn't get called again */ Assert(pgstat_fetch_consistency == PGSTAT_FETCH_CONSISTENCY_CACHE); return; } - Assert(!pgStatLocal.snapshot.fixed_valid[kind]); + Assert(!valid[idx]); kind_info->snapshot_cb(); - Assert(!pgStatLocal.snapshot.fixed_valid[kind]); - pgStatLocal.snapshot.fixed_valid[kind] = true; + Assert(!valid[idx]); + valid[idx] = true; } @@ -1285,30 +1346,127 @@ 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 * pgstat_get_kind_info(PgStat_Kind kind) { - Assert(pgstat_is_kind_valid(kind)); + if (pgstat_is_kind_builtin(kind)) + return &pgstat_kind_builtin_infos[kind]; - return &pgstat_kind_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]; + } + + return NULL; +} + +/* + * 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\"."))); + + /* + * Check some data for fixed-numbered stats. + */ + if (kind_info->fixed_amount) + { + if (kind_info->shared_size == 0) + ereport(ERROR, + (errmsg("custom cumulative statistics property is invalid"), + errhint("Custom cumulative statistics require a shared memory size for fixed-numbered objects."))); + } + + /* + * 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 cumulative statistics \"%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 cumulative statistics \"%s\" with ID %u", + kind_info->name, kind))); } /* @@ -1385,18 +1543,22 @@ pgstat_write_statsfile(void) write_chunk_s(fpout, &format_id); /* Write various stats structs for fixed number of objects */ - for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + for (int kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) { char *ptr; const PgStat_KindInfo *info = pgstat_get_kind_info(kind); - if (!info->fixed_amount) + if (!info || !info->fixed_amount) continue; - Assert(info->snapshot_ctl_off != 0); + if (pgstat_is_kind_builtin(kind)) + Assert(info->snapshot_ctl_off != 0); pgstat_build_snapshot_fixed(kind); - ptr = ((char *) &pgStatLocal.snapshot) + info->snapshot_ctl_off; + if (pgstat_is_kind_builtin(kind)) + ptr = ((char *) &pgStatLocal.snapshot) + info->snapshot_ctl_off; + else + ptr = pgStatLocal.snapshot.custom_data[kind - PGSTAT_KIND_CUSTOM_MIN]; fputc(PGSTAT_FILE_ENTRY_FIXED, fpout); write_chunk_s(fpout, &kind); @@ -1419,6 +1581,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); @@ -1570,8 +1743,16 @@ pgstat_read_statsfile(void) goto error; /* Load back stats into shared memory */ - ptr = ((char *) shmem) + info->shared_ctl_off + - info->shared_data_off; + if (pgstat_is_kind_builtin(kind)) + ptr = ((char *) shmem) + info->shared_ctl_off + + info->shared_data_off; + else + { + int idx = kind - PGSTAT_KIND_CUSTOM_MIN; + + ptr = ((char *) shmem->custom_data[idx]) + + info->shared_data_off; + } if (!read_chunk(fpin, ptr, info->shared_data_len)) goto error; @@ -1638,7 +1819,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; } @@ -1696,11 +1877,11 @@ pgstat_reset_after_failure(void) TimestampTz ts = GetCurrentTimestamp(); /* reset fixed-numbered stats */ - for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + for (int kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) { const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); - if (!kind_info->fixed_amount) + if (!kind_info || !kind_info->fixed_amount) continue; kind_info->reset_all_cb(ts); diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 1c2b69d563..ca3d3eecfb 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -131,6 +131,21 @@ StatsShmemSize(void) sz = MAXALIGN(sizeof(PgStat_ShmemControl)); sz = add_size(sz, pgstat_dsa_init_size()); + /* Add shared memory for all the custom fixed-numbered statistics */ + 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; + + Assert(kind_info->shared_size != 0); + + sz += MAXALIGN(kind_info->shared_size); + } + return sz; } @@ -197,15 +212,25 @@ StatsShmemInit(void) pg_atomic_init_u64(&ctl->gc_request_count, 1); /* initialize fixed-numbered stats */ - for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + for (int kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) { const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); char *ptr; - if (!kind_info->fixed_amount) + if (!kind_info || !kind_info->fixed_amount) continue; - ptr = ((char *) ctl) + kind_info->shared_ctl_off; + if (pgstat_is_kind_builtin(kind)) + ptr = ((char *) ctl) + kind_info->shared_ctl_off; + else + { + int idx = kind - PGSTAT_KIND_CUSTOM_MIN; + + Assert(kind_info->shared_size != 0); + ctl->custom_data[idx] = ShmemAlloc(kind_info->shared_size); + ptr = ctl->custom_data[idx]; + } + kind_info->init_shmem_cb(ptr); } } 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.2
From 6081492b5d0e603496dcf6edfe667bd431d8b86f Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Wed, 3 Jul 2024 13:49:53 +0900
Subject: [PATCH v6 3/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 756a9d07fb..878722af51 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3700,6 +3700,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.2
From ae6acc8de61ebd9e1c040b46c859b4d075f70e2a Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 8 Jul 2024 13:36:06 +0900
Subject: [PATCH v6 4/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 | 48 +++++
src/tools/pgindent/typedefs.list | 2 +
8 files changed, 322 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 2ffd2f77ed..7b9cd12a2a 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"
@@ -11,9 +14,13 @@ REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
ISOLATION = inplace
+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 e275c2cf5b..747a64e812 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -64,3 +64,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 b6c8e89324..eb411b9d44 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();
}
@@ -417,5 +431,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 3c23c14d81..a52fe5121e 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'
@@ -42,4 +43,12 @@ tests += {
'inplace',
],
},
+ '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..7d5a96e522
--- /dev/null
+++ b/src/test/modules/injection_points/t/001_stats.pl
@@ -0,0 +1,48 @@
+
+# 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 635e6d6e21..5206029e96 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2118,6 +2118,7 @@ PgStatShared_Common
PgStatShared_Database
PgStatShared_Function
PgStatShared_HashEntry
+PgStatShared_InjectionPoint
PgStatShared_IO
PgStatShared_Relation
PgStatShared_ReplSlot
@@ -2149,6 +2150,7 @@ PgStat_Snapshot
PgStat_SnapshotEntry
PgStat_StatDBEntry
PgStat_StatFuncEntry
+PgStat_StatInjEntry
PgStat_StatReplSlotEntry
PgStat_StatSubEntry
PgStat_StatTabEntry
--
2.45.2
From f3a4855aa5356c7d21f656f37d8799b91ba85b52 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 8 Jul 2024 13:44:14 +0900
Subject: [PATCH v6 5/5] injection_points: Add example for fixed-numbered
statistics
This acts as a template to show what can be achieved with fixed-numbered
stats (like WAL, bgwriter, etc.) for pluggable cumulative statistics.
---
.../injection_points--1.0.sql | 11 ++
.../injection_points/injection_points.c | 3 +
.../injection_points/injection_stats.c | 155 +++++++++++++++++-
.../injection_points/injection_stats.h | 3 +
.../modules/injection_points/t/001_stats.pl | 11 +-
5 files changed, 180 insertions(+), 3 deletions(-)
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 747a64e812..fa0b1d06ae 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -74,3 +74,14 @@ CREATE FUNCTION injection_points_stats_numcalls(IN point_name TEXT)
RETURNS bigint
AS 'MODULE_PATHNAME', 'injection_points_stats_numcalls'
LANGUAGE C STRICT;
+
+--
+-- injection_points_stats_fixed()
+--
+-- Reports fixed-numbered statistics for injection points.
+CREATE FUNCTION injection_points_stats_fixed(OUT numattach int8,
+ OUT numdetach int8,
+ OUT numrun int8)
+RETURNS record
+AS 'MODULE_PATHNAME', 'injection_points_stats_fixed'
+LANGUAGE C STRICT;
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index eb411b9d44..f65bd83cc3 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -297,6 +297,7 @@ injection_points_attach(PG_FUNCTION_ARGS)
condition.pid = MyProcPid;
}
+ pgstat_report_inj_fixed(1, 0, 0);
InjectionPointAttach(name, "injection_points", function, &condition,
sizeof(InjectionPointCondition));
@@ -342,6 +343,7 @@ injection_points_run(PG_FUNCTION_ARGS)
{
char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ pgstat_report_inj_fixed(0, 0, 1);
INJECTION_POINT(name);
PG_RETURN_VOID();
@@ -418,6 +420,7 @@ injection_points_detach(PG_FUNCTION_ARGS)
{
char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ pgstat_report_inj_fixed(0, 1, 0);
if (!InjectionPointDetach(name))
elog(ERROR, "could not detach injection point \"%s\"", name);
diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c
index c37b0b33d3..69560b7fdc 100644
--- a/src/test/modules/injection_points/injection_stats.c
+++ b/src/test/modules/injection_points/injection_stats.c
@@ -17,12 +17,13 @@
#include "fmgr.h"
#include "common/hashfn.h"
+#include "funcapi.h"
#include "injection_stats.h"
#include "pgstat.h"
#include "utils/builtins.h"
#include "utils/pgstat_internal.h"
-/* Structures for statistics of injection points */
+/* Structures for statistics of injection points, variable-size */
typedef struct PgStat_StatInjEntry
{
PgStat_Counter numcalls; /* number of times point has been run */
@@ -35,6 +36,9 @@ typedef struct PgStatShared_InjectionPoint
} PgStatShared_InjectionPoint;
static bool injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
+static void injection_stats_fixed_init_shmem_cb(void *stats);
+static void injection_stats_fixed_reset_all_cb(TimestampTz ts);
+static void injection_stats_fixed_snapshot_cb(void);
static const PgStat_KindInfo injection_stats = {
.name = "injection_points",
@@ -50,18 +54,51 @@ static const PgStat_KindInfo injection_stats = {
.flush_pending_cb = injection_stats_flush_cb,
};
+/* Structures for statistics of injection points, fixed-size */
+typedef struct PgStat_StatInjFixedEntry
+{
+ PgStat_Counter numattach; /* number of points attached */
+ PgStat_Counter numdetach; /* number of points detached */
+ PgStat_Counter numrun; /* number of points run */
+ TimestampTz stat_reset_timestamp;
+} PgStat_StatInjFixedEntry;
+
+typedef struct PgStatShared_InjectionPointFixed
+{
+ LWLock lock; /* protects all the counters */
+ uint32 changecount;
+ PgStat_StatInjFixedEntry stats;
+ PgStat_StatInjFixedEntry reset_offset;
+} PgStatShared_InjectionPointFixed;
+
+static const PgStat_KindInfo injection_stats_fixed = {
+ .name = "injection_points_fixed",
+ .fixed_amount = true,
+
+ .shared_size = sizeof(PgStat_StatInjFixedEntry),
+ .shared_data_off = offsetof(PgStatShared_InjectionPointFixed, stats),
+ .shared_data_len = sizeof(((PgStatShared_InjectionPointFixed *) 0)->stats),
+
+ .init_shmem_cb = injection_stats_fixed_init_shmem_cb,
+ .reset_all_cb = injection_stats_fixed_reset_all_cb,
+ .snapshot_cb = injection_stats_fixed_snapshot_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
+#define PGSTAT_KIND_INJECTION_FIXED (PGSTAT_KIND_EXPERIMENTAL + 1)
+/* Position of fixed-numbered data in internal structures */
+#define PGSTAT_KIND_INJECTION_IDX (PGSTAT_KIND_INJECTION_FIXED - PGSTAT_KIND_CUSTOM_MIN)
/* Track if stats are loaded */
static bool inj_stats_loaded = false;
/*
- * Callback for stats handling
+ * Callbacks for stats handling
*/
static bool
injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
@@ -79,6 +116,59 @@ injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
return true;
}
+static void
+injection_stats_fixed_init_shmem_cb(void *stats)
+{
+ PgStatShared_InjectionPointFixed *stats_shmem =
+ (PgStatShared_InjectionPointFixed *) stats;
+
+ LWLockInitialize(&stats_shmem->lock, LWTRANCHE_PGSTATS_DATA);
+}
+
+static void
+injection_stats_fixed_reset_all_cb(TimestampTz ts)
+{
+ PgStatShared_InjectionPointFixed *stats_shmem =
+ (PgStatShared_InjectionPointFixed *)
+ pgStatLocal.shmem->custom_data[PGSTAT_KIND_INJECTION_IDX];
+
+ LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE);
+ pgstat_copy_changecounted_stats(&stats_shmem->reset_offset,
+ &stats_shmem->stats,
+ sizeof(stats_shmem->stats),
+ &stats_shmem->changecount);
+ stats_shmem->stats.stat_reset_timestamp = ts;
+ LWLockRelease(&stats_shmem->lock);
+}
+
+static void
+injection_stats_fixed_snapshot_cb(void)
+{
+ PgStatShared_InjectionPointFixed *stats_shmem =
+ (PgStatShared_InjectionPointFixed *)
+ pgStatLocal.shmem->custom_data[PGSTAT_KIND_INJECTION_IDX];
+ PgStat_StatInjFixedEntry *stat_snap = (PgStat_StatInjFixedEntry *)
+ pgStatLocal.snapshot.custom_data[PGSTAT_KIND_INJECTION_IDX];
+ PgStat_StatInjFixedEntry *reset_offset = &stats_shmem->reset_offset;
+ PgStat_StatInjFixedEntry reset;
+
+ pgstat_copy_changecounted_stats(stat_snap,
+ &stats_shmem->stats,
+ sizeof(stats_shmem->stats),
+ &stats_shmem->changecount);
+
+ LWLockAcquire(&stats_shmem->lock, LW_SHARED);
+ memcpy(&reset, reset_offset, sizeof(stats_shmem->stats));
+ LWLockRelease(&stats_shmem->lock);
+
+ /* compensate by reset offsets */
+#define FIXED_COMP(fld) stat_snap->fld -= reset.fld;
+ FIXED_COMP(numattach);
+ FIXED_COMP(numdetach);
+ FIXED_COMP(numrun);
+#undef FIXED_COMP
+}
+
/*
* Support function for the SQL-callable pgstat* functions. Returns
* a pointer to the injection point statistics struct.
@@ -105,6 +195,7 @@ void
pgstat_register_inj(void)
{
pgstat_register_kind(PGSTAT_KIND_INJECTION, &injection_stats);
+ pgstat_register_kind(PGSTAT_KIND_INJECTION_FIXED, &injection_stats_fixed);
/* mark stats as loaded */
inj_stats_loaded = true;
@@ -176,6 +267,30 @@ pgstat_report_inj(const char *name)
pgstat_unlock_entry(entry_ref);
}
+/*
+ * Report fixed number of statistics for an injection point.
+ */
+void
+pgstat_report_inj_fixed(uint32 numattach,
+ uint32 numdetach,
+ uint32 numrun)
+{
+ PgStatShared_InjectionPointFixed *stats_shmem;
+
+ /* leave if disabled */
+ if (!inj_stats_loaded)
+ return;
+
+ stats_shmem = (PgStatShared_InjectionPointFixed *)
+ pgStatLocal.shmem->custom_data[PGSTAT_KIND_INJECTION_IDX];
+
+ pgstat_begin_changecount_write(&stats_shmem->changecount);
+ stats_shmem->stats.numattach += numattach;
+ stats_shmem->stats.numdetach += numdetach;
+ stats_shmem->stats.numrun += numrun;
+ pgstat_end_changecount_write(&stats_shmem->changecount);
+}
+
/*
* SQL function returning the number of times an injection point
* has been called.
@@ -192,3 +307,39 @@ injection_points_stats_numcalls(PG_FUNCTION_ARGS)
PG_RETURN_INT64(entry->numcalls);
}
+
+/*
+ * SQL function returning fixed-numbered statistics for injection points.
+ */
+PG_FUNCTION_INFO_V1(injection_points_stats_fixed);
+Datum
+injection_points_stats_fixed(PG_FUNCTION_ARGS)
+{
+ TupleDesc tupdesc;
+ Datum values[3] = {0};
+ bool nulls[3] = {0};
+ PgStat_StatInjFixedEntry *stats;
+
+ pgstat_snapshot_fixed(PGSTAT_KIND_INJECTION_FIXED);
+ stats = pgStatLocal.snapshot.custom_data[PGSTAT_KIND_INJECTION_IDX];
+
+ /* Initialise attributes information in the tuple descriptor */
+ tupdesc = CreateTemplateTupleDesc(3);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 1, "numattach",
+ INT8OID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 2, "numdetach",
+ INT8OID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 3, "numrun",
+ INT8OID, -1, 0);
+ BlessTupleDesc(tupdesc);
+
+ values[0] = Int64GetDatum(stats->numattach);
+ values[1] = Int64GetDatum(stats->numdetach);
+ values[2] = Int64GetDatum(stats->numrun);
+ nulls[0] = false;
+ nulls[1] = false;
+ nulls[2] = false;
+
+ /* Returns the record as Datum */
+ PG_RETURN_DATUM(HeapTupleGetDatum(heap_form_tuple(tupdesc, values, nulls)));
+}
diff --git a/src/test/modules/injection_points/injection_stats.h b/src/test/modules/injection_points/injection_stats.h
index 3e99705483..cf68b25f7b 100644
--- a/src/test/modules/injection_points/injection_stats.h
+++ b/src/test/modules/injection_points/injection_stats.h
@@ -19,5 +19,8 @@ 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);
+extern void pgstat_report_inj_fixed(uint32 numattach,
+ uint32 numdetach,
+ uint32 numrun);
#endif
diff --git a/src/test/modules/injection_points/t/001_stats.pl b/src/test/modules/injection_points/t/001_stats.pl
index 7d5a96e522..e3c69b94ca 100644
--- a/src/test/modules/injection_points/t/001_stats.pl
+++ b/src/test/modules/injection_points/t/001_stats.pl
@@ -31,18 +31,27 @@ $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');
+my $fixedstats = $node->safe_psql('postgres',
+ "SELECT * FROM injection_points_stats_fixed();");
+is($fixedstats, '1|0|2', 'number of fixed stats');
# 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');
+$fixedstats = $node->safe_psql('postgres',
+ "SELECT * FROM injection_points_stats_fixed();");
+is($fixedstats, '1|0|2', 'number of fixed 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');
+is($numcalls, '', 'number of stats after crash');
+$fixedstats = $node->safe_psql('postgres',
+ "SELECT * FROM injection_points_stats_fixed();");
+is($fixedstats, '0|0|0', 'number of fixed stats after crash');
done_testing();
--
2.45.2
signature.asc
Description: PGP signature
