Hi all, While looking at ways to make pg_stat_statements more scalable and dynamically manageable (no more PGC_POSTMASTER for the max number of entries), which came out as using a dshash, Andres has mentioned me off-list (on twitter/X) that we'd better plug in it to the shmem pgstats facility, moving the text file that holds the query strings into memory (with size restrictions for the query strings, for example). This has challenges on its own (query ID is 8 bytes incompatible with the dboid/objid hash key used by pgstats, discard of entries when maximum). Anyway, this won't happen if we don't do one of these two things: 1) Move pg_stat_statements into core, adapting pgstats for its requirements. 2) Make the shmem pgstats pluggable so as it is possible for extensions to register their own stats kinds.
1) may have its advantages, still I am not sure if we want to do that.
And 2) is actually something that can be used for more things than
just pg_stat_statements, because people love extensions and
statistics (spoiler: I do). The idea is simple: any extension
defining a custom stats kind would be able to rely on all the in-core
facilities we use for the existing in-core kinds:
a) Snapshotting and caching of the stats, via stats_fetch_consistency.
b) Native handling and persistency of the custom stats data.
c) Reuse stats after a crash, pointing at this comment in xlog.c:
* TODO: With a bit of extra work we could just start with a pgstat file
* associated with the checkpoint redo location we're starting from.
This means that we always remove the stats after a crash. That's
something I have a patch for, not for this thread, but the idea is
that custom stats would also benefit from this property.
The implementation is based on the following ideas:
* A structure in shared memory that tracks the IDs of the custom stats
kinds with their names. These are incremented starting from
PGSTAT_KIND_LAST.
* Processes use a local array cache that keeps tracks of all the
custom PgStat_KindInfos, indexed by (kind_id - PGSTAT_KIND_LAST).
* The kind IDs may change across restarts, meaning that any stats data
associated to a custom kind is stored with the *name* of the custom
stats kind. Depending on the discussion happening here, I'd be open
to use the same concept as custom RMGRs, where custom kind IDs are
"reserved", fixed in time, and tracked in the Postgres wiki. It is
cheaper to store the stats this way, as well, while managing conflicts
across extensions available in the community ecosystem.
* Custom stats can be added without shared_preload_libraries,
loading them from a shmem startup hook with shared_preload_libraries
is also possible.
* The shmem pgstats defines two types of statistics: the ones in a
dshash and what's called a "fixed" type like for archiver, WAL, etc.
pointing to areas of shared memory. All the fixed types are linked to
structures for snapshotting and shmem tracking. As a matter of
simplification and because I could not really see a case where I'd
want to plug in a fixed stats kind, the patch forbids this case. This
case could be allowed, but I'd rather refactor the structures of
pgstat_internal.h so as we don't have traces of the "fixed" stats
structures in so many areas.
* Making custom stats data persistent is an interesting problem, and
there are a couple of approaches I've considered:
** Allow custom kinds to define callbacks to read and write data from
a source they'd want, like their own file through a fd. This has the
disadvantage to remove the benefit of c) above.
** Store everything in the existing stats file, adding one type of
entry like 'S' and 'N' with a "custom" type, where the *name* of the
custom stats kind is stored instead of its ID computed from shared
memory.
A mix of both? The patch attached has used the second approach. If
the process reading/writing the stats does not know about the custom
stats data, the data is discarded.
* pgstat.c has a big array called pgstat_kind_infos to define all the
existing stats kinds. Perhaps the code should be refactored to use
this new API? That would make the code more consistent with what we
do for resource managers, for one, while moving the KindInfos into
their own file. With that in mind, storing the kind ID in KindInfos
feels intuitive.
While thinking about a use case to show what these APIs can do, I have
decided to add statistics to the existing module injection_points
rather than implement a new test module, gathering data about them and
have tests that could use this data (like tracking the number of times
a point is taken). This is simple enough that it can be used as a
template, as well. There is a TAP test checking the data persistence
across restarts, so I did not mess up this part much, hopefully.
Please find attached a patch set implementing these ideas:
- 0001 switches PgStat_Kind from an enum to a uint32, for the internal
counters.
- 0002 is some cleanup for the hardcoded S, N and E in pgstat.c.
- 0003 introduces the backend-side APIs, with the shmem table counter
and the routine to give code paths a way to register their own stats
kind (see pgstat_add_kind).
- 0004 implements an example of how to use these APIs, see
injection_stats.c in src/test/modules/injection_points/.
- 0005 adds some docs.
- 0006 is an idea of how to make this custom stats data persistent.
This will hopefully spark a discussion, and I was looking for answers
regarding these questions:
- Should the pgstat_kind_infos array in pgstat.c be refactored to use
something similar to pgstat_add_kind?
- How should the persistence of the custom stats be achieved?
Callbacks to give custom stats kinds a way to write/read their data,
push everything into a single file, or support both?
- Should this do like custom RMGRs and assign to each stats kinds ID
that are set in stone rather than dynamic ones?
Thanks for reading.
--
Michael
From 0847075d42b9f9ad5ba882d51a3c3de984f8e563 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 13 Jun 2024 13:25:05 +0900
Subject: [PATCH 1/6] 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.43.0
From dd7a98a990aa28a06a82607052180f69ee44e05f Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 13 Jun 2024 13:25:48 +0900
Subject: [PATCH 2/6] 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.43.0
From 8fbf5e027dfbcc9fd3ebc52cb82000a855c23064 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 13 Jun 2024 13:32:22 +0900
Subject: [PATCH 3/6] 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.
---
src/include/pgstat.h | 2 +
src/include/storage/lwlocklist.h | 1 +
src/include/utils/pgstat_internal.h | 1 +
src/backend/storage/ipc/ipci.c | 2 +
src/backend/utils/activity/pgstat.c | 301 +++++++++++++++++-
.../utils/activity/wait_event_names.txt | 1 +
src/tools/pgindent/typedefs.list | 3 +
7 files changed, 308 insertions(+), 3 deletions(-)
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 2d30fadaf1..b3cdc0da6d 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -463,6 +463,8 @@ typedef struct PgStat_PendingWalStats
/* functions called from postmaster */
extern Size StatsShmemSize(void);
extern void StatsShmemInit(void);
+extern Size StatsKindShmemSize(void);
+extern void StatsKindShmemInit(void);
/* Functions called during server startup / shutdown */
extern void pgstat_restore_stats(void);
diff --git a/src/include/storage/lwlocklist.h b/src/include/storage/lwlocklist.h
index 85f6568b9e..ed78f93683 100644
--- a/src/include/storage/lwlocklist.h
+++ b/src/include/storage/lwlocklist.h
@@ -83,3 +83,4 @@ PG_LWLOCK(49, WALSummarizer)
PG_LWLOCK(50, DSMRegistry)
PG_LWLOCK(51, InjectionPoint)
PG_LWLOCK(52, SerialControl)
+PG_LWLOCK(53, PgStatKind)
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index dbbca31602..21dfff740d 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -503,6 +503,7 @@ static inline void *pgstat_get_entry_data(PgStat_Kind kind, PgStatShared_Common
*/
extern const PgStat_KindInfo *pgstat_get_kind_info(PgStat_Kind kind);
+extern PgStat_Kind pgstat_add_kind(const PgStat_KindInfo *kind_info);
#ifdef USE_ASSERT_CHECKING
extern void pgstat_assert_is_up(void);
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index 521ed5418c..8b5023d9de 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -149,6 +149,7 @@ CalculateShmemSize(int *num_semaphores)
size = add_size(size, SyncScanShmemSize());
size = add_size(size, AsyncShmemSize());
size = add_size(size, StatsShmemSize());
+ size = add_size(size, StatsKindShmemSize());
size = add_size(size, WaitEventExtensionShmemSize());
size = add_size(size, InjectionPointShmemSize());
size = add_size(size, SlotSyncShmemSize());
@@ -355,6 +356,7 @@ CreateOrAttachShmemStructs(void)
SyncScanShmemInit();
AsyncShmemInit();
StatsShmemInit();
+ StatsKindShmemInit();
WaitEventExtensionShmemInit();
InjectionPointShmemInit();
}
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index f03fee7cd5..b96743ce84 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -58,6 +58,12 @@
* PGSTAT_FETCH_CONSISTENCY_SNAPSHOT statistics are stored in
* pgStatLocal.snapshot.
*
+ * 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
+ * kind is assigned a unique PgStat_Kind stored in shared memory with the
+ * name of the statistics kind. Each PgStat_KindInfo is maintained in a
+ * local array cache known to the current process.
+ *
* To keep things manageable, stats handling is split across several
* files. Infrastructure pieces are in:
* - pgstat.c - this file, to tie it all together
@@ -94,13 +100,18 @@
#include <unistd.h>
#include "access/xact.h"
+#include "common/int.h"
#include "lib/dshash.h"
#include "pgstat.h"
#include "port/atomics.h"
#include "storage/fd.h"
#include "storage/ipc.h"
#include "storage/lwlock.h"
+#include "storage/shmem.h"
+#include "storage/spin.h"
+#include "storage/s_lock.h"
#include "utils/guc_hooks.h"
+#include "utils/hsearch.h"
#include "utils/memutils.h"
#include "utils/pgstat_internal.h"
#include "utils/timestamp.h"
@@ -400,6 +411,121 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
};
+/* --------
+ * Hash tables for storing custom pgstats kinds
+ *
+ * PgStatKindHashById is used to find the name from a PgStat_Kind.
+ * Any backend can search it to find custom stats kinds
+ *
+ * PgStatKindHashByName is used to find the PgStat_Kind from a name.
+ * It is used to ensure that no duplicated entries are registered.
+ *
+ * The size of the hash table is based on the assumption that
+ * PGSTAT_KIND_HASH_INIT_SIZE is enough for most cases, and it seems
+ * unlikely that the number of entries will reach
+ * PGSTAT_KIND_HASH_MAX_SIZE.
+ * --------
+ */
+
+static HTAB *PgStatKindHashById; /* find PgStat_KindInfo from IDs */
+static HTAB *PgStatKindHashByName; /* find PgStat_Kind from names */
+
+#define PGSTAT_KIND_HASH_INIT_SIZE 16
+#define PGSTAT_KIND_HASH_MAX_SIZE 128
+
+/* hash table entries */
+typedef struct PgStatKindEntryById
+{
+ PgStat_Kind kind; /* hash key */
+ char kind_name[NAMEDATALEN]; /* stats kind name */
+} PgStatKindEntryById;
+
+typedef struct PgStatKindEntryByName
+{
+ char kind_name[NAMEDATALEN]; /* hash key */
+ PgStat_Kind kind; /* kind ID */
+} PgStatKindEntryByName;
+
+/* dynamic allocation counter for custom pgstats kinds */
+typedef struct PgStatKindCounterData
+{
+ int nextId; /* next ID to assign */
+ slock_t mutex; /* protects the counter */
+} PgStatKindCounterData;
+
+/* pointer to the shared memory */
+static PgStatKindCounterData *PgStatKindCounter;
+
+/* first ID of custom pgstats kinds, as stored in pgstats tables */
+#define PGSTAT_KIND_INITIAL_ID (PGSTAT_KIND_LAST + 1)
+
+/*
+ * Local array cache pointing to the custom PgStat_KindInfos known to the
+ * current process, indexed by kind ID minus PGSTAT_KIND_LAST. Any unused
+ * entries in the array will contain NULL.
+ */
+static const PgStat_KindInfo **PgStatKindCache = NULL;
+static uint32 PgStatKindCacheNum = 0;
+
+/* -----------------------------------------------------------
+ * Functions managing the shared memory for custom stats kinds
+ * -----------------------------------------------------------
+ */
+
+/*
+ * Compute shared memory space needed for custom stats kinds
+ */
+Size
+StatsKindShmemSize(void)
+{
+ Size sz;
+
+ sz = MAXALIGN(sizeof(PgStatKindCounterData));
+ sz = add_size(sz, hash_estimate_size(PGSTAT_KIND_HASH_MAX_SIZE,
+ sizeof(PgStatKindEntryById)));
+ sz = add_size(sz, hash_estimate_size(PGSTAT_KIND_HASH_MAX_SIZE,
+ sizeof(PgStatKindEntryByName)));
+ return sz;
+}
+
+/*
+ * Initialize shared memory area for custom stats kinds during startup
+ */
+void
+StatsKindShmemInit(void)
+{
+ bool found;
+ HASHCTL info;
+
+ PgStatKindCounter = (PgStatKindCounterData *)
+ ShmemInitStruct("PgStatKindCounterData",
+ sizeof(PgStatKindCounterData), &found);
+ if (!found)
+ {
+ /* initialize the allocation counter and its spinlock. */
+ PgStatKindCounter->nextId = PGSTAT_KIND_INITIAL_ID;
+ SpinLockInit(&PgStatKindCounter->mutex);
+ }
+
+ /* initialize or attach the hash tables to store custom stats kinds */
+ info.keysize = sizeof(PgStat_Kind);
+ info.entrysize = sizeof(PgStatKindEntryById);
+ PgStatKindHashById = ShmemInitHash("PgStatKind hash by id",
+ PGSTAT_KIND_HASH_INIT_SIZE,
+ PGSTAT_KIND_HASH_MAX_SIZE,
+ &info,
+ HASH_ELEM | HASH_BLOBS);
+
+ /* key is a NULL-terminated string */
+ info.keysize = sizeof(char[NAMEDATALEN]);
+ info.entrysize = sizeof(PgStatKindEntryByName);
+ PgStatKindHashByName = ShmemInitHash("PgStatKind hash by name",
+ PGSTAT_KIND_HASH_INIT_SIZE,
+ PGSTAT_KIND_HASH_MAX_SIZE,
+ &info,
+ HASH_ELEM | HASH_STRINGS);
+}
+
/* ------------------------------------------------------------
* Functions managing the state of the stats system for all backends.
* ------------------------------------------------------------
@@ -1254,6 +1380,18 @@ pgstat_get_kind_from_str(char *kind_str)
return kind;
}
+ /* Check the local cache if any */
+ if (PgStatKindCacheNum > 0)
+ {
+ for (int kind = 0; kind <= PgStatKindCacheNum; kind++)
+ {
+ if (PgStatKindCache[kind] == NULL)
+ continue;
+ if (pg_strcasecmp(kind_str, PgStatKindCache[kind]->name) == 0)
+ return kind + PGSTAT_KIND_INITIAL_ID;
+ }
+ }
+
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid statistics kind: \"%s\"", kind_str)));
@@ -1263,7 +1401,8 @@ pgstat_get_kind_from_str(char *kind_str)
static inline bool
pgstat_is_kind_valid(PgStat_Kind kind)
{
- return kind >= PGSTAT_KIND_FIRST_VALID && kind <= PGSTAT_KIND_LAST;
+ return kind >= PGSTAT_KIND_FIRST_VALID &&
+ kind <= (PGSTAT_KIND_LAST + PgStatKindCacheNum);
}
const PgStat_KindInfo *
@@ -1271,7 +1410,152 @@ pgstat_get_kind_info(PgStat_Kind kind)
{
Assert(pgstat_is_kind_valid(kind));
- return &pgstat_kind_infos[kind];
+ if (kind >= PGSTAT_KIND_FIRST_VALID && kind <= PGSTAT_KIND_LAST)
+ return &pgstat_kind_infos[kind];
+
+ /* Look in local cache */
+ if (kind >= PGSTAT_KIND_INITIAL_ID &&
+ kind <= PGSTAT_KIND_INITIAL_ID + PgStatKindCacheNum)
+ return PgStatKindCache[kind - PGSTAT_KIND_INITIAL_ID];
+
+ Assert(false);
+ return NULL; /* keep compiler quiet */
+}
+
+/*
+ * Save a PgStat_KindInfo into the local cache, if not already done.
+ */
+static void
+pgstat_save_kind_info(PgStat_Kind kind, const PgStat_KindInfo *kind_info)
+{
+ /* This should only be called for user-defined stats kinds */
+ if (kind <= PGSTAT_KIND_LAST)
+ return;
+
+ /* Convert to array index */
+ kind -= PGSTAT_KIND_INITIAL_ID;
+
+ /* If necessary, create or enlarge local cache array. */
+ if (kind >= PgStatKindCacheNum)
+ {
+ uint32 newalloc;
+
+ /*
+ * Do a simple increment, to keep an exact count of the custom stats
+ * kinds stored rather than an upper-bound. This is more costly each
+ * time a new PgStat_KindInfo is added, but saves in correctness. This
+ * overflow should not happen as this is capped by
+ * PGSTAT_KIND_HASH_MAX_SIZE, but let's be safe.
+ */
+ if (pg_add_u32_overflow(kind, 1, &newalloc))
+ elog(ERROR, "could not allocate memory for custom pgstats");
+
+ if (PgStatKindCache == NULL)
+ PgStatKindCache = (const PgStat_KindInfo **)
+ MemoryContextAllocZero(TopMemoryContext,
+ newalloc * sizeof(PgStat_KindInfo *));
+ else
+ PgStatKindCache = repalloc0_array(PgStatKindCache,
+ const PgStat_KindInfo *,
+ PgStatKindCacheNum,
+ newalloc);
+ PgStatKindCacheNum = newalloc;
+ }
+
+ PgStatKindCache[kind] = kind_info;
+}
+
+/*
+ * Allocate a new stats kind and return its PgStat_Kind.
+ */
+PgStat_Kind
+pgstat_add_kind(const PgStat_KindInfo *kind_info)
+{
+ PgStat_Kind kind;
+ bool found;
+ PgStatKindEntryByName *entry_by_name;
+ PgStatKindEntryById *entry_by_id;
+
+ if (strlen(kind_info->name) >= NAMEDATALEN)
+ elog(ERROR,
+ "cannot use custom stats kind longer than %u characters",
+ NAMEDATALEN - 1);
+
+ /*
+ * These are not supported for now, as these point out to fixed areas of
+ * shared memory.
+ */
+ if (kind_info->fixed_amount)
+ elog(ERROR,
+ "cannot define custom stats kind with fixed amount of data");
+
+ /*
+ * Check if kind ID associated to the name is already defined, and return
+ * it if so.
+ */
+ LWLockAcquire(PgStatKindLock, LW_SHARED);
+ entry_by_name = (PgStatKindEntryByName *)
+ hash_search(PgStatKindHashByName, kind_info->name,
+ HASH_FIND, &found);
+ LWLockRelease(PgStatKindLock);
+
+ if (found)
+ {
+ pgstat_save_kind_info(entry_by_name->kind, kind_info);
+ return entry_by_name->kind;
+ }
+
+ /*
+ * Allocate and register a new stats kind. Recheck if this kind name
+ * exists, as it could be possible that a concurrent process has inserted
+ * one with the same name since the LWLock acquired again here was
+ * previously released.
+ */
+ LWLockAcquire(PgStatKindLock, LW_EXCLUSIVE);
+ entry_by_name = (PgStatKindEntryByName *)
+ hash_search(PgStatKindHashByName, kind_info->name,
+ HASH_FIND, &found);
+ if (found)
+ {
+ LWLockRelease(PgStatKindLock);
+ pgstat_save_kind_info(entry_by_name->kind, kind_info);
+ return entry_by_name->kind;
+ }
+
+ /* Allocate a new kind ID */
+ SpinLockAcquire(&PgStatKindCounter->mutex);
+
+ if (PgStatKindCounter->nextId >= PGSTAT_KIND_HASH_MAX_SIZE - PGSTAT_KIND_INITIAL_ID)
+ {
+ SpinLockRelease(&PgStatKindCounter->mutex);
+ ereport(ERROR,
+ errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("too many stats kinds"));
+ }
+
+ kind = PgStatKindCounter->nextId;
+ PgStatKindCounter->nextId++;
+
+ SpinLockRelease(&PgStatKindCounter->mutex);
+
+ /* Register the new stats kind */
+ entry_by_id = (PgStatKindEntryById *)
+ hash_search(PgStatKindHashById, &kind,
+ HASH_ENTER, &found);
+ Assert(!found);
+ strlcpy(entry_by_id->kind_name, kind_info->name,
+ sizeof(entry_by_id->kind_name));
+
+ entry_by_name = (PgStatKindEntryByName *)
+ hash_search(PgStatKindHashByName, kind_info->name,
+ HASH_ENTER, &found);
+ Assert(!found);
+ entry_by_name->kind = kind;
+
+ LWLockRelease(PgStatKindLock);
+
+ pgstat_save_kind_info(kind, kind_info);
+ return kind;
}
/*
@@ -1405,6 +1689,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);
@@ -1639,7 +1934,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;
}
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index 87cbca2811..32d6d8fd74 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -345,6 +345,7 @@ WALSummarizer "Waiting to read or update WAL summarization state."
DSMRegistry "Waiting to read or update the dynamic shared memory registry."
InjectionPoint "Waiting to read or update information related to injection points."
SerialControl "Waiting to read or update shared <filename>pg_serial</filename> state."
+PgStatKind "Waiting to read or update custom pgstats kind information."
#
# END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE)
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 4f57078d13..8718ca54e0 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2109,6 +2109,9 @@ PgFdwRelationInfo
PgFdwSamplingMethod
PgFdwScanState
PgIfAddrCallback
+PgStatKindCounterData
+PgStatKindEntryById
+PgStatKindEntryByName
PgStatShared_Archiver
PgStatShared_BgWriter
PgStatShared_Checkpointer
--
2.43.0
From 6728eb684760be8b883c69dedc7bb089fa090d7c Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 13 Jun 2024 13:37:24 +0900
Subject: [PATCH 4/6] 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.
---
src/test/modules/injection_points/Makefile | 7 +-
.../expected/injection_points.out | 67 +++++++
.../injection_points--1.0.sql | 10 +
.../injection_points/injection_points.c | 17 ++
.../injection_points/injection_stats.c | 176 ++++++++++++++++++
.../injection_points/injection_stats.h | 22 +++
src/test/modules/injection_points/meson.build | 1 +
.../injection_points/sql/injection_points.sql | 15 ++
src/tools/pgindent/typedefs.list | 2 +
9 files changed, 315 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
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 31bd787994..676823a87f 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"
diff --git a/src/test/modules/injection_points/expected/injection_points.out b/src/test/modules/injection_points/expected/injection_points.out
index dd9db06e10..cb50a664f9 100644
--- a/src/test/modules/injection_points/expected/injection_points.out
+++ b/src/test/modules/injection_points/expected/injection_points.out
@@ -208,5 +208,72 @@ SELECT injection_points_detach('TestConditionLocal1');
(1 row)
+-- Statistics
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- nothing
+ injection_points_stats_numcalls
+---------------------------------
+
+(1 row)
+
+SELECT injection_points_attach('TestConditionStats', 'notice');
+ injection_points_attach
+-------------------------
+
+(1 row)
+
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- returns 0
+ injection_points_stats_numcalls
+---------------------------------
+ 0
+(1 row)
+
+SELECT injection_points_run('TestConditionStats');
+NOTICE: notice triggered for injection point TestConditionStats
+ injection_points_run
+----------------------
+
+(1 row)
+
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- returns 1
+ injection_points_stats_numcalls
+---------------------------------
+ 1
+(1 row)
+
+-- Check transactions with stats snapshots
+BEGIN;
+SET stats_fetch_consistency TO snapshot;
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- returns 1
+ injection_points_stats_numcalls
+---------------------------------
+ 1
+(1 row)
+
+SELECT injection_points_run('TestConditionStats');
+NOTICE: notice triggered for injection point TestConditionStats
+ injection_points_run
+----------------------
+
+(1 row)
+
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- still 1
+ injection_points_stats_numcalls
+---------------------------------
+ 1
+(1 row)
+
+COMMIT;
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- now 2
+ injection_points_stats_numcalls
+---------------------------------
+ 2
+(1 row)
+
+SELECT injection_points_detach('TestConditionStats');
+ injection_points_detach
+-------------------------
+
+(1 row)
+
DROP EXTENSION injection_points;
DROP FUNCTION wait_pid;
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..6346af6cf6 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,8 @@ injection_points_detach(PG_FUNCTION_ARGS)
MemoryContextSwitchTo(oldctx);
}
+ /* Remove stats entry */
+ pgstat_drop_inj(name);
+
PG_RETURN_VOID();
}
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..6314486d79
--- /dev/null
+++ b/src/test/modules/injection_points/injection_stats.c
@@ -0,0 +1,176 @@
+/*--------------------------------------------------------------------------
+ *
+ * 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))
+
+static PgStat_Kind inj_stats_kind = PGSTAT_KIND_INVALID;
+
+/*
+ * 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_kind == PGSTAT_KIND_INVALID)
+ inj_stats_kind = pgstat_add_kind(&injection_stats);
+
+ /* Compile the lookup key as a hash of the point name */
+ entry = (PgStat_StatInjEntry *) pgstat_fetch_entry(inj_stats_kind,
+ InvalidOid,
+ PGSTAT_INJ_IDX(name));
+ return entry;
+}
+
+/*
+ * Report injection point creation.
+ */
+void
+pgstat_create_inj(const char *name)
+{
+ PgStat_EntryRef *entry_ref;
+ PgStatShared_InjectionPoint *shstatent;
+
+ if (inj_stats_kind == PGSTAT_KIND_INVALID)
+ inj_stats_kind = pgstat_add_kind(&injection_stats);
+
+ entry_ref = pgstat_get_entry_ref_locked(inj_stats_kind, 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)
+{
+ if (inj_stats_kind == PGSTAT_KIND_INVALID)
+ inj_stats_kind = pgstat_add_kind(&injection_stats);
+
+ if (!pgstat_drop_entry(inj_stats_kind, 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;
+
+ if (inj_stats_kind == PGSTAT_KIND_INVALID)
+ inj_stats_kind = pgstat_add_kind(&injection_stats);
+
+ entry_ref = pgstat_get_entry_ref_locked(inj_stats_kind, 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..15621f49fd
--- /dev/null
+++ b/src/test/modules/injection_points/injection_stats.h
@@ -0,0 +1,22 @@
+/*--------------------------------------------------------------------------
+ *
+ * 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_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..526fbc1457 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'
diff --git a/src/test/modules/injection_points/sql/injection_points.sql b/src/test/modules/injection_points/sql/injection_points.sql
index 71e2972a7e..04cf7f48db 100644
--- a/src/test/modules/injection_points/sql/injection_points.sql
+++ b/src/test/modules/injection_points/sql/injection_points.sql
@@ -66,5 +66,20 @@ SELECT injection_points_detach('TestConditionError');
SELECT injection_points_attach('TestConditionLocal1', 'error');
SELECT injection_points_detach('TestConditionLocal1');
+-- Statistics
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- nothing
+SELECT injection_points_attach('TestConditionStats', 'notice');
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- returns 0
+SELECT injection_points_run('TestConditionStats');
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- returns 1
+-- Check transactions with stats snapshots
+BEGIN;
+SET stats_fetch_consistency TO snapshot;
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- returns 1
+SELECT injection_points_run('TestConditionStats');
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- still 1
+COMMIT;
+SELECT injection_points_stats_numcalls('TestConditionStats'); -- now 2
+SELECT injection_points_detach('TestConditionStats');
DROP EXTENSION injection_points;
DROP FUNCTION wait_pid;
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 8718ca54e0..29197fe212 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2119,6 +2119,7 @@ PgStatShared_Common
PgStatShared_Database
PgStatShared_Function
PgStatShared_HashEntry
+PgStatShared_InjectionPoint
PgStatShared_IO
PgStatShared_Relation
PgStatShared_ReplSlot
@@ -2150,6 +2151,7 @@ PgStat_Snapshot
PgStat_SnapshotEntry
PgStat_StatDBEntry
PgStat_StatFuncEntry
+PgStat_StatInjEntry
PgStat_StatReplSlotEntry
PgStat_StatSubEntry
PgStat_StatTabEntry
--
2.43.0
From 6b2c701f703febe129a444f43ca290b5ea6fcc7a Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 13 Jun 2024 13:37:33 +0900
Subject: [PATCH 5/6] 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 | 50 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index a7c170476a..e611ab233e 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3676,6 +3676,56 @@ 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 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_add_kind</literal>, that returns a unique ID
+ used to store the entries related to this type of statistics:
+<programlisting>
+extern PgStat_Kind pgstat_add_kind(const PgStat_KindInfo *kind_info);
+</programlisting>
+ </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 PgStat_KindInfo.
+ </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.43.0
From 1a096ea5bff6550815d81e80134a8e6b9aa44d6e Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 13 Jun 2024 15:54:32 +0900
Subject: [PATCH 6/6] Extend custom cumulative stats to be persistent
This patch uses the approach of a unique file, so as all the stats are
stored in the existing PGSTAT_STAT_PERMANENT_FILENAME, extending its
format with a new entry type to be able to read on load and write at
shutdown the custom kinds of statistics registered:
- The name of the custom stats kinds is stored into the stats file
rather than its ID.
- The rest of the hash key, made of the database OID and the object OID,
is stored if the custom kind is not serialized. If serialized, where
callbacks are used to do a mapping of the on-disk name <-> hash key, the
serialized name is pushed instead.
The patch includes an example of what can be achieved with the test
module injection_points, with a TAP test checking if statistics can be
kept after a clean restart.
---
src/include/pgstat.h | 2 +-
src/backend/utils/activity/pgstat.c | 96 ++++++++++++++++++-
src/backend/utils/adt/pgstatfuncs.c | 2 +-
src/test/modules/injection_points/Makefile | 4 +
.../injection_points/injection_points.c | 29 +++++-
.../injection_points/injection_stats.c | 54 ++++++++++-
.../injection_points/injection_stats.h | 4 +
src/test/modules/injection_points/meson.build | 8 ++
.../modules/injection_points/t/001_stats.pl | 50 ++++++++++
9 files changed, 240 insertions(+), 9 deletions(-)
create mode 100644 src/test/modules/injection_points/t/001_stats.pl
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index b3cdc0da6d..6c9084b977 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -487,7 +487,7 @@ extern void pgstat_clear_snapshot(void);
extern TimestampTz pgstat_get_stat_snapshot_timestamp(bool *have_snapshot);
/* helpers */
-extern PgStat_Kind pgstat_get_kind_from_str(char *kind_str);
+extern PgStat_Kind pgstat_get_kind_from_str(char *kind_str, int elevel);
extern bool pgstat_have_entry(PgStat_Kind kind, Oid dboid, Oid objoid);
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index b96743ce84..5ea7dbc64b 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -110,6 +110,7 @@
#include "storage/shmem.h"
#include "storage/spin.h"
#include "storage/s_lock.h"
+#include "utils/builtins.h"
#include "utils/guc_hooks.h"
#include "utils/hsearch.h"
#include "utils/memutils.h"
@@ -142,6 +143,7 @@
* Identifiers in stats file.
* ---------
*/
+#define PGSTAT_FILE_CUSTOM 'C' /* custom stats kind */
#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 */
@@ -1372,7 +1374,7 @@ pgstat_flush_pending_entries(bool nowait)
*/
PgStat_Kind
-pgstat_get_kind_from_str(char *kind_str)
+pgstat_get_kind_from_str(char *kind_str, int elevel)
{
for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++)
{
@@ -1392,10 +1394,10 @@ pgstat_get_kind_from_str(char *kind_str)
}
}
- ereport(ERROR,
+ ereport(elevel,
(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
@@ -1478,7 +1480,7 @@ pgstat_add_kind(const PgStat_KindInfo *kind_info)
if (strlen(kind_info->name) >= NAMEDATALEN)
elog(ERROR,
- "cannot use custom stats kind longer than %u characters",
+ "cannot define custom stats kind name longer than %u characters",
NAMEDATALEN - 1);
/*
@@ -1489,6 +1491,15 @@ pgstat_add_kind(const PgStat_KindInfo *kind_info)
elog(ERROR,
"cannot define custom stats kind with fixed amount of data");
+ /*
+ * Custom stats entries are represented on disk with their kind name
+ * and their entry name, so these are mandatory.
+ */
+ if (kind_info->to_serialized_name == NULL ||
+ kind_info->from_serialized_name == NULL)
+ elog(ERROR,
+ "cannot define custom stats kind without serialization callbacks");
+
/*
* Check if kind ID associated to the name is already defined, and return
* it if so.
@@ -1707,7 +1718,37 @@ pgstat_write_statsfile(void)
/* if not dropped the valid-entry refcount should exist */
Assert(pg_atomic_read_u32(&ps->refcount) > 0);
- if (!kind_info->to_serialized_name)
+ if (ps->key.kind > PGSTAT_KIND_LAST)
+ {
+ /*
+ * Custom stats entry, identified by kind name and entry name on
+ * disk.
+ */
+ NameData name;
+ NameData kind_name;
+
+ fputc(PGSTAT_FILE_CUSTOM, fpout);
+
+ /* Kind name */
+ namestrcpy(&kind_name, kind_info->name);
+ write_chunk_s(fpout, &kind_name);
+
+ /*
+ * If serialized use the on-disk format, or use the hash
+ * key components for the database OID and object OID.
+ */
+ if (kind_info->to_serialized_name)
+ {
+ kind_info->to_serialized_name(&ps->key, shstats, &name);
+ write_chunk_s(fpout, &name);
+ }
+ else
+ {
+ write_chunk_s(fpout, &ps->key.dboid);
+ write_chunk_s(fpout, &ps->key.objoid);
+ }
+ }
+ else if (!kind_info->to_serialized_name)
{
/* normal stats entry, identified by PgStat_HashKey */
fputc(PGSTAT_FILE_SYSTEM, fpout);
@@ -1874,6 +1915,7 @@ pgstat_read_statsfile(void)
switch (t)
{
+ case PGSTAT_FILE_CUSTOM:
case PGSTAT_FILE_SYSTEM:
case PGSTAT_FILE_NAME:
{
@@ -1892,6 +1934,50 @@ pgstat_read_statsfile(void)
if (!pgstat_is_kind_valid(key.kind))
goto error;
}
+ else if (t == PGSTAT_FILE_CUSTOM)
+ {
+ NameData kind_name;
+ NameData name;
+ PgStat_Kind kind;
+ const PgStat_KindInfo *kind_info = NULL;
+
+ /* First comes the name of the stats */
+ if (!read_chunk_s(fpin, &kind_name))
+ goto error;
+
+ /* Check if it is a valid stats kind */
+ kind = pgstat_get_kind_from_str(NameStr(kind_name),
+ WARNING);
+ if (!pgstat_is_kind_valid(kind))
+ goto error;
+
+ kind_info = pgstat_get_kind_info(kind);
+
+ /* Then comes the entry name, if serialized */
+ if (kind_info->from_serialized_name)
+ {
+ if (!read_chunk_s(fpin, &name))
+ goto error;
+
+ /* Compile its key */
+ if (!kind_info->from_serialized_name(&name, &key))
+ {
+ /* skip over data for entry we don't care about */
+ if (fseek(fpin, pgstat_get_entry_len(kind), SEEK_CUR) != 0)
+ goto error;
+ continue;
+ }
+ }
+ else
+ {
+ /* Extract the rest of the hash key */
+ key.kind = kind;
+ if (!read_chunk_s(fpin, &key.dboid))
+ goto error;
+ if (!read_chunk_s(fpin, &key.objoid))
+ goto error;
+ }
+ }
else
{
/* stats entry identified by name on disk (e.g. slots) */
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 3876339ee1..59ef463687 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -2028,7 +2028,7 @@ pg_stat_have_stats(PG_FUNCTION_ARGS)
char *stats_type = text_to_cstring(PG_GETARG_TEXT_P(0));
Oid dboid = PG_GETARG_OID(1);
Oid objoid = PG_GETARG_OID(2);
- PgStat_Kind kind = pgstat_get_kind_from_str(stats_type);
+ PgStat_Kind kind = pgstat_get_kind_from_str(stats_type, ERROR);
PG_RETURN_BOOL(pgstat_have_entry(kind, dboid, objoid));
}
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 676823a87f..9d84d9aaf3 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -12,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.c b/src/test/modules/injection_points/injection_points.c
index 6346af6cf6..f0c8c67826 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -34,9 +34,11 @@
PG_MODULE_MAGIC;
+/* Hooks */
+static shmem_startup_hook_type prev_shmem_startup_hook = NULL;
+
/* Maximum number of waits usable in injection points at once */
#define INJ_MAX_WAIT 8
-#define INJ_NAME_MAXLEN 64
/*
* Conditions related to injection points. This tracks in shared memory the
@@ -419,3 +421,28 @@ injection_points_detach(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+
+static void
+injection_points_shmem_startup(void)
+{
+ if (prev_shmem_startup_hook)
+ prev_shmem_startup_hook();
+
+ /*
+ * Note that this does not call injection_init_shmem(), as it relies
+ * on the DSM registry, which cannot be used in the postmaster context.
+ */
+
+ /* Register custom statistics */
+ pgstat_register_inj();
+}
+
+void
+_PG_init(void)
+{
+ if (!process_shared_preload_libraries_in_progress)
+ return;
+
+ prev_shmem_startup_hook = shmem_startup_hook;
+ shmem_startup_hook = injection_points_shmem_startup;
+}
diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c
index 6314486d79..3acbccd32a 100644
--- a/src/test/modules/injection_points/injection_stats.c
+++ b/src/test/modules/injection_points/injection_stats.c
@@ -22,9 +22,13 @@
#include "utils/builtins.h"
#include "utils/pgstat_internal.h"
+/* Maximum name length */
+#define INJ_NAME_MAXLEN 64
+
/* Structures for statistics of injection points */
typedef struct PgStat_StatInjEntry
{
+ char name[INJ_NAME_MAXLEN]; /* used for serialization */
PgStat_Counter numcalls; /* number of times point has been run */
} PgStat_StatInjEntry;
@@ -35,6 +39,11 @@ typedef struct PgStatShared_InjectionPoint
} PgStatShared_InjectionPoint;
static bool injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
+static void injection_stats_to_serialized_name_cb(const PgStat_HashKey *key,
+ const PgStatShared_Common *header,
+ NameData *name);
+static bool injection_stats_from_serialized_name_cb(const NameData *name,
+ PgStat_HashKey *key);
static const PgStat_KindInfo injection_stats = {
.name = "injection_points",
@@ -48,6 +57,8 @@ static const PgStat_KindInfo injection_stats = {
.shared_data_len = sizeof(((PgStatShared_InjectionPoint *) 0)->stats),
.pending_size = sizeof(PgStat_StatInjEntry),
.flush_pending_cb = injection_stats_flush_cb,
+ .to_serialized_name = injection_stats_to_serialized_name_cb,
+ .from_serialized_name = injection_stats_from_serialized_name_cb,
};
/*
@@ -58,7 +69,7 @@ static const PgStat_KindInfo injection_stats = {
static PgStat_Kind inj_stats_kind = PGSTAT_KIND_INVALID;
/*
- * Callback for stats handling
+ * Callbacks for stats handling
*/
static bool
injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
@@ -76,6 +87,35 @@ injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
return true;
}
+/*
+ * Transforms a hash key into a name that is then stored on disk.
+ */
+static void
+injection_stats_to_serialized_name_cb(const PgStat_HashKey *key,
+ const PgStatShared_Common *header,
+ NameData *name)
+{
+ PgStatShared_InjectionPoint *shstatent =
+ (PgStatShared_InjectionPoint *) header;
+
+ namestrcpy(name, shstatent->stats.name);
+}
+
+/*
+ * Computes the hash key used for this injection point name.
+ */
+static bool
+injection_stats_from_serialized_name_cb(const NameData *name,
+ PgStat_HashKey *key)
+{
+ uint32 idx = PGSTAT_INJ_IDX(NameStr(*name));
+
+ key->kind = inj_stats_kind;
+ key->dboid = InvalidOid;
+ key->objoid = idx;
+ return true;
+}
+
/*
* Support function for the SQL-callable pgstat* functions. Returns
* a pointer to the injection point statistics struct.
@@ -95,6 +135,16 @@ pgstat_fetch_stat_injentry(const char *name)
return entry;
}
+/*
+ * Workhorse to do the registration work, called in _PG_init().
+ */
+void
+pgstat_register_inj(void)
+{
+ if (inj_stats_kind == PGSTAT_KIND_INVALID)
+ inj_stats_kind = pgstat_add_kind(&injection_stats);
+}
+
/*
* Report injection point creation.
*/
@@ -113,6 +163,8 @@ pgstat_create_inj(const char *name)
/* initialize shared memory data */
memset(&shstatent->stats, 0, sizeof(shstatent->stats));
+ strlcpy(shstatent->stats.name, name, INJ_NAME_MAXLEN);
+
pgstat_unlock_entry(entry_ref);
}
diff --git a/src/test/modules/injection_points/injection_stats.h b/src/test/modules/injection_points/injection_stats.h
index 15621f49fd..b59ac4cf1c 100644
--- a/src/test/modules/injection_points/injection_stats.h
+++ b/src/test/modules/injection_points/injection_stats.h
@@ -15,6 +15,10 @@
#ifndef INJECTION_STATS
#define INJECTION_STATS
+/* Maximum name length */
+#define INJ_NAME_MAXLEN 64
+
+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);
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index 526fbc1457..612216c144 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -38,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();
--
2.43.0
signature.asc
Description: PGP signature
