On Fri, Jun 21, 2024 at 08:13:15AM +0900, Michael Paquier wrote: > On Thu, Jun 20, 2024 at 02:27:14PM +0000, Bertrand Drouvot wrote: >> On Thu, Jun 20, 2024 at 09:46:30AM +0900, Michael Paquier wrote: >> I think it makes sense to follow the same "behavior" as the custom >> wal resource managers. That, indeed, looks much more simpler than v1. > > Thanks for the feedback.
While looking at a different patch from Tristan in this area at [1], I still got annoyed that this patch set was not able to support the case of custom fixed-numbered stats, so as it is possible to plug in pgstats things similar to the archiver, the checkpointer, WAL, etc. These are plugged in shared memory, and are handled with copies in the stats snapshots. After a good night of sleep, I have come up with a good solution for that, among the following lines: - PgStat_ShmemControl holds an array of void* indexed by PGSTAT_NUM_KINDS, pointing to shared memory areas allocated for each fixed-numbered stats. Each entry is allocated a size corresponding to PgStat_KindInfo->shared_size. - PgStat_Snapshot holds an array of void* also indexed by PGSTAT_NUM_KINDS, pointing to the fixed stats stored in the snapshots. These have a size of PgStat_KindInfo->shared_data_len, set up when stats are initialized at process startup, so this reflects everywhere. - Fixed numbered stats now set shared_size, and we use this number to determine the size to allocate for each fixed-numbered stats in shmem. - A callback is added to initialize the shared memory assigned to each fixed-numbered stats, consisting of LWLock initializations for the current types of stats. So this initialization step is moved out of pgstat.c into each stats kind file. All that has been done in the rebased patch set as of 0001, which is kind of a nice cleanup overall because it removes all the dependencies to the fixed-numbered stats structures from the "main" pgstats code in pgstat.c and pgstat_shmem.c. The remaining patches consist of: - 0002, Switch PgStat_Kind to a uint32. Cleanup. - 0003 introduces the pluggable stats facility. Feeding on the refactoring for the fixed-numbered stats in 0001, it is actually possible to get support for these in the pluggable APIs by just removing the restriction in the registration path. This extends the void* arrays to store references that cover the range of custom kind IDs. - 0004 has some docs. - 0005 includes an example of implementation for variable-numbered stats with the injection_points module. - 0006 is new for this thread, implementing an example for fixed-numbered stats, using again the injection_points module. This stuff gathers stats about the number of times points are run, attached and detached. Perhaps that's useful in itself, I don't know, but it provides the coverage I want for this facility. While on it, I have applied one of the cleanup patches as 9fd02525793f. [1]: https://www.postgresql.org/message-id/zonytpohozhgb...@paquier.xyz -- Michael
From 596424c30af63e96f3ab4274541ab6cb6cfaab0e Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Wed, 3 Jul 2024 13:26:22 +0900 Subject: [PATCH v3 1/6] Rework handling of fixed-numbered statistics in pgstats This commit removes fixed-numbered statistics structures from PgStat_ShmemControl, the central shared memory structure of pgstats and PgStat_Snapshot, in charge of holding stats snapshots for backends, based on copies of what is in shared memory. This has the advantage to simplify the read and write of the pgstats file by not tracking the types of fixed-numbered stats in pgstat.c itself, replacing it with what PgStat_KindInfo already knows for each one of them. This refactoring makes possible the introduction of more pluggable APIs into pgstats for fixed-numbered stats. The following changes are applied to the internal structures: - PgStat_ShmemControl holds an array of void* indexed by PGSTAT_NUM_KINDS, pointing to shared memory areas allocated for each fixed-numbered stats. Each entry is allocated a size corresponding to PgStat_KindInfo->shared_size. - PgStat_Snapshot holds an array of void* also indexed by PGSTAT_NUM_KINDS, pointing to the fixed stats stored in the snapshots. These have a size of PgStat_KindInfo->shared_data_len. - Fixed numbered stats now set shared_size, so as - A callback is added to initialize the shared memory assigned to each fixed-numbered stats, consisting of LWLock initializations for the current types of stats. So this initialization is moved out of pgstat.c into each stats kind file. --- src/include/utils/pgstat_internal.h | 52 +++---- src/backend/utils/activity/pgstat.c | 130 ++++++++++-------- src/backend/utils/activity/pgstat_archiver.c | 23 +++- src/backend/utils/activity/pgstat_bgwriter.c | 26 +++- .../utils/activity/pgstat_checkpointer.c | 26 +++- src/backend/utils/activity/pgstat_io.c | 44 ++++-- src/backend/utils/activity/pgstat_shmem.c | 33 +++-- src/backend/utils/activity/pgstat_slru.c | 25 +++- src/backend/utils/activity/pgstat_wal.c | 26 +++- 9 files changed, 250 insertions(+), 135 deletions(-) diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index e21ef4e2c9..b8b2152d71 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -195,16 +195,11 @@ 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->fixed_data. */ uint32 shared_size; - /* - * The offset of the statistics struct in the containing shared memory - * control structure PgStat_ShmemControl, for fixed-numbered statistics. - */ - uint32 shared_ctl_off; - /* * The offset/size of statistics inside the shared stats entry. Used when * [de-]serializing statistics to / from disk respectively. Separate from @@ -245,6 +240,13 @@ typedef struct PgStat_KindInfo const PgStatShared_Common *header, NameData *name); bool (*from_serialized_name) (const NameData *name, PgStat_HashKey *key); + /* + * For fixed-numbered statistics: Initialize shared memory state. + * + * "stats" is the pointer to the allocated shared memory area. + */ + void (*init_shmem_cb) (void *stats); + /* * For fixed-numbered statistics: Reset All. */ @@ -425,14 +427,12 @@ typedef struct PgStat_ShmemControl pg_atomic_uint64 gc_request_count; /* - * Stats data for fixed-numbered objects. + * Stats data for fixed-numbered objects, indexed by PgStat_Kind. + * + * Each entry has a size of PgStat_KindInfo->shared_size. */ - PgStatShared_Archiver archiver; - PgStatShared_BgWriter bgwriter; - PgStatShared_Checkpointer checkpointer; - PgStatShared_IO io; - PgStatShared_SLRU slru; - PgStatShared_Wal wal; + void *fixed_data[PGSTAT_NUM_KINDS]; + } PgStat_ShmemControl; @@ -446,19 +446,13 @@ typedef struct PgStat_Snapshot /* time at which snapshot was taken */ TimestampTz snapshot_timestamp; + /* + * Data in snapshot for fixed-numbered statistics, indexed by PgStat_Kind. + * Each entry is allocated in TopMemoryContext, for a size of + * shared_data_len. + */ bool fixed_valid[PGSTAT_NUM_KINDS]; - - PgStat_ArchiverStats archiver; - - PgStat_BgWriterStats bgwriter; - - PgStat_CheckpointerStats checkpointer; - - PgStat_IO io; - - PgStat_SLRUStats slru[SLRU_NUM_ELEMENTS]; - - PgStat_WalStats wal; + void *fixed_data[PGSTAT_NUM_KINDS]; /* to free snapshot in bulk */ MemoryContext context; @@ -522,6 +516,7 @@ extern void pgstat_snapshot_fixed(PgStat_Kind kind); * Functions in pgstat_archiver.c */ +extern void pgstat_archiver_init_shmem_cb(void *stats); extern void pgstat_archiver_reset_all_cb(TimestampTz ts); extern void pgstat_archiver_snapshot_cb(void); @@ -530,6 +525,7 @@ extern void pgstat_archiver_snapshot_cb(void); * Functions in pgstat_bgwriter.c */ +extern void pgstat_bgwriter_init_shmem_cb(void *stats); extern void pgstat_bgwriter_reset_all_cb(TimestampTz ts); extern void pgstat_bgwriter_snapshot_cb(void); @@ -538,6 +534,7 @@ extern void pgstat_bgwriter_snapshot_cb(void); * Functions in pgstat_checkpointer.c */ +extern void pgstat_checkpointer_init_shmem_cb(void *stats); extern void pgstat_checkpointer_reset_all_cb(TimestampTz ts); extern void pgstat_checkpointer_snapshot_cb(void); @@ -568,6 +565,7 @@ extern bool pgstat_function_flush_cb(PgStat_EntryRef *entry_ref, bool nowait); */ extern bool pgstat_flush_io(bool nowait); +extern void pgstat_io_init_shmem_cb(void *stats); extern void pgstat_io_reset_all_cb(TimestampTz ts); extern void pgstat_io_snapshot_cb(void); @@ -626,6 +624,7 @@ extern PgStatShared_Common *pgstat_init_entry(PgStat_Kind kind, */ extern bool pgstat_slru_flush(bool nowait); +extern void pgstat_slru_init_shmem_cb(void *stats); extern void pgstat_slru_reset_all_cb(TimestampTz ts); extern void pgstat_slru_snapshot_cb(void); @@ -638,6 +637,7 @@ extern bool pgstat_flush_wal(bool nowait); extern void pgstat_init_wal(void); extern bool pgstat_have_pending_wal(void); +extern void pgstat_wal_init_shmem_cb(void *stats); extern void pgstat_wal_reset_all_cb(TimestampTz ts); extern void pgstat_wal_snapshot_cb(void); diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index c37c11b2ec..13ddbcdcfb 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -132,6 +132,7 @@ * --------- */ #define PGSTAT_FILE_ENTRY_END 'E' /* end of file */ +#define PGSTAT_FILE_ENTRY_FIXED 'F' /* fixed-numbered stats entry */ #define PGSTAT_FILE_ENTRY_NAME 'N' /* stats entry identified by name */ #define PGSTAT_FILE_ENTRY_HASH 'S' /* stats entry identified by * PgStat_HashKey */ @@ -173,6 +174,8 @@ typedef struct PgStat_SnapshotEntry static void pgstat_write_statsfile(void); static void pgstat_read_statsfile(void); +static void pgstat_init_snapshot(void); + static void pgstat_reset_after_failure(void); static bool pgstat_flush_pending_entries(bool nowait); @@ -349,10 +352,11 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = { .fixed_amount = true, - .shared_ctl_off = offsetof(PgStat_ShmemControl, archiver), + .shared_size = sizeof(PgStatShared_Archiver), .shared_data_off = offsetof(PgStatShared_Archiver, stats), .shared_data_len = sizeof(((PgStatShared_Archiver *) 0)->stats), + .init_shmem_cb = pgstat_archiver_init_shmem_cb, .reset_all_cb = pgstat_archiver_reset_all_cb, .snapshot_cb = pgstat_archiver_snapshot_cb, }, @@ -362,10 +366,11 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = { .fixed_amount = true, - .shared_ctl_off = offsetof(PgStat_ShmemControl, bgwriter), + .shared_size = sizeof(PgStatShared_BgWriter), .shared_data_off = offsetof(PgStatShared_BgWriter, stats), .shared_data_len = sizeof(((PgStatShared_BgWriter *) 0)->stats), + .init_shmem_cb = pgstat_bgwriter_init_shmem_cb, .reset_all_cb = pgstat_bgwriter_reset_all_cb, .snapshot_cb = pgstat_bgwriter_snapshot_cb, }, @@ -375,10 +380,11 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = { .fixed_amount = true, - .shared_ctl_off = offsetof(PgStat_ShmemControl, checkpointer), + .shared_size = sizeof(PgStatShared_Checkpointer), .shared_data_off = offsetof(PgStatShared_Checkpointer, stats), .shared_data_len = sizeof(((PgStatShared_Checkpointer *) 0)->stats), + .init_shmem_cb = pgstat_checkpointer_init_shmem_cb, .reset_all_cb = pgstat_checkpointer_reset_all_cb, .snapshot_cb = pgstat_checkpointer_snapshot_cb, }, @@ -388,10 +394,11 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = { .fixed_amount = true, - .shared_ctl_off = offsetof(PgStat_ShmemControl, io), + .shared_size = sizeof(PgStatShared_IO), .shared_data_off = offsetof(PgStatShared_IO, stats), .shared_data_len = sizeof(((PgStatShared_IO *) 0)->stats), + .init_shmem_cb = pgstat_io_init_shmem_cb, .reset_all_cb = pgstat_io_reset_all_cb, .snapshot_cb = pgstat_io_snapshot_cb, }, @@ -401,10 +408,11 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = { .fixed_amount = true, - .shared_ctl_off = offsetof(PgStat_ShmemControl, slru), + .shared_size = sizeof(PgStatShared_SLRU), .shared_data_off = offsetof(PgStatShared_SLRU, stats), .shared_data_len = sizeof(((PgStatShared_SLRU *) 0)->stats), + .init_shmem_cb = pgstat_slru_init_shmem_cb, .reset_all_cb = pgstat_slru_reset_all_cb, .snapshot_cb = pgstat_slru_snapshot_cb, }, @@ -414,10 +422,11 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = { .fixed_amount = true, - .shared_ctl_off = offsetof(PgStat_ShmemControl, wal), + .shared_size = sizeof(PgStatShared_Wal), .shared_data_off = offsetof(PgStatShared_Wal, stats), .shared_data_len = sizeof(((PgStatShared_Wal *) 0)->stats), + .init_shmem_cb = pgstat_wal_init_shmem_cb, .reset_all_cb = pgstat_wal_reset_all_cb, .snapshot_cb = pgstat_wal_snapshot_cb, }, @@ -571,6 +580,8 @@ pgstat_initialize(void) pgstat_attach_shmem(); + pgstat_init_snapshot(); + pgstat_init_wal(); /* Set up a process-exit hook to clean up */ @@ -982,6 +993,22 @@ pgstat_snapshot_fixed(PgStat_Kind kind) Assert(pgStatLocal.snapshot.fixed_valid[kind]); } +static void +pgstat_init_snapshot(void) +{ + /* Initialize fixed-numbered statistics data in snapshots */ + for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + { + const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); + + if (!kind_info->fixed_amount) + continue; + + pgStatLocal.snapshot.fixed_data[kind] = + MemoryContextAlloc(TopMemoryContext, kind_info->shared_data_len); + } +} + static void pgstat_prep_snapshot(void) { @@ -1371,47 +1398,25 @@ pgstat_write_statsfile(void) format_id = PGSTAT_FILE_FORMAT_ID; write_chunk_s(fpout, &format_id); - /* - * XXX: The following could now be generalized to just iterate over - * pgstat_kind_infos instead of knowing about the different kinds of - * stats. - */ + /* Write various stats structs with fixed number of objects */ + for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + { + char *ptr; + const PgStat_KindInfo *info = pgstat_get_kind_info(kind); - /* - * Write archiver stats struct - */ - pgstat_build_snapshot_fixed(PGSTAT_KIND_ARCHIVER); - write_chunk_s(fpout, &pgStatLocal.snapshot.archiver); + if (!info->fixed_amount) + continue; - /* - * Write bgwriter stats struct - */ - pgstat_build_snapshot_fixed(PGSTAT_KIND_BGWRITER); - write_chunk_s(fpout, &pgStatLocal.snapshot.bgwriter); + Assert(info->shared_size != 0 && info->shared_data_len != 0); - /* - * Write checkpointer stats struct - */ - pgstat_build_snapshot_fixed(PGSTAT_KIND_CHECKPOINTER); - write_chunk_s(fpout, &pgStatLocal.snapshot.checkpointer); + /* prepare snapshot data and write it */ + pgstat_build_snapshot_fixed(kind); + ptr = pgStatLocal.snapshot.fixed_data[kind]; - /* - * Write IO stats struct - */ - pgstat_build_snapshot_fixed(PGSTAT_KIND_IO); - write_chunk_s(fpout, &pgStatLocal.snapshot.io); - - /* - * Write SLRU stats struct - */ - pgstat_build_snapshot_fixed(PGSTAT_KIND_SLRU); - write_chunk_s(fpout, &pgStatLocal.snapshot.slru); - - /* - * Write WAL stats struct - */ - pgstat_build_snapshot_fixed(PGSTAT_KIND_WAL); - write_chunk_s(fpout, &pgStatLocal.snapshot.wal); + fputc(PGSTAT_FILE_ENTRY_FIXED, fpout); + write_chunk_s(fpout, &kind); + write_chunk(fpout, ptr, info->shared_data_len); + } /* * Walk through the stats entries @@ -1551,22 +1556,6 @@ pgstat_read_statsfile(void) format_id != PGSTAT_FILE_FORMAT_ID) goto error; - /* Read various stats structs with fixed number of objects */ - for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) - { - char *ptr; - const PgStat_KindInfo *info = pgstat_get_kind_info(kind); - - if (!info->fixed_amount) - continue; - - Assert(info->shared_ctl_off != 0); - - ptr = ((char *) shmem) + info->shared_ctl_off + info->shared_data_off; - if (!read_chunk(fpin, ptr, info->shared_data_len)) - goto error; - } - /* * We found an existing statistics file. Read it and put all the hash * table entries into place. @@ -1577,6 +1566,29 @@ pgstat_read_statsfile(void) switch (t) { + case PGSTAT_FILE_ENTRY_FIXED: + { + PgStat_Kind kind; + const PgStat_KindInfo *info; + char *ptr; + + if (!read_chunk_s(fpin, &kind)) + goto error; + + if (!pgstat_is_kind_valid(kind)) + goto error; + + info = pgstat_get_kind_info(kind); + Assert(info->fixed_amount); + + /* Load back stats into shared memory */ + ptr = ((char *) shmem->fixed_data[kind]) + info->shared_data_off; + if (!read_chunk(fpin, ptr, + info->shared_data_len)) + goto error; + + break; + } case PGSTAT_FILE_ENTRY_HASH: case PGSTAT_FILE_ENTRY_NAME: { diff --git a/src/backend/utils/activity/pgstat_archiver.c b/src/backend/utils/activity/pgstat_archiver.c index 66398b20e5..99cd461f81 100644 --- a/src/backend/utils/activity/pgstat_archiver.c +++ b/src/backend/utils/activity/pgstat_archiver.c @@ -27,7 +27,8 @@ void pgstat_report_archiver(const char *xlog, bool failed) { - PgStatShared_Archiver *stats_shmem = &pgStatLocal.shmem->archiver; + PgStatShared_Archiver *stats_shmem = (PgStatShared_Archiver *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_ARCHIVER]; TimestampTz now = GetCurrentTimestamp(); pgstat_begin_changecount_write(&stats_shmem->changecount); @@ -59,13 +60,23 @@ pgstat_fetch_stat_archiver(void) { pgstat_snapshot_fixed(PGSTAT_KIND_ARCHIVER); - return &pgStatLocal.snapshot.archiver; + return (PgStat_ArchiverStats *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_ARCHIVER]; +} + +void +pgstat_archiver_init_shmem_cb(void *stats) +{ + PgStatShared_Archiver *stats_shmem = (PgStatShared_Archiver *) stats; + + LWLockInitialize(&stats_shmem->lock, LWTRANCHE_PGSTATS_DATA); } void pgstat_archiver_reset_all_cb(TimestampTz ts) { - PgStatShared_Archiver *stats_shmem = &pgStatLocal.shmem->archiver; + PgStatShared_Archiver *stats_shmem = (PgStatShared_Archiver *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_ARCHIVER]; /* see explanation above PgStatShared_Archiver for the reset protocol */ LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE); @@ -80,8 +91,10 @@ pgstat_archiver_reset_all_cb(TimestampTz ts) void pgstat_archiver_snapshot_cb(void) { - PgStatShared_Archiver *stats_shmem = &pgStatLocal.shmem->archiver; - PgStat_ArchiverStats *stat_snap = &pgStatLocal.snapshot.archiver; + PgStatShared_Archiver *stats_shmem = (PgStatShared_Archiver *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_ARCHIVER]; + PgStat_ArchiverStats *stat_snap = (PgStat_ArchiverStats *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_ARCHIVER]; PgStat_ArchiverStats *reset_offset = &stats_shmem->reset_offset; PgStat_ArchiverStats reset; diff --git a/src/backend/utils/activity/pgstat_bgwriter.c b/src/backend/utils/activity/pgstat_bgwriter.c index 7d2432e4fa..778d3f0b0e 100644 --- a/src/backend/utils/activity/pgstat_bgwriter.c +++ b/src/backend/utils/activity/pgstat_bgwriter.c @@ -29,7 +29,8 @@ PgStat_BgWriterStats PendingBgWriterStats = {0}; void pgstat_report_bgwriter(void) { - PgStatShared_BgWriter *stats_shmem = &pgStatLocal.shmem->bgwriter; + PgStatShared_BgWriter *stats_shmem = (PgStatShared_BgWriter *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_BGWRITER]; static const PgStat_BgWriterStats all_zeroes; Assert(!pgStatLocal.shmem->is_shutdown); @@ -72,13 +73,23 @@ pgstat_fetch_stat_bgwriter(void) { pgstat_snapshot_fixed(PGSTAT_KIND_BGWRITER); - return &pgStatLocal.snapshot.bgwriter; + return (PgStat_BgWriterStats *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_BGWRITER]; +} + +void +pgstat_bgwriter_init_shmem_cb(void *stats) +{ + PgStatShared_BgWriter *stats_shmem = (PgStatShared_BgWriter *) stats; + + LWLockInitialize(&stats_shmem->lock, LWTRANCHE_PGSTATS_DATA); } void pgstat_bgwriter_reset_all_cb(TimestampTz ts) { - PgStatShared_BgWriter *stats_shmem = &pgStatLocal.shmem->bgwriter; + PgStatShared_BgWriter *stats_shmem = (PgStatShared_BgWriter *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_BGWRITER]; /* see explanation above PgStatShared_BgWriter for the reset protocol */ LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE); @@ -93,11 +104,14 @@ pgstat_bgwriter_reset_all_cb(TimestampTz ts) void pgstat_bgwriter_snapshot_cb(void) { - PgStatShared_BgWriter *stats_shmem = &pgStatLocal.shmem->bgwriter; + PgStatShared_BgWriter *stats_shmem = (PgStatShared_BgWriter *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_BGWRITER]; + PgStat_BgWriterStats *stat_snap = (PgStat_BgWriterStats *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_BGWRITER]; PgStat_BgWriterStats *reset_offset = &stats_shmem->reset_offset; PgStat_BgWriterStats reset; - pgstat_copy_changecounted_stats(&pgStatLocal.snapshot.bgwriter, + pgstat_copy_changecounted_stats(stat_snap, &stats_shmem->stats, sizeof(stats_shmem->stats), &stats_shmem->changecount); @@ -107,7 +121,7 @@ pgstat_bgwriter_snapshot_cb(void) LWLockRelease(&stats_shmem->lock); /* compensate by reset offsets */ -#define BGWRITER_COMP(fld) pgStatLocal.snapshot.bgwriter.fld -= reset.fld; +#define BGWRITER_COMP(fld) stat_snap->fld -= reset.fld; BGWRITER_COMP(buf_written_clean); BGWRITER_COMP(maxwritten_clean); BGWRITER_COMP(buf_alloc); diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c index 30a8110e38..6e86255b8d 100644 --- a/src/backend/utils/activity/pgstat_checkpointer.c +++ b/src/backend/utils/activity/pgstat_checkpointer.c @@ -31,7 +31,8 @@ pgstat_report_checkpointer(void) { /* We assume this initializes to zeroes */ static const PgStat_CheckpointerStats all_zeroes; - PgStatShared_Checkpointer *stats_shmem = &pgStatLocal.shmem->checkpointer; + PgStatShared_Checkpointer *stats_shmem = (PgStatShared_Checkpointer *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_CHECKPOINTER]; Assert(!pgStatLocal.shmem->is_shutdown); pgstat_assert_is_up(); @@ -81,13 +82,23 @@ pgstat_fetch_stat_checkpointer(void) { pgstat_snapshot_fixed(PGSTAT_KIND_CHECKPOINTER); - return &pgStatLocal.snapshot.checkpointer; + return (PgStat_CheckpointerStats *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_CHECKPOINTER]; +} + +void +pgstat_checkpointer_init_shmem_cb(void *stats) +{ + PgStatShared_Checkpointer *stats_shmem = (PgStatShared_Checkpointer *) stats; + + LWLockInitialize(&stats_shmem->lock, LWTRANCHE_PGSTATS_DATA); } void pgstat_checkpointer_reset_all_cb(TimestampTz ts) { - PgStatShared_Checkpointer *stats_shmem = &pgStatLocal.shmem->checkpointer; + PgStatShared_Checkpointer *stats_shmem = (PgStatShared_Checkpointer *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_CHECKPOINTER]; /* see explanation above PgStatShared_Checkpointer for the reset protocol */ LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE); @@ -102,11 +113,14 @@ pgstat_checkpointer_reset_all_cb(TimestampTz ts) void pgstat_checkpointer_snapshot_cb(void) { - PgStatShared_Checkpointer *stats_shmem = &pgStatLocal.shmem->checkpointer; + PgStatShared_Checkpointer *stats_shmem = (PgStatShared_Checkpointer *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_CHECKPOINTER]; + PgStat_CheckpointerStats *stat_snap = (PgStat_CheckpointerStats *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_CHECKPOINTER]; PgStat_CheckpointerStats *reset_offset = &stats_shmem->reset_offset; PgStat_CheckpointerStats reset; - pgstat_copy_changecounted_stats(&pgStatLocal.snapshot.checkpointer, + pgstat_copy_changecounted_stats(stat_snap, &stats_shmem->stats, sizeof(stats_shmem->stats), &stats_shmem->changecount); @@ -116,7 +130,7 @@ pgstat_checkpointer_snapshot_cb(void) LWLockRelease(&stats_shmem->lock); /* compensate by reset offsets */ -#define CHECKPOINTER_COMP(fld) pgStatLocal.snapshot.checkpointer.fld -= reset.fld; +#define CHECKPOINTER_COMP(fld) stat_snap->fld -= reset.fld; CHECKPOINTER_COMP(num_timed); CHECKPOINTER_COMP(num_requested); CHECKPOINTER_COMP(restartpoints_timed); diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c index 9d6e067382..6cbbc2094a 100644 --- a/src/backend/utils/activity/pgstat_io.c +++ b/src/backend/utils/activity/pgstat_io.c @@ -158,7 +158,8 @@ pgstat_fetch_stat_io(void) { pgstat_snapshot_fixed(PGSTAT_KIND_IO); - return &pgStatLocal.snapshot.io; + return (PgStat_IO *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_IO]; } /* @@ -174,13 +175,16 @@ pgstat_flush_io(bool nowait) { LWLock *bktype_lock; PgStat_BktypeIO *bktype_shstats; + PgStatShared_IO *stat_shmem; if (!have_iostats) return false; - bktype_lock = &pgStatLocal.shmem->io.locks[MyBackendType]; - bktype_shstats = - &pgStatLocal.shmem->io.stats.stats[MyBackendType]; + stat_shmem = (PgStatShared_IO *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_IO]; + + bktype_lock = &stat_shmem->locks[MyBackendType]; + bktype_shstats = &stat_shmem->stats.stats[MyBackendType]; if (!nowait) LWLockAcquire(bktype_lock, LW_EXCLUSIVE); @@ -251,13 +255,25 @@ pgstat_get_io_object_name(IOObject io_object) pg_unreachable(); } +void +pgstat_io_init_shmem_cb(void *stats) +{ + PgStatShared_IO *stat_shmem = (PgStatShared_IO *) stats; + + for (int i = 0; i < BACKEND_NUM_TYPES; i++) + LWLockInitialize(&stat_shmem->locks[i], LWTRANCHE_PGSTATS_DATA); +} + void pgstat_io_reset_all_cb(TimestampTz ts) { + PgStatShared_IO *stat_shmem = (PgStatShared_IO *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_IO]; + for (int i = 0; i < BACKEND_NUM_TYPES; i++) { - LWLock *bktype_lock = &pgStatLocal.shmem->io.locks[i]; - PgStat_BktypeIO *bktype_shstats = &pgStatLocal.shmem->io.stats.stats[i]; + LWLock *bktype_lock = &stat_shmem->locks[i]; + PgStat_BktypeIO *bktype_shstats = &stat_shmem->stats.stats[i]; LWLockAcquire(bktype_lock, LW_EXCLUSIVE); @@ -266,7 +282,7 @@ pgstat_io_reset_all_cb(TimestampTz ts) * the reset timestamp as well. */ if (i == 0) - pgStatLocal.shmem->io.stats.stat_reset_timestamp = ts; + stat_shmem->stats.stat_reset_timestamp = ts; memset(bktype_shstats, 0, sizeof(*bktype_shstats)); LWLockRelease(bktype_lock); @@ -276,11 +292,16 @@ pgstat_io_reset_all_cb(TimestampTz ts) void pgstat_io_snapshot_cb(void) { + PgStatShared_IO *stat_shmem = (PgStatShared_IO *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_IO]; + PgStat_IO *stat_snap = (PgStat_IO *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_IO]; + for (int i = 0; i < BACKEND_NUM_TYPES; i++) { - LWLock *bktype_lock = &pgStatLocal.shmem->io.locks[i]; - PgStat_BktypeIO *bktype_shstats = &pgStatLocal.shmem->io.stats.stats[i]; - PgStat_BktypeIO *bktype_snap = &pgStatLocal.snapshot.io.stats[i]; + LWLock *bktype_lock = &stat_shmem->locks[i]; + PgStat_BktypeIO *bktype_shstats = &stat_shmem->stats.stats[i]; + PgStat_BktypeIO *bktype_snap = &stat_snap->stats[i]; LWLockAcquire(bktype_lock, LW_SHARED); @@ -289,8 +310,7 @@ pgstat_io_snapshot_cb(void) * the reset timestamp as well. */ if (i == 0) - pgStatLocal.snapshot.io.stat_reset_timestamp = - pgStatLocal.shmem->io.stats.stat_reset_timestamp; + stat_snap->stat_reset_timestamp = stat_shmem->stats.stat_reset_timestamp; /* using struct assignment due to better type safety */ *bktype_snap = *bktype_shstats; diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 634b967820..bd62c4b72a 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -131,6 +131,19 @@ StatsShmemSize(void) sz = MAXALIGN(sizeof(PgStat_ShmemControl)); sz = add_size(sz, pgstat_dsa_init_size()); + /* Add shared memory for all the fixed-numbered statistics */ + for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + { + const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); + + if (!kind_info->fixed_amount) + continue; + + Assert(kind_info->shared_size != 0); + + sz += MAXALIGN(kind_info->shared_size); + } + return sz; } @@ -196,17 +209,19 @@ StatsShmemInit(void) pg_atomic_init_u64(&ctl->gc_request_count, 1); + /* initialize fixed-numbered statistics */ + for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + { + const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); - /* initialize fixed-numbered stats */ - LWLockInitialize(&ctl->archiver.lock, LWTRANCHE_PGSTATS_DATA); - LWLockInitialize(&ctl->bgwriter.lock, LWTRANCHE_PGSTATS_DATA); - LWLockInitialize(&ctl->checkpointer.lock, LWTRANCHE_PGSTATS_DATA); - LWLockInitialize(&ctl->slru.lock, LWTRANCHE_PGSTATS_DATA); - LWLockInitialize(&ctl->wal.lock, LWTRANCHE_PGSTATS_DATA); + if (!kind_info->fixed_amount) + continue; - for (int i = 0; i < BACKEND_NUM_TYPES; i++) - LWLockInitialize(&ctl->io.locks[i], - LWTRANCHE_PGSTATS_DATA); + Assert(kind_info->shared_size != 0); + + ctl->fixed_data[kind] = ShmemAlloc(kind_info->shared_size); + kind_info->init_shmem_cb(ctl->fixed_data[kind]); + } } else { diff --git a/src/backend/utils/activity/pgstat_slru.c b/src/backend/utils/activity/pgstat_slru.c index 56ea1c3378..d50589c1de 100644 --- a/src/backend/utils/activity/pgstat_slru.c +++ b/src/backend/utils/activity/pgstat_slru.c @@ -106,7 +106,8 @@ pgstat_fetch_slru(void) { pgstat_snapshot_fixed(PGSTAT_KIND_SLRU); - return pgStatLocal.snapshot.slru; + return (PgStat_SLRUStats *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_SLRU]; } /* @@ -155,7 +156,8 @@ pgstat_get_slru_index(const char *name) bool pgstat_slru_flush(bool nowait) { - PgStatShared_SLRU *stats_shmem = &pgStatLocal.shmem->slru; + PgStatShared_SLRU *stats_shmem = (PgStatShared_SLRU *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_SLRU]; int i; if (!have_slrustats) @@ -192,6 +194,14 @@ pgstat_slru_flush(bool nowait) return false; } +void +pgstat_slru_init_shmem_cb(void *stats) +{ + PgStatShared_SLRU *stats_shmem = (PgStatShared_SLRU *) stats; + + LWLockInitialize(&stats_shmem->lock, LWTRANCHE_PGSTATS_DATA); +} + void pgstat_slru_reset_all_cb(TimestampTz ts) { @@ -202,12 +212,14 @@ pgstat_slru_reset_all_cb(TimestampTz ts) void pgstat_slru_snapshot_cb(void) { - PgStatShared_SLRU *stats_shmem = &pgStatLocal.shmem->slru; + PgStatShared_SLRU *stats_shmem = (PgStatShared_SLRU *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_SLRU]; + PgStat_SLRUStats *stat_snap = (PgStat_SLRUStats *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_SLRU]; LWLockAcquire(&stats_shmem->lock, LW_SHARED); - memcpy(pgStatLocal.snapshot.slru, &stats_shmem->stats, - sizeof(stats_shmem->stats)); + memcpy(stat_snap, &stats_shmem->stats, sizeof(stats_shmem->stats)); LWLockRelease(&stats_shmem->lock); } @@ -237,7 +249,8 @@ get_slru_entry(int slru_idx) static void pgstat_reset_slru_counter_internal(int index, TimestampTz ts) { - PgStatShared_SLRU *stats_shmem = &pgStatLocal.shmem->slru; + PgStatShared_SLRU *stats_shmem = (PgStatShared_SLRU *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_SLRU]; LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE); diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/activity/pgstat_wal.c index 0e374f133a..030b339e47 100644 --- a/src/backend/utils/activity/pgstat_wal.c +++ b/src/backend/utils/activity/pgstat_wal.c @@ -68,7 +68,8 @@ pgstat_fetch_stat_wal(void) { pgstat_snapshot_fixed(PGSTAT_KIND_WAL); - return &pgStatLocal.snapshot.wal; + return (PgStat_WalStats *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_WAL]; } /* @@ -81,7 +82,8 @@ pgstat_fetch_stat_wal(void) bool pgstat_flush_wal(bool nowait) { - PgStatShared_Wal *stats_shmem = &pgStatLocal.shmem->wal; + PgStatShared_Wal *stats_shmem = (PgStatShared_Wal *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_WAL]; WalUsage wal_usage_diff = {0}; Assert(IsUnderPostmaster || !IsPostmasterEnvironment); @@ -163,10 +165,20 @@ pgstat_have_pending_wal(void) PendingWalStats.wal_sync != 0; } +void +pgstat_wal_init_shmem_cb(void *stats) +{ + PgStatShared_Wal *stats_shmem = (PgStatShared_Wal *) stats; + + LWLockInitialize(&stats_shmem->lock, LWTRANCHE_PGSTATS_DATA); +} + + void pgstat_wal_reset_all_cb(TimestampTz ts) { - PgStatShared_Wal *stats_shmem = &pgStatLocal.shmem->wal; + PgStatShared_Wal *stats_shmem = (PgStatShared_Wal *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_WAL]; LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE); memset(&stats_shmem->stats, 0, sizeof(stats_shmem->stats)); @@ -177,10 +189,12 @@ pgstat_wal_reset_all_cb(TimestampTz ts) void pgstat_wal_snapshot_cb(void) { - PgStatShared_Wal *stats_shmem = &pgStatLocal.shmem->wal; + PgStatShared_Wal *stats_shmem = (PgStatShared_Wal *) + pgStatLocal.shmem->fixed_data[PGSTAT_KIND_WAL]; + PgStat_WalStats *stat_snap = (PgStat_WalStats *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_WAL]; LWLockAcquire(&stats_shmem->lock, LW_SHARED); - memcpy(&pgStatLocal.snapshot.wal, &stats_shmem->stats, - sizeof(pgStatLocal.snapshot.wal)); + memcpy(stat_snap, &stats_shmem->stats, sizeof(PgStat_WalStats)); LWLockRelease(&stats_shmem->lock); } -- 2.45.2
From 790d900cca796a82aa3761da25597bf1496c2615 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Wed, 3 Jul 2024 13:39:01 +0900 Subject: [PATCH v3 2/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 13ddbcdcfb..fb27272e86 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -184,7 +184,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); /* ---------- @@ -1312,9 +1312,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 173976d81c0adeeef8768d708099b0b5f4584144 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Wed, 3 Jul 2024 15:04:45 +0900 Subject: [PATCH v3 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. 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 As of this patch, fixed-numbered stats kind (like WAL, archiver, bgwriter) are not supported, still some infrastructure is added to make its support easier, as the fixed_data areas for the shmem and snapshot structures are extended to cover custom and builtin stats kinds. --- src/include/pgstat.h | 35 ++++- src/include/utils/pgstat_internal.h | 8 +- src/backend/utils/activity/pgstat.c | 161 +++++++++++++++++++--- src/backend/utils/activity/pgstat_shmem.c | 8 +- src/backend/utils/adt/pgstatfuncs.c | 2 +- 5 files changed, 186 insertions(+), 28 deletions(-) diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 2d30fadaf1..8d523607a4 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 b8b2152d71..01b43a80e0 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -431,7 +431,7 @@ typedef struct PgStat_ShmemControl * * Each entry has a size of PgStat_KindInfo->shared_size. */ - void *fixed_data[PGSTAT_NUM_KINDS]; + void *fixed_data[PGSTAT_KIND_MAX + 1]; } PgStat_ShmemControl; @@ -451,8 +451,8 @@ typedef struct PgStat_Snapshot * Each entry is allocated in TopMemoryContext, for a size of * shared_data_len. */ - bool fixed_valid[PGSTAT_NUM_KINDS]; - void *fixed_data[PGSTAT_NUM_KINDS]; + bool fixed_valid[PGSTAT_KIND_MAX + 1]; + void *fixed_data[PGSTAT_KIND_MAX + 1]; /* to free snapshot in bulk */ MemoryContext context; @@ -497,6 +497,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 fb27272e86..994aa2ba51 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 @@ -253,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. @@ -265,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 */ @@ -432,6 +440,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. @@ -997,11 +1014,11 @@ static void pgstat_init_snapshot(void) { /* Initialize fixed-numbered statistics data in snapshots */ - 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; pgStatLocal.snapshot.fixed_data[kind] = @@ -1102,10 +1119,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); @@ -1299,30 +1318,125 @@ 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\"."))); + + /* + * These are not supported for now, as these point out to fixed areas of + * shared memory. + */ + if (kind_info->fixed_amount) + ereport(ERROR, + (errmsg("custom cumulative statistics property is invalid"), + errhint("Custom cumulative statistics cannot use a fixed amount of data."))); + + /* + * If pgstat_kind_custom_infos is not available yet, allocate it. + */ + if (pgstat_kind_custom_infos == NULL) + { + pgstat_kind_custom_infos = (const PgStat_KindInfo **) + MemoryContextAllocZero(TopMemoryContext, + sizeof(PgStat_KindInfo *) * PGSTAT_KIND_CUSTOM_SIZE); + } + + if (pgstat_kind_custom_infos[idx] != NULL && + pgstat_kind_custom_infos[idx]->name != NULL) + ereport(ERROR, + (errmsg("failed to register custom cumulative statistics \"%s\" with ID %u", kind_info->name, kind), + errdetail("Custom 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))); } /* @@ -1399,12 +1513,12 @@ pgstat_write_statsfile(void) write_chunk_s(fpout, &format_id); /* Write various stats structs with 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->shared_size != 0 && info->shared_data_len != 0); @@ -1434,6 +1548,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); @@ -1649,7 +1774,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; } @@ -1706,12 +1831,12 @@ pgstat_reset_after_failure(void) { TimestampTz ts = GetCurrentTimestamp(); - /* reset fixed-numbered stats */ - for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++) + /* reset fixed-numbered stats for built-in */ + for (int kind = PGSTAT_KIND_MIN; 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 bd62c4b72a..0b50d58cce 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -132,10 +132,12 @@ StatsShmemSize(void) sz = add_size(sz, pgstat_dsa_init_size()); /* Add shared memory for all the fixed-numbered statistics */ - 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) continue; @@ -210,11 +212,11 @@ StatsShmemInit(void) pg_atomic_init_u64(&ctl->gc_request_count, 1); /* initialize fixed-numbered statistics */ - 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; Assert(kind_info->shared_size != 0); 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 2286f57cddd6e5b1cf7b6f771c9f08f9acb2a937 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Wed, 3 Jul 2024 13:49:53 +0900 Subject: [PATCH v3 4/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 | 63 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index f3a3e4e2f8..7e34c5020b 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3686,6 +3686,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 143d38f0292e08cde279a3073a5ea45844601cc7 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Wed, 3 Jul 2024 13:58:58 +0900 Subject: [PATCH v3 5/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. 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 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 1b695a1820..f65b34ce82 100644 --- a/src/test/modules/injection_points/injection_points.c +++ b/src/test/modules/injection_points/injection_points.c @@ -18,6 +18,7 @@ #include "postgres.h" #include "fmgr.h" +#include "injection_stats.h" #include "miscadmin.h" #include "nodes/pg_list.h" #include "nodes/value.h" @@ -170,6 +171,9 @@ injection_points_cleanup(int code, Datum arg) char *name = strVal(lfirst(lc)); (void) InjectionPointDetach(name); + + /* Remove stats entry */ + pgstat_drop_inj(name); } } @@ -182,6 +186,8 @@ injection_error(const char *name, const void *private_data) if (!injection_point_allowed(condition)) return; + pgstat_report_inj(name); + elog(ERROR, "error triggered for injection point %s", name); } @@ -193,6 +199,8 @@ injection_notice(const char *name, const void *private_data) if (!injection_point_allowed(condition)) return; + pgstat_report_inj(name); + elog(NOTICE, "notice triggered for injection point %s", name); } @@ -211,6 +219,8 @@ injection_wait(const char *name, const void *private_data) if (!injection_point_allowed(condition)) return; + pgstat_report_inj(name); + /* * Use the injection point name for this custom wait event. Note that * this custom wait event name is not released, but we don't care much for @@ -299,6 +309,10 @@ injection_points_attach(PG_FUNCTION_ARGS) inj_list_local = lappend(inj_list_local, makeString(pstrdup(name))); MemoryContextSwitchTo(oldctx); } + + /* Add entry for stats */ + pgstat_create_inj(name); + PG_RETURN_VOID(); } @@ -400,5 +414,18 @@ injection_points_detach(PG_FUNCTION_ARGS) MemoryContextSwitchTo(oldctx); } + /* Remove stats entry */ + pgstat_drop_inj(name); + PG_RETURN_VOID(); } + + +void +_PG_init(void) +{ + if (!process_shared_preload_libraries_in_progress) + return; + + pgstat_register_inj(); +} diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c new file mode 100644 index 0000000000..c37b0b33d3 --- /dev/null +++ b/src/test/modules/injection_points/injection_stats.c @@ -0,0 +1,194 @@ +/*-------------------------------------------------------------------------- + * + * injection_stats.c + * Code for statistics of injection points. + * + * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * src/test/modules/injection_points/injection_stats.c + * + * ------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "fmgr.h" + +#include "common/hashfn.h" +#include "injection_stats.h" +#include "pgstat.h" +#include "utils/builtins.h" +#include "utils/pgstat_internal.h" + +/* Structures for statistics of injection points */ +typedef struct PgStat_StatInjEntry +{ + PgStat_Counter numcalls; /* number of times point has been run */ +} PgStat_StatInjEntry; + +typedef struct PgStatShared_InjectionPoint +{ + PgStatShared_Common header; + PgStat_StatInjEntry stats; +} PgStatShared_InjectionPoint; + +static bool injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait); + +static const PgStat_KindInfo injection_stats = { + .name = "injection_points", + .fixed_amount = false, /* Bounded by the number of points */ + + /* Injection points are system-wide */ + .accessed_across_databases = true, + + .shared_size = sizeof(PgStatShared_InjectionPoint), + .shared_data_off = offsetof(PgStatShared_InjectionPoint, stats), + .shared_data_len = sizeof(((PgStatShared_InjectionPoint *) 0)->stats), + .pending_size = sizeof(PgStat_StatInjEntry), + .flush_pending_cb = injection_stats_flush_cb, +}; + +/* + * Compute stats entry idx from point name with a 4-byte hash. + */ +#define PGSTAT_INJ_IDX(name) hash_bytes((const unsigned char *) name, strlen(name)) + +#define PGSTAT_KIND_INJECTION PGSTAT_KIND_EXPERIMENTAL + +/* Track if stats are loaded */ +static bool inj_stats_loaded = false; + +/* + * Callback for stats handling + */ +static bool +injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) +{ + PgStat_StatInjEntry *localent; + PgStatShared_InjectionPoint *shfuncent; + + localent = (PgStat_StatInjEntry *) entry_ref->pending; + shfuncent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats; + + if (!pgstat_lock_entry(entry_ref, nowait)) + return false; + + shfuncent->stats.numcalls += localent->numcalls; + return true; +} + +/* + * Support function for the SQL-callable pgstat* functions. Returns + * a pointer to the injection point statistics struct. + */ +static PgStat_StatInjEntry * +pgstat_fetch_stat_injentry(const char *name) +{ + PgStat_StatInjEntry *entry = NULL; + + if (!inj_stats_loaded) + return NULL; + + /* Compile the lookup key as a hash of the point name */ + entry = (PgStat_StatInjEntry *) pgstat_fetch_entry(PGSTAT_KIND_INJECTION, + InvalidOid, + PGSTAT_INJ_IDX(name)); + return entry; +} + +/* + * Workhorse to do the registration work, called in _PG_init(). + */ +void +pgstat_register_inj(void) +{ + pgstat_register_kind(PGSTAT_KIND_INJECTION, &injection_stats); + + /* mark stats as loaded */ + inj_stats_loaded = true; +} + +/* + * Report injection point creation. + */ +void +pgstat_create_inj(const char *name) +{ + PgStat_EntryRef *entry_ref; + PgStatShared_InjectionPoint *shstatent; + + /* leave if disabled */ + if (!inj_stats_loaded) + return; + + entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_INJECTION, InvalidOid, + PGSTAT_INJ_IDX(name), false); + shstatent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats; + + /* initialize shared memory data */ + memset(&shstatent->stats, 0, sizeof(shstatent->stats)); + pgstat_unlock_entry(entry_ref); +} + +/* + * Report injection point drop. + */ +void +pgstat_drop_inj(const char *name) +{ + /* leave if disabled */ + if (!inj_stats_loaded) + return; + + if (!pgstat_drop_entry(PGSTAT_KIND_INJECTION, InvalidOid, + PGSTAT_INJ_IDX(name))) + pgstat_request_entry_refs_gc(); +} + +/* + * Report statistics for injection point. + * + * This is simple because the set of stats to report currently is simple: + * track the number of times a point has been run. + */ +void +pgstat_report_inj(const char *name) +{ + PgStat_EntryRef *entry_ref; + PgStatShared_InjectionPoint *shstatent; + PgStat_StatInjEntry *statent; + + /* leave if disabled */ + if (!inj_stats_loaded) + return; + + entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_INJECTION, InvalidOid, + PGSTAT_INJ_IDX(name), false); + + shstatent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats; + statent = &shstatent->stats; + + /* Update the injection point statistics */ + statent->numcalls++; + + pgstat_unlock_entry(entry_ref); +} + +/* + * SQL function returning the number of times an injection point + * has been called. + */ +PG_FUNCTION_INFO_V1(injection_points_stats_numcalls); +Datum +injection_points_stats_numcalls(PG_FUNCTION_ARGS) +{ + char *name = text_to_cstring(PG_GETARG_TEXT_PP(0)); + PgStat_StatInjEntry *entry = pgstat_fetch_stat_injentry(name); + + if (entry == NULL) + PG_RETURN_NULL(); + + PG_RETURN_INT64(entry->numcalls); +} diff --git a/src/test/modules/injection_points/injection_stats.h b/src/test/modules/injection_points/injection_stats.h new file mode 100644 index 0000000000..3e99705483 --- /dev/null +++ b/src/test/modules/injection_points/injection_stats.h @@ -0,0 +1,23 @@ +/*-------------------------------------------------------------------------- + * + * injection_stats.h + * Definitions for statistics of injection points. + * + * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * src/test/modules/injection_points/injection_stats.h + * + * ------------------------------------------------------------------------- + */ + +#ifndef INJECTION_STATS +#define INJECTION_STATS + +extern void pgstat_register_inj(void); +extern void pgstat_create_inj(const char *name); +extern void pgstat_drop_inj(const char *name); +extern void pgstat_report_inj(const char *name); + +#endif diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 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 e6c1caf649..b81c137479 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2116,6 +2116,7 @@ PgStatShared_Common PgStatShared_Database PgStatShared_Function PgStatShared_HashEntry +PgStatShared_InjectionPoint PgStatShared_IO PgStatShared_Relation PgStatShared_ReplSlot @@ -2147,6 +2148,7 @@ PgStat_Snapshot PgStat_SnapshotEntry PgStat_StatDBEntry PgStat_StatFuncEntry +PgStat_StatInjEntry PgStat_StatReplSlotEntry PgStat_StatSubEntry PgStat_StatTabEntry -- 2.45.2
From 3cda5fe5588a20c3f037334a4879289ca57ec222 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Wed, 3 Jul 2024 18:26:25 +0900 Subject: [PATCH v3 6/6] Add support for fixed-numbered statistics in pluggable pgstats facility These kinds of stats (like WAL, bgwriter, checkpointer, etc.) are stored in shared memory with a fixed amount of entries. With the previous commits in place, this commit only lifts a past restriction in place. The meat of the change is in the test module injection_points, that provides an example of how to implement fixed-numbered stats kinds. --- src/backend/utils/activity/pgstat.c | 9 -- .../injection_points--1.0.sql | 11 ++ .../injection_points/injection_points.c | 3 + .../injection_points/injection_stats.c | 153 +++++++++++++++++- .../injection_points/injection_stats.h | 3 + .../modules/injection_points/t/001_stats.pl | 11 +- 6 files changed, 178 insertions(+), 12 deletions(-) diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 994aa2ba51..d87999d091 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -1395,15 +1395,6 @@ pgstat_register_kind(PgStat_Kind kind, const PgStat_KindInfo *kind_info) (errmsg("failed to register custom cumulative statistics \"%s\" with ID %u", kind_info->name, kind), errdetail("Custom cumulative statistics must be registered while initializing modules in \"shared_preload_libraries\"."))); - /* - * These are not supported for now, as these point out to fixed areas of - * shared memory. - */ - if (kind_info->fixed_amount) - ereport(ERROR, - (errmsg("custom cumulative statistics property is invalid"), - errhint("Custom cumulative statistics cannot use a fixed amount of data."))); - /* * If pgstat_kind_custom_infos is not available yet, allocate it. */ 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 deaf47d8ae..a52b7a6b7c 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,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 f65b34ce82..c71ad0d367 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)); @@ -325,6 +326,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(); @@ -401,6 +403,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..53440f5184 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,49 @@ 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) /* 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 +114,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->fixed_data[PGSTAT_KIND_INJECTION_FIXED]; + + 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->fixed_data[PGSTAT_KIND_INJECTION_FIXED]; + PgStat_StatInjFixedEntry *stat_snap = (PgStat_StatInjFixedEntry *) + pgStatLocal.snapshot.fixed_data[PGSTAT_KIND_INJECTION_FIXED]; + 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 +193,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 +265,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->fixed_data[PGSTAT_KIND_INJECTION_FIXED]; + + 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 +305,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.fixed_data[PGSTAT_KIND_INJECTION_FIXED]; + + /* 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